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/app.ts b/packages/loot-core/src/server/accounts/app.ts index efc8fa4993..38ce4f05a5 100644 --- a/packages/loot-core/src/server/accounts/app.ts +++ b/packages/loot-core/src/server/accounts/app.ts @@ -1069,7 +1069,7 @@ async function simpleFinBatchSync({ const matchedTransactions: Array = []; const updatedAccounts: Array = []; - if (syncResponse.res.error_code) { + if (syncResponse.res?.error_code) { errors.push( handleSyncError( { @@ -1081,7 +1081,7 @@ async function simpleFinBatchSync({ account, ), ); - } else { + } else if (syncResponse.res) { const syncResponseData = await handleSyncResponse( syncResponse.res, account, @@ -1090,6 +1090,15 @@ async function simpleFinBatchSync({ newTransactions.push(...syncResponseData.newTransactions); matchedTransactions.push(...syncResponseData.matchedTransactions); updatedAccounts.push(...syncResponseData.updatedAccounts); + } else { + errors.push( + handleSyncError( + new Error( + 'Failed syncing account "' + account.name + '": empty response', + ), + account, + ), + ); } retVal.push({ 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..e986c6fedb 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, }; } @@ -1074,11 +1092,34 @@ export async function simpleFinBatchSync( startDates, ); + if (!res) { + return accounts.map(account => ({ + accountId: account.id, + res: { + error_type: 'NO_DATA', + error_code: 'NO_DATA', + }, + })); + } + const promises = []; for (let i = 0; i < accounts.length; i++) { const account = accounts[i]; const download = res[account.account_id]; + if (!download || Object.keys(download).length === 0) { + 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; @@ -1094,13 +1135,32 @@ export async function simpleFinBatchSync( continue; } + if (!download.transactions) { + promises.push( + Promise.resolve({ + accountId: account.id, + res: { + error_type: 'ACCOUNT_MISSING', + error_code: 'ACCOUNT_MISSING', + }, + }), + ); + continue; + } + promises.push( - processBankSyncDownload(download, account.id, acctRow, newAccount).then( - res => ({ + processBankSyncDownload(download, account.id, acctRow, newAccount) + .then(res => ({ accountId: account.id, res, - }), - ), + })) + .catch(err => ({ + accountId: account.id, + res: { + error_type: err?.category || 'INTERNAL_ERROR', + error_code: err?.code || 'INTERNAL_ERROR', + }, + })), ); } diff --git a/upcoming-release-notes/7152.md b/upcoming-release-notes/7152.md new file mode 100644 index 0000000000..2d87299772 --- /dev/null +++ b/upcoming-release-notes/7152.md @@ -0,0 +1,6 @@ +--- +category: Bugfixes +authors: [MatissJanis] +--- + +Handle missing accounts in SimpleFin batch sync with localized error messaging for users.