fix(enable-banking): recover error-state connections, respect PSD2 balance quota, clean error surface (#968)
* fix(enable-banking): recover error-state connections, respect PSD2 balance quota, clean error surface Three defects from the 2026-07-09 production log triage, all in how the enable-banking extension handles upstream (Enable Banking / ASPSP) failures: 1. Retry dead-end: a non-session sync failure parked the connection in status='error', but POST /sync rejected anything not 'active' with 400, so the UI's "Försök igen" button could never succeed and the connection stayed stranded until a full re-auth. /sync now accepts 'error' (while still rejecting 'expired': a dead consent needs re-authorization), and a successful sync restores status='active' and clears error_message. 2. Balance quota burn: every sync (manual or cron) called the BALANCES endpoint although PSD2 unattended consents allow only 4 calls/day (observed 429 "Consent daily limit 4 is exceeded"), and the retry wrapper retried those 429s twice against a daily quota. The sync now skips the balance call while the stored balance_updated_at is fresher than 12 hours, and authenticatedFetchWithRetry fails fast on a 429 whose body signals a daily limit. 3. Raw JSON in UI: sync failures persisted the raw English Enable Banking error body into bank_connections.error_message, which the settings panel renders verbatim. Failures are now mapped to short Swedish user messages (shared constants in api-client.ts); the raw body stays in server logs only. Also ratchets the eslint baseline down by 1: the no-explicit-any disable in the cron route was on the wrong line and never suppressed anything. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(enable-banking): treat future balance timestamps as stale (CodeRabbit) A future balance_updated_at yielded a negative age that always passed the freshness check, suppressing balance refreshes indefinitely; only 0 <= age < BALANCE_MAX_AGE_MS now counts as fresh. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Fable 5
Claude Fable 5
parent
6a9adcf00a
commit
b06d73c23e
@@ -86,6 +86,45 @@ describe('api-client', () => {
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('does not retry a 429 whose body signals a daily quota', async () => {
|
||||
// PSD2 unattended consents cap balance calls per DAY (observed body:
|
||||
// "Consent daily limit 4 is exceeded"). A retry a second later cannot
|
||||
// succeed against a daily quota, so it must fail fast.
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
new Response('{"message":"Consent daily limit 4 is exceeded"}', { status: 429 })
|
||||
)
|
||||
|
||||
await expect(getAccountBalances('acc-1')).rejects.toThrow(
|
||||
'Failed to get account balances (429)'
|
||||
)
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1)
|
||||
|
||||
warnSpy.mockRestore()
|
||||
errorSpy.mockRestore()
|
||||
})
|
||||
|
||||
it('still retries a 429 without a daily-limit body (transient rate limit)', async () => {
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
|
||||
fetchSpy
|
||||
.mockResolvedValueOnce(new Response('Too Many Requests', { status: 429 }))
|
||||
.mockResolvedValueOnce(
|
||||
new Response(JSON.stringify({ balances: [] }), {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
})
|
||||
)
|
||||
|
||||
const result = await getAccountBalances('acc-1')
|
||||
expect(result).toEqual([])
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2)
|
||||
|
||||
warnSpy.mockRestore()
|
||||
})
|
||||
|
||||
it('does not retry on 400 errors', async () => {
|
||||
const badRequest = new Response('Bad Request', { status: 400 })
|
||||
fetchSpy.mockResolvedValueOnce(badRequest)
|
||||
|
||||
@@ -506,6 +506,76 @@ describe('syncAccountTransactions', () => {
|
||||
expect(ingestOptions).toMatchObject({ settlementAccount: '1932' })
|
||||
})
|
||||
|
||||
it('skips the balance call when the stored balance is fresher than 12 hours', async () => {
|
||||
// PSD2 unattended consents allow only 4 BALANCES calls per account per
|
||||
// day; a fresh stored balance must not burn the quota on every manual sync.
|
||||
mockGetAllTransactionsWithRaw.mockResolvedValue({ transactions: [], rawPages: [] })
|
||||
|
||||
const freshAt = new Date(Date.now() - 1 * 60 * 60 * 1000).toISOString()
|
||||
const account = makeAccount({ balance: 500, balance_updated_at: freshAt })
|
||||
|
||||
await syncAccountTransactions(
|
||||
{} as never, COMPANY_ID, USER_ID, CONNECTION_ID, account,
|
||||
'2026-01-01', '2026-06-01', mockIngest
|
||||
)
|
||||
|
||||
expect(mockGetAccountBalance).not.toHaveBeenCalled()
|
||||
expect(account.balance).toBe(500)
|
||||
expect(account.balance_updated_at).toBe(freshAt)
|
||||
})
|
||||
|
||||
it('refreshes the balance when the stored balance is older than 12 hours', async () => {
|
||||
mockGetAllTransactionsWithRaw.mockResolvedValue({ transactions: [], rawPages: [] })
|
||||
mockGetAccountBalance.mockResolvedValue({ amount: 1234.56, date: '2026-06-01' })
|
||||
|
||||
const staleAt = new Date(Date.now() - 13 * 60 * 60 * 1000).toISOString()
|
||||
const account = makeAccount({ balance: 500, balance_updated_at: staleAt })
|
||||
|
||||
await syncAccountTransactions(
|
||||
{} as never, COMPANY_ID, USER_ID, CONNECTION_ID, account,
|
||||
'2026-01-01', '2026-06-01', mockIngest
|
||||
)
|
||||
|
||||
expect(mockGetAccountBalance).toHaveBeenCalledWith('acc-uid-1')
|
||||
expect(account.balance).toBe(1234.56)
|
||||
expect(account.balance_updated_at).not.toBe(staleAt)
|
||||
})
|
||||
|
||||
it('treats a future balance_updated_at as stale and refreshes', async () => {
|
||||
// Clock skew or bad data can store a future timestamp; its negative age
|
||||
// must not count as fresh, or refreshes would be suppressed indefinitely.
|
||||
mockGetAllTransactionsWithRaw.mockResolvedValue({ transactions: [], rawPages: [] })
|
||||
mockGetAccountBalance.mockResolvedValue({ amount: 1234.56, date: '2026-06-01' })
|
||||
|
||||
const futureAt = new Date(Date.now() + 60 * 60 * 1000).toISOString()
|
||||
const account = makeAccount({ balance: 500, balance_updated_at: futureAt })
|
||||
|
||||
await syncAccountTransactions(
|
||||
{} as never, COMPANY_ID, USER_ID, CONNECTION_ID, account,
|
||||
'2026-01-01', '2026-06-01', mockIngest
|
||||
)
|
||||
|
||||
expect(mockGetAccountBalance).toHaveBeenCalledWith('acc-uid-1')
|
||||
expect(account.balance).toBe(1234.56)
|
||||
expect(account.balance_updated_at).not.toBe(futureAt)
|
||||
})
|
||||
|
||||
it('attempts a balance refresh when no timestamp is stored', async () => {
|
||||
mockGetAllTransactionsWithRaw.mockResolvedValue({ transactions: [], rawPages: [] })
|
||||
|
||||
const account = makeAccount() // no balance_updated_at
|
||||
|
||||
await syncAccountTransactions(
|
||||
{} as never, COMPANY_ID, USER_ID, CONNECTION_ID, account,
|
||||
'2026-01-01', '2026-06-01', mockIngest
|
||||
)
|
||||
|
||||
// The beforeEach default rejects the call: the sync must survive that and
|
||||
// leave the timestamp unset (so the next sync tries again).
|
||||
expect(mockGetAccountBalance).toHaveBeenCalledTimes(1)
|
||||
expect(account.balance_updated_at).toBeUndefined()
|
||||
})
|
||||
|
||||
it('omits settlementAccount when account.ledger_account is unset (mapping engine defaults to 1930)', async () => {
|
||||
mockGetAllTransactionsWithRaw.mockResolvedValue({
|
||||
transactions: [{ transaction_amount: { amount: '100', currency: 'SEK' }, booking_date: '2026-04-01' }],
|
||||
|
||||
@@ -223,6 +223,18 @@ export function isSessionExpiredResponse(status: number, body: string): boolean
|
||||
return SESSION_DEAD_NEEDLES.some(needle => normalized.includes(needle))
|
||||
}
|
||||
|
||||
/**
|
||||
* User-facing (Swedish) messages persisted to bank_connections.error_message
|
||||
* and returned to the settings UI. error_message is a literal string in the
|
||||
* DB, not an i18n key, matching the extension's other user-facing strings.
|
||||
* Raw Enable Banking error bodies are English JSON envelopes and must never
|
||||
* land here: they belong in server logs only.
|
||||
*/
|
||||
export const REAUTH_REQUIRED_MESSAGE =
|
||||
'Bankanslutningen har löpt ut. Förnya anslutningen för att fortsätta synka.'
|
||||
export const SYNC_FAILED_MESSAGE =
|
||||
'Banksynkningen misslyckades. Försök igen, eller förnya anslutningen om felet kvarstår.'
|
||||
|
||||
/**
|
||||
* Thrown when a transactions fetch fails because the PSD2 session is dead
|
||||
* (closed/expired/invalid). Distinct from TransactionsFetchError so the sync
|
||||
@@ -279,6 +291,21 @@ async function authenticatedFetchWithRetry(
|
||||
try {
|
||||
const response = await authenticatedFetch(endpoint, options)
|
||||
if (attempt < MAX_RETRIES && [429, 502, 503, 504].includes(response.status)) {
|
||||
// A 429 caused by a DAILY quota cannot clear within the retry window:
|
||||
// PSD2 unattended consents allow only a handful of balance calls per
|
||||
// day (observed body: "Consent daily limit 4 is exceeded"), so
|
||||
// retrying just burns time and duplicates the failure in logs. Read
|
||||
// the body from a clone so the returned response stays consumable.
|
||||
if (response.status === 429) {
|
||||
const body = await response.clone().text().catch(() => '')
|
||||
if (/daily limit/i.test(body)) {
|
||||
console.warn(`[enable-banking] 429 daily quota exhausted for ${endpoint}: not retrying`, {
|
||||
status: response.status,
|
||||
body,
|
||||
})
|
||||
return response
|
||||
}
|
||||
}
|
||||
console.warn(`[enable-banking] Retrying ${endpoint} (attempt ${attempt + 1}/${MAX_RETRIES})`, {
|
||||
status: response.status,
|
||||
statusText: response.statusText,
|
||||
|
||||
@@ -27,6 +27,15 @@ export interface SyncOptions {
|
||||
strategy?: TransactionsFetchStrategy
|
||||
}
|
||||
|
||||
/**
|
||||
* How long a stored account balance stays fresh before a sync refreshes it.
|
||||
* PSD2 unattended consents allow only 4 balance calls per account per day
|
||||
* (observed: 429 "Consent daily limit 4 is exceeded"), while transaction
|
||||
* fetches are budgeted separately. 12h keeps at most 2 balance calls per day
|
||||
* regardless of how many manual "Synka nu" clicks or cron runs happen.
|
||||
*/
|
||||
const BALANCE_MAX_AGE_MS = 12 * 60 * 60 * 1000
|
||||
|
||||
export interface SyncResult {
|
||||
imported: number
|
||||
duplicates: number
|
||||
@@ -209,13 +218,32 @@ export async function syncAccountTransactions(
|
||||
}
|
||||
}
|
||||
|
||||
// Update account balance
|
||||
try {
|
||||
const balance = await getAccountBalance(account.uid)
|
||||
account.balance = balance.amount
|
||||
account.balance_updated_at = new Date().toISOString()
|
||||
} catch {
|
||||
// Keep previous balance, don't update timestamp
|
||||
// Update account balance, but only when the stored one has gone stale:
|
||||
// every skipped call preserves the account's scarce daily BALANCES quota
|
||||
// (see BALANCE_MAX_AGE_MS). balance_updated_at is written ONLY on a
|
||||
// successful refresh below, so a stale/missing/invalid timestamp always
|
||||
// falls through to a refresh attempt (NaN and Infinity both fail the
|
||||
// freshness comparison). A FUTURE timestamp (clock skew, bad data) yields a
|
||||
// negative age; treat it as stale too, or refreshes would be suppressed
|
||||
// until the wall clock catches up.
|
||||
const balanceAgeMs = account.balance_updated_at
|
||||
? Date.now() - new Date(account.balance_updated_at).getTime()
|
||||
: Number.POSITIVE_INFINITY
|
||||
const balanceIsFresh = balanceAgeMs >= 0 && balanceAgeMs < BALANCE_MAX_AGE_MS
|
||||
if (balanceIsFresh) {
|
||||
console.log('[enable-banking] Skipping balance refresh (stored balance is fresh)', {
|
||||
connectionId,
|
||||
accountUid: account.uid,
|
||||
balanceUpdatedAt: account.balance_updated_at,
|
||||
})
|
||||
} else {
|
||||
try {
|
||||
const balance = await getAccountBalance(account.uid)
|
||||
account.balance = balance.amount
|
||||
account.balance_updated_at = new Date().toISOString()
|
||||
} catch {
|
||||
// Keep previous balance, don't update timestamp
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user