1
0
Fork 0
mirror of https://github.com/maybe-finance/maybe.git synced 2025-07-19 21:29:38 +02:00

Implement error handling and logging for sparkline and series methods

- Added rescue blocks to handle exceptions in the Accounts and AccountableSparklines controllers, logging errors and rendering error partials.
- Enhanced error handling in the Account::Chartable and Balance::ChartSeriesBuilder models, logging specific error messages for series generation failures.
- Updated the accounts view to include a timeout for Turbo frame loading.
- Added a test to ensure graceful handling of sparkline errors in the AccountsController.

In reference to bug #2315
This commit is contained in:
Josh Pigford 2025-05-26 20:05:16 -05:00
parent 3cc88f3e98
commit 6f67827f14
9 changed files with 113 additions and 8 deletions

View file

@ -17,6 +17,9 @@ class AccountableSparklinesController < ApplicationController
end end
render layout: false render layout: false
rescue => e
Rails.logger.error "Accountable sparkline error for #{@accountable&.name}: #{e.message}"
render partial: "accountable_sparklines/error", layout: false
end end
private private

View file

@ -24,6 +24,9 @@ class AccountsController < ApplicationController
def sparkline def sparkline
render layout: false render layout: false
rescue => e
Rails.logger.error "Sparkline error for account #{@account.id}: #{e.message}"
render partial: "accounts/sparkline_error", layout: false
end end
private private

View file

@ -0,0 +1,42 @@
import { Controller } from "@hotwired/stimulus"
// Connects to data-controller="turbo-frame-timeout"
export default class extends Controller {
static values = { timeout: { type: Number, default: 10000 } }
connect() {
this.timeoutId = setTimeout(() => {
this.handleTimeout()
}, this.timeoutValue)
// Listen for successful frame loads to clear timeout
this.element.addEventListener("turbo:frame-load", this.clearTimeout.bind(this))
}
disconnect() {
this.clearTimeout()
}
clearTimeout() {
if (this.timeoutId) {
clearTimeout(this.timeoutId)
this.timeoutId = null
}
}
handleTimeout() {
// Replace loading content with error state
this.element.innerHTML = `
<div class="flex items-center justify-end gap-1">
<div class="w-8 h-4 flex items-center justify-center">
<svg xmlns="http://www.w3.org/2000/svg" width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="text-warning">
<path d="m21.73 18-8-14a2 2 0 0 0-3.48 0l-8 14A2 2 0 0 0 4 21h16a2 2 0 0 0 1.73-3Z"/>
<path d="M12 9v4"/>
<path d="m12 17 .01 0"/>
</svg>
</div>
<p class="font-mono text-right text-xs text-warning">Timeout</p>
</div>
`
}
}

View file

@ -29,5 +29,15 @@ module Account::Chartable
Rails.cache.fetch(cache_key) do Rails.cache.fetch(cache_key) do
balance_series balance_series
end end
rescue => e
Rails.logger.error "Sparkline series error for account #{id}: #{e.message}"
# Return empty series as fallback
Series.new(
start_date: 30.days.ago.to_date,
end_date: Date.current,
interval: "1 day",
values: [],
favorable_direction: favorable_direction
)
end end
end end

View file

@ -9,14 +9,23 @@ class Balance::ChartSeriesBuilder
def balance_series def balance_series
build_series_for(:balance) build_series_for(:balance)
rescue => e
Rails.logger.error "Balance series error: #{e.message} for accounts #{@account_ids}"
raise
end end
def cash_balance_series def cash_balance_series
build_series_for(:cash_balance) build_series_for(:cash_balance)
rescue => e
Rails.logger.error "Cash balance series error: #{e.message} for accounts #{@account_ids}"
raise
end end
def holdings_balance_series def holdings_balance_series
build_series_for(:holdings_balance) build_series_for(:holdings_balance)
rescue => e
Rails.logger.error "Holdings balance series error: #{e.message} for accounts #{@account_ids}"
raise
end end
private private
@ -61,6 +70,9 @@ class Balance::ChartSeriesBuilder
sign_multiplier: sign_multiplier sign_multiplier: sign_multiplier
} }
]) ])
rescue => e
Rails.logger.error "Query data error: #{e.message} for accounts #{account_ids}, period #{period.start_date} to #{period.end_date}"
raise
end end
# Since the query aggregates the *net* of assets - liabilities, this means that if we're looking at # Since the query aggregates the *net* of assets - liabilities, this means that if we're looking at

