From 938a7c11f9740024da3df5ed5f2adc85e4698ebd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Mar 2026 19:49:16 +0000 Subject: [PATCH] [AI] Fix SimpleFin batch sync crash when accounts are missing from response When SimpleFin doesn't return data for all requested accounts during batch sync, the code crashed with a TypeError accessing properties on undefined, resulting in a generic "internal error" message for users. This fix: - Adds a guard in simpleFinBatchSync for missing account data, returning an ACCOUNT_MISSING error instead of crashing - Propagates error entries from the SimpleFin response's errors map for accounts that have no data entry - Adds a user-friendly ACCOUNT_MISSING error message in the UI suggesting to unlink and relink the account - Adds test cases covering both scenarios https://claude.ai/code/session_01XbHgxxrXYR3UTyW6VmYj47 --- .../components/accounts/AccountSyncCheck.tsx | 5 + .../src/server/accounts/sync.test.ts | 133 +++++++++++++++++- .../loot-core/src/server/accounts/sync.ts | 49 +++++-- 3 files changed, 177 insertions(+), 10 deletions(-) diff --git a/packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx b/packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx index 9d78ad4734..ca0d7786bf 100644 --- a/packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx +++ b/packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx @@ -68,6 +68,11 @@ function useErrorMessage() { ); + case 'ACCOUNT_MISSING': + return t( + 'This account was not found in SimpleFIN. Try unlinking and relinking the account.', + ); + default: } diff --git a/packages/loot-core/src/server/accounts/sync.test.ts b/packages/loot-core/src/server/accounts/sync.test.ts index ced063c781..6ca0bdcbe8 100644 --- a/packages/loot-core/src/server/accounts/sync.test.ts +++ b/packages/loot-core/src/server/accounts/sync.test.ts @@ -1,13 +1,19 @@ // @ts-strict-ignore +import * as asyncStorage from '../../platform/server/asyncStorage'; import * as monthUtils from '../../shared/months'; import type { SyncedPrefs } from '../../types/prefs'; import * as db from '../db'; import { loadMappings } from '../db/mappings'; import { post } from '../post'; import { getServer } from '../server-config'; +import { handlers } from '../tests/mockSyncServer'; import { insertRule, loadRules } from '../transactions/transaction-rules'; -import { addTransactions, reconcileTransactions } from './sync'; +import { + addTransactions, + reconcileTransactions, + simpleFinBatchSync, +} from './sync'; vi.mock('../../shared/months', async () => ({ ...(await vi.importActual('../../shared/months')), @@ -546,3 +552,128 @@ describe('Account sync', () => { }, ); }); + +describe('SimpleFin batch sync', () => { + function mockSimpleFinTransactions(response) { + vi.mocked(asyncStorage.getItem).mockResolvedValue('test-token'); + handlers['/simplefin/transactions'] = () => response; + } + + afterEach(() => { + delete handlers['/simplefin/transactions']; + }); + + test('returns ACCOUNT_MISSING error when an account is not in the response', async () => { + const presentAccountId = 'sf-account-1'; + const missingAccountId = 'sf-account-2'; + + // Mock SimpleFin response that only returns data for one of two accounts + mockSimpleFinTransactions({ + [presentAccountId]: { + transactions: { all: [], booked: [], pending: [] }, + balances: [], + startingBalance: 0, + }, + errors: {}, + }); + + // Insert two accounts linked to SimpleFin + const acct1Id = await db.insertAccount({ + id: 'acct-1', + account_id: presentAccountId, + name: 'Account 1', + account_sync_source: 'simpleFin', + }); + await db.insertPayee({ + id: 'transfer-' + acct1Id, + name: '', + transfer_acct: acct1Id, + }); + + const acct2Id = await db.insertAccount({ + id: 'acct-2', + account_id: missingAccountId, + name: 'Account 2', + account_sync_source: 'simpleFin', + }); + await db.insertPayee({ + id: 'transfer-' + acct2Id, + name: '', + transfer_acct: acct2Id, + }); + + const results = await simpleFinBatchSync([ + { id: 'acct-1', account_id: presentAccountId }, + { id: 'acct-2', account_id: missingAccountId }, + ]); + + // The present account should succeed (no error_code) + const presentResult = results.find(r => r.accountId === 'acct-1'); + expect(presentResult).toBeDefined(); + expect(presentResult.res.error_code).toBeUndefined(); + + // The missing account should have ACCOUNT_MISSING error + const missingResult = results.find(r => r.accountId === 'acct-2'); + expect(missingResult).toBeDefined(); + expect(missingResult.res.error_code).toBe('ACCOUNT_MISSING'); + expect(missingResult.res.error_type).toBe('ACCOUNT_MISSING'); + }); + + test('propagates ACCOUNT_MISSING error from SimpleFin response errors', async () => { + const presentAccountId = 'sf-account-1'; + const missingAccountId = 'sf-account-2'; + + // Mock SimpleFin response with error entry for missing account + mockSimpleFinTransactions({ + [presentAccountId]: { + transactions: { all: [], booked: [], pending: [] }, + balances: [], + startingBalance: 0, + }, + errors: { + [missingAccountId]: [ + { + error_type: 'ACCOUNT_MISSING', + error_code: 'ACCOUNT_MISSING', + reason: 'Account not found', + }, + ], + }, + }); + + const acct1Id = await db.insertAccount({ + id: 'acct-1', + account_id: presentAccountId, + name: 'Account 1', + account_sync_source: 'simpleFin', + }); + await db.insertPayee({ + id: 'transfer-' + acct1Id, + name: '', + transfer_acct: acct1Id, + }); + + const acct2Id = await db.insertAccount({ + id: 'acct-2', + account_id: missingAccountId, + name: 'Account 2', + account_sync_source: 'simpleFin', + }); + await db.insertPayee({ + id: 'transfer-' + acct2Id, + name: '', + transfer_acct: acct2Id, + }); + + const results = await simpleFinBatchSync([ + { id: 'acct-1', account_id: presentAccountId }, + { id: 'acct-2', account_id: missingAccountId }, + ]); + + // The missing account should get the ACCOUNT_MISSING error from the errors map + const missingResult = results.find(r => r.accountId === 'acct-2'); + expect(missingResult).toBeDefined(); + expect(missingResult.res.error_code).toBe('ACCOUNT_MISSING'); + expect(missingResult.res.error_type).toBe('ACCOUNT_MISSING'); + }); +}); diff --git a/packages/loot-core/src/server/accounts/sync.ts b/packages/loot-core/src/server/accounts/sync.ts index 7ec07dc847..6ec4bb9014 100644 --- a/packages/loot-core/src/server/accounts/sync.ts +++ b/packages/loot-core/src/server/accounts/sync.ts @@ -18,7 +18,6 @@ import { import type { AccountEntity, BankSyncResponse, - SimpleFinBatchSyncResponse, TransactionEntity, } from '../../types/models'; import { aqlQuery } from '../aql'; @@ -226,12 +225,12 @@ async function downloadSimpleFinTransactions( let retVal = {}; if (batchSync) { - for (const [accountId, data] of Object.entries( - res as SimpleFinBatchSyncResponse, - )) { + const batchErrors = res.errors; + for (const accountId of Object.keys(res)) { if (accountId === 'errors') continue; - const error = res?.errors?.[accountId]?.[0]; + const data = res[accountId]; + const error = batchErrors?.[accountId]?.[0]; retVal[accountId] = { transactions: data?.transactions?.all, @@ -244,12 +243,31 @@ async function downloadSimpleFinTransactions( retVal[accountId].error_code = error.error_code; } } + + // Add entries for accounts that only have errors (no data in the response) + if (batchErrors) { + for (const [accountId, errorList] of Object.entries(batchErrors)) { + if ( + !retVal[accountId] && + Array.isArray(errorList) && + errorList.length > 0 + ) { + const error = errorList[0]; + retVal[accountId] = { + transactions: [], + accountBalance: [], + startingBalance: 0, + error_type: error.error_type, + error_code: error.error_code, + }; + } + } + } } else { - const singleRes = res as BankSyncResponse; retVal = { - transactions: singleRes.transactions.all, - accountBalance: singleRes.balances, - startingBalance: singleRes.startingBalance, + transactions: res.transactions.all, + accountBalance: res.balances, + startingBalance: res.startingBalance, }; } @@ -1079,6 +1097,19 @@ export async function simpleFinBatchSync( const account = accounts[i]; const download = res[account.account_id]; + if (!download) { + promises.push( + Promise.resolve({ + accountId: account.id, + res: { + error_type: 'ACCOUNT_MISSING', + error_code: 'ACCOUNT_MISSING', + }, + }), + ); + continue; + } + const acctRow = await db.select('accounts', account.id); const oldestTransaction = await getAccountOldestTransaction(account.id); const newAccount = oldestTransaction == null;