maybe-finance/maybe

Invalid data state: accountable records missing for some accounts in self hosted instances

jakubkottnauer opened this issue · 13 comments

Describe the bug
I'm getting a 500 error page right after opening Maybe. My Maybe instances auto-updates every night, I believe it started crashing ~2 days ago.

Screenshots / Recordings
Here's a log with that should provide enough info to get to the bottom of this.
Screenshot 2024-08-07 at 15 23 19

@jakubkottnauer would you mind running this against your DB to see if this is a stale data issue?

select accountable_id, accountable_type from accounts;

Hello @zachgoll,

I have the same issue on my end, I was starting to write an issue and saw this one.

2024-08-07T03:36:38.946816215Z I, [2024-08-07T03:36:38.946637 #1] INFO -- : [61abdebc-e433-4e8d-8168-1e43426f75d8] Completed 500 Internal Server Error in 7148ms (ActiveRecord: 5682.6ms (29 queries, 0 cached) | GC: 13.3ms)
2024-08-07T03:36:38.947723619Z E, [2024-08-07T03:36:38.947462 #1] ERROR -- : [61abdebc-e433-4e8d-8168-1e43426f75d8]  
2024-08-07T03:36:38.947730419Z [61abdebc-e433-4e8d-8168-1e43426f75d8] ActiveSupport::DelegationError (series delegated to accountable, but accountable is nil):
2024-08-07T03:36:38.947733979Z [61abdebc-e433-4e8d-8168-1e43426f75d8]
2024-08-07T03:36:38.947736539Z Causes:
2024-08-07T03:36:38.947738859Z [61abdebc-e433-4e8d-8168-1e43426f75d8] NoMethodError (undefined method `series' for nil)
2024-08-07T03:36:38.947759659Z [61abdebc-e433-4e8d-8168-1e43426f75d8]   
2024-08-07T03:36:38.947762660Z [61abdebc-e433-4e8d-8168-1e43426f75d8] app/models/account.rb:31:in `rescue in series'
2024-08-07T03:36:38.947765220Z [61abdebc-e433-4e8d-8168-1e43426f75d8] app/models/account.rb:31:in `series'
2024-08-07T03:36:38.947767700Z [61abdebc-e433-4e8d-8168-1e43426f75d8] app/models/account.rb:44:in `block (3 levels) in by_group'
2024-08-07T03:36:38.947770180Z [61abdebc-e433-4e8d-8168-1e43426f75d8] app/models/account.rb:40:in `block (2 levels) in by_group'
2024-08-07T03:36:38.947772660Z [61abdebc-e433-4e8d-8168-1e43426f75d8] app/models/account.rb:38:in `each'
2024-08-07T03:36:38.947775060Z [61abdebc-e433-4e8d-8168-1e43426f75d8] app/models/account.rb:38:in `block in by_group'
2024-08-07T03:36:38.947777700Z [61abdebc-e433-4e8d-8168-1e43426f75d8] app/models/account.rb:37:in `each'
2024-08-07T03:36:38.947780060Z [61abdebc-e433-4e8d-8168-1e43426f75d8] app/models/account.rb:37:in `by_group'
2024-08-07T03:36:38.947782620Z [61abdebc-e433-4e8d-8168-1e43426f75d8] app/controllers/pages_controller.rb:23:in `dashboard'

I've executed the query you mentioned

select accountable_id, accountable_type from accounts;
Screenshot 2024-08-08 at 07 15 53

@jermorin thanks for providing that. Appears to be a bug since all of the references seem to be intact.

I'll take a closer look at this shortly.

@jakubkottnauer @jermorin just to confirm, you're both on the latest version of Maybe and have migrated the database to the latest schema correct? (DB migrations should happen on container startup)

@zachgoll Thanks for your answer, I confirm that I'm using the latest version available in the docker registry.

docker images --digests | grep maybe
ghcr.io/maybe-finance/maybe         latest                         sha256:dc43c36445f1b8cdc0f70a46af88b58a3f367fa01812beceb30eaac5f07ad879   2d985b03aca0   15 hours ago    708MB

I don't see any error at the container startup

select * from schema_migrations order by version desc limit 1

It seems to be the latest migration file

20240731191344

@jermorin awesome, very helpful thank you!

@jermorin would you mind running this updated query so we can see if any of your "accountables" records have accidentally been deleted?

I'm able to reproduce this error by manually deleting the "accountable" record for an account, but struggling to see how that could have happened in the first place:

SELECT 
    a.accountable_type,
    CASE a.accountable_type
        WHEN 'Crypto' THEN cryptos.id
        WHEN 'Depository' THEN depositories.id
        WHEN 'OtherAsset' THEN other_assets.id
        WHEN 'OtherLiability' THEN other_liabilities.id
        WHEN 'Investment' THEN investments.id
        WHEN 'Property' THEN properties.id
        WHEN 'Vehicle' THEN vehicles.id
        WHEN 'CreditCard' THEN credit_cards.id
        WHEN 'Loan' THEN loans.id
    END AS accountable_id
FROM accounts a 
LEFT JOIN cryptos ON cryptos.id = a.accountable_id AND a.accountable_type = 'Crypto'
LEFT JOIN depositories ON depositories.id = a.accountable_id AND a.accountable_type = 'Depository'
LEFT JOIN other_assets ON other_assets.id = a.accountable_id AND a.accountable_type = 'OtherAsset'
LEFT JOIN other_liabilities ON other_liabilities.id = a.accountable_id AND a.accountable_type = 'OtherLiability'
LEFT JOIN investments ON investments.id = a.accountable_id AND a.accountable_type = 'Investment'
LEFT JOIN properties ON properties.id = a.accountable_id AND a.accountable_type = 'Property'
LEFT JOIN vehicles ON vehicles.id = a.accountable_id AND a.accountable_type = 'Vehicle'
LEFT JOIN credit_cards ON credit_cards.id = a.accountable_id AND a.accountable_type = 'CreditCard'
LEFT JOIN loans ON loans.id = a.accountable_id AND a.accountable_type = 'Loan';

We're looking for any NULL values in the accountable_id column:

CleanShot 2024-08-09 at 12 09 29@2x

I'll merge #1071 to avoid the app crash, but leaving this issue open until we identify the root cause.

Thanks @zachgoll, can confirm the latest Maybe version works again 👍

@zachgoll I confirm the latest version works, thanks a lot

The result of the query you sent

Screenshot 2024-08-10 at 08 32 01

@jakubkottnauer @jermorin thanks for confirming!

Based on @jermorin's screenshot, it appears that this is a referential integrity issue, which I believe must have happened in a prior state of the app (this shouldn't happen moving forward).

What do you both think of the following solution moving forward?

  1. Write a migration that creates an empty "accountable" record for each account where one is missing
  2. Revert my temporary changes (I don't think we want to suppress this sort of error in the long term since it is so integral to the app's domain)

@zachgoll I believe your approach is correct; the migration should resolve the issue. Indeed, it's challenging to maintain the evolving state of the app. I've been using Maybe, since it's available in the docker registry. Therefore, it's understandable that occasionally changes might impact its current stability.

@jermorin yep we're starting to reach a bit of stability now so hopefully the experience continues to improve for self hosting :)