View file

@ -0,0 +1,8 @@
<%= turbo_frame_tag "#{params[:accountable_type]}_sparkline" do %>
<div class="flex items-center justify-end gap-1">
<div class="w-8 h-3 flex items-center justify-center">
<%= icon("alert-triangle", size: "sm", class: "text-warning") %>
</div>
<p class="font-mono text-right text-xs text-warning">Error</p>
</div>
<% end %>

View file

@ -14,7 +14,7 @@
</div> </div>
<% else %> <% else %>
<%= tag.p format_money(account_group.total_money), class: "text-sm font-medium text-primary" %> <%= tag.p format_money(account_group.total_money), class: "text-sm font-medium text-primary" %>
<%= turbo_frame_tag "#{account_group.key}_sparkline", src: accountable_sparkline_path(account_group.key), loading: "lazy" do %> <%= turbo_frame_tag "#{account_group.key}_sparkline", src: accountable_sparkline_path(account_group.key), loading: "lazy", data: { controller: "turbo-frame-timeout", turbo_frame_timeout_timeout_value: 10000 } do %>
<div class="flex items-center w-8 h-4 ml-auto"> <div class="flex items-center w-8 h-4 ml-auto">
<div class="w-6 h-px bg-loader"></div> <div class="w-6 h-px bg-loader"></div>
</div> </div>
@ -50,7 +50,7 @@
<% else %> <% else %>
<div class="ml-auto text-right grow h-10"> <div class="ml-auto text-right grow h-10">
<%= tag.p format_money(account.balance_money), class: "text-sm font-medium text-primary whitespace-nowrap" %> <%= tag.p format_money(account.balance_money), class: "text-sm font-medium text-primary whitespace-nowrap" %>
<%= turbo_frame_tag dom_id(account, :sparkline), src: sparkline_account_path(account), loading: "lazy" do %> <%= turbo_frame_tag dom_id(account, :sparkline), src: sparkline_account_path(account), loading: "lazy", data: { controller: "turbo-frame-timeout", turbo_frame_timeout_timeout_value: 10000 } do %>
<div class="flex items-center w-8 h-4 ml-auto"> <div class="flex items-center w-8 h-4 ml-auto">
<div class="w-6 h-px bg-loader"></div> <div class="w-6 h-px bg-loader"></div>
</div> </div>

View file

@ -0,0 +1,8 @@
<%= turbo_frame_tag dom_id(@account, :sparkline) do %>
<div class="flex items-center justify-end gap-1">
<div class="w-8 h-5 flex items-center justify-center">
<%= icon("alert-triangle", size: "sm", class: "text-warning") %>
</div>
<p class="font-mono text-right text-xs text-warning">Error</p>
</div>
<% end %>

View file

@ -6,13 +6,32 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
@account = accounts(:depository) @account = accounts(:depository)
end end
test "new" do test "should get index" do
get new_account_path get accounts_url
assert_response :ok assert_response :success
end end
test "can sync an account" do test "should sync account" do
post sync_account_path(@account) post sync_account_url(@account)
assert_redirected_to account_path(@account) assert_redirected_to account_url(@account)
end
test "should get chart" do
get chart_account_url(@account)
assert_response :success
end
test "should get sparkline" do
get sparkline_account_url(@account)
assert_response :success
end
test "should handle sparkline errors gracefully" do
# Mock an error in the sparkline_series method
@account.stubs(:sparkline_series).raises(StandardError.new("Test error"))
get sparkline_account_url(@account)
assert_response :success
assert_match /Error/, response.body
end end
end end