mirror of
https://github.com/actualbudget/actual.git
synced 2026-03-11 12:43:09 -05:00
[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
This commit is contained in:
@@ -68,6 +68,11 @@ function useErrorMessage() {
|
||||
</Trans>
|
||||
);
|
||||
|
||||
case 'ACCOUNT_MISSING':
|
||||
return t(
|
||||
'This account was not found in SimpleFIN. Try unlinking and relinking the account.',
|
||||
);
|
||||
|
||||
default:
|
||||
}
|
||||
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user