diff --git a/app/api/extensions/enable-banking/sync/cron/route.ts b/app/api/extensions/enable-banking/sync/cron/route.ts index 842ef23c..c3cd9509 100644 --- a/app/api/extensions/enable-banking/sync/cron/route.ts +++ b/app/api/extensions/enable-banking/sync/cron/route.ts @@ -5,7 +5,13 @@ import { runReconciliation, DEFAULT_UNATTENDED_CONFIDENCE_THRESHOLD, } from '@/lib/reconciliation/bank-reconciliation' -import { isConsentExpiringSoon, getDaysUntilExpiry, SessionExpiredError } from '@/extensions/general/enable-banking/lib/api-client' +import { + isConsentExpiringSoon, + getDaysUntilExpiry, + SessionExpiredError, + REAUTH_REQUIRED_MESSAGE, + SYNC_FAILED_MESSAGE, +} from '@/extensions/general/enable-banking/lib/api-client' import { getEmailService } from '@/lib/email/service' import { generateConsentExpiryEmailHtml, @@ -278,7 +284,6 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { daysUntilExpiry: daysLeft, }) } catch (error) { - const message = error instanceof Error ? error.message : 'Unknown error' ctx.log.error('sync failed for connection', error as Error, { connectionId: connection.id, userId: connection.user_id, @@ -291,11 +296,13 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { // condition, not a transient failure: flip it to 'expired' (same state // the consent-elapsed branch uses) so the UI offers a reconnect instead // of a retry. Other errors stay 'error'. + // + // error_message is rendered verbatim on the settings panel, so it gets + // the short Swedish user message in both cases: the raw Enable Banking + // error body (an English JSON envelope) stays in the server log above. const isSessionDead = error instanceof SessionExpiredError const failureStatus = isSessionDead ? 'expired' : 'error' - const failureMessage = isSessionDead - ? 'Bankanslutningen har löpt ut. Förnya anslutningen för att fortsätta synka.' - : message + const failureMessage = isSessionDead ? REAUTH_REQUIRED_MESSAGE : SYNC_FAILED_MESSAGE await supabase .from('bank_connections') @@ -341,8 +348,8 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { * Send consent expiry notification email. * Guards with last_expiry_notification_at to avoid spamming (2-day cooldown). */ -// eslint-disable-next-line @typescript-eslint/no-explicit-any async function sendConsentExpiryNotification( + // eslint-disable-next-line @typescript-eslint/no-explicit-any supabase: SupabaseClient, connection: Record, daysLeft: number, diff --git a/extensions/general/enable-banking/__tests__/error-status-retry.test.ts b/extensions/general/enable-banking/__tests__/error-status-retry.test.ts new file mode 100644 index 00000000..4b410d00 --- /dev/null +++ b/extensions/general/enable-banking/__tests__/error-status-retry.test.ts @@ -0,0 +1,183 @@ +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest' +import type { ExtensionContext } from '@/lib/extensions/types' +import type { StoredAccount } from '../types' + +// Mock the JWT signer so api-client can be imported without real +// ENABLE_BANKING credentials. +vi.mock('../lib/jwt', () => ({ + getAuthorizationHeader: () => 'Bearer test-token', +})) + +// Mock the sync orchestrator so the /sync handler tests can force success or +// failure without hitting the network. +vi.mock('../lib/sync', () => ({ + syncAccountTransactions: vi.fn(), +})) + +vi.mock('@/lib/entitlements/has-capability', () => ({ + requireCapability: vi.fn().mockResolvedValue(null), +})) + +import { SYNC_FAILED_MESSAGE } from '../lib/api-client' +import { enableBankingExtension } from '../index' +import { syncAccountTransactions } from '../lib/sync' + +const syncRoute = enableBankingExtension.apiRoutes?.find( + r => r.method === 'POST' && r.path === '/sync' +) + +if (!syncRoute) { + throw new Error('POST /sync route not registered on enable-banking extension') +} + +const RAW_EB_ERROR = + 'Failed to get transactions (400): {"code":400,"message":"Error interacting with ASPSP","detail":null,"error":"ASPSP_ERROR"}' + +function makeContext(connection: Record, updateSpy: Mock): ExtensionContext { + // One universal chainable per from() call (same shape as the other /sync + // handler tests): bank_connections terminates on single(), sie_imports / + // company_members on maybeSingle(), and update() records its payload while + // returning the chain so trailing .eq() calls resolve. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const chain: any = {} + chain.select = vi.fn(() => chain) + chain.eq = vi.fn(() => chain) + chain.gte = vi.fn(() => chain) + chain.limit = vi.fn(() => chain) + chain.order = vi.fn(() => chain) + chain.single = vi.fn().mockResolvedValue({ data: connection, error: null }) + chain.maybeSingle = vi.fn().mockResolvedValue({ data: null, error: null }) + chain.update = vi.fn((payload: unknown) => { + updateSpy(payload) + return chain + }) + + const supabase = { + auth: { + getUser: vi.fn().mockResolvedValue({ data: { user: { id: 'user-1' } }, error: null }), + }, + from: vi.fn(() => chain), + } + + return { + userId: 'user-1', + companyId: 'company-1', + extensionId: 'enable-banking', + requestId: 'req_test', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + supabase: supabase as any, + emit: vi.fn().mockResolvedValue(undefined), + settings: { get: vi.fn(), set: vi.fn(), getAll: vi.fn() } as never, + storage: {} as never, + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as never, + services: {} as never, + } +} + +function makeRequest(): Request { + return new Request('http://localhost/api/extensions/ext/enable-banking/sync', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ connection_id: 'conn-1' }), + }) +} + +function makeConnection(overrides: Record = {}): Record { + return { + id: 'conn-1', + company_id: 'company-1', + bank_name: 'SEB', + accounts_data: [{ uid: 'acc-1', currency: 'SEK', enabled: true }] as StoredAccount[], + ...overrides, + } +} + +describe('POST /sync (enable-banking): retry from error status', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('allows sync from status=error and restores active + clears error_message on success', async () => { + // Regression: a transient ASPSP failure parked the connection in 'error', + // but the old status gate rejected everything but 'active', so the UI's + // "Försök igen" button always got 400 and the connection was stranded + // until a full re-auth. + ;(syncAccountTransactions as unknown as Mock).mockResolvedValue({ + imported: 0, + duplicates: 0, + errors: 0, + }) + + const updateSpy = vi.fn() + const ctx = makeContext( + makeConnection({ status: 'error', error_message: RAW_EB_ERROR }), + updateSpy + ) + + const res = await syncRoute.handler(makeRequest(), ctx) + + expect(res.status).toBe(200) + expect(syncAccountTransactions).toHaveBeenCalledTimes(1) + expect(updateSpy).toHaveBeenCalledWith( + expect.objectContaining({ + status: 'active', + error_message: null, + last_synced_at: expect.any(String), + }) + ) + }) + + it('still rejects expired connections with 400 (re-auth required, not retry)', async () => { + const updateSpy = vi.fn() + const ctx = makeContext(makeConnection({ status: 'expired' }), updateSpy) + + const res = await syncRoute.handler(makeRequest(), ctx) + + expect(res.status).toBe(400) + const body = await res.json() + expect(body.error).toMatch(/not active/i) + expect(syncAccountTransactions).not.toHaveBeenCalled() + expect(updateSpy).not.toHaveBeenCalled() + }) + + it('does not touch status for an active connection but clears a leftover error_message', async () => { + ;(syncAccountTransactions as unknown as Mock).mockResolvedValue({ + imported: 0, + duplicates: 0, + errors: 0, + }) + + const updateSpy = vi.fn() + const ctx = makeContext( + makeConnection({ status: 'active', error_message: RAW_EB_ERROR }), + updateSpy + ) + + const res = await syncRoute.handler(makeRequest(), ctx) + + expect(res.status).toBe(200) + const payload = updateSpy.mock.calls[0][0] + expect(payload).not.toHaveProperty('status') + expect(payload).toMatchObject({ error_message: null }) + }) + + it('maps a non-session failure to the Swedish user message and refreshes the stored error_message', async () => { + // The raw Enable Banking body is an English JSON envelope: it belongs in + // server logs, never in the toast or the settings panel. + ;(syncAccountTransactions as unknown as Mock).mockRejectedValue(new Error(RAW_EB_ERROR)) + + const updateSpy = vi.fn() + const ctx = makeContext( + makeConnection({ status: 'error', error_message: RAW_EB_ERROR }), + updateSpy + ) + + const res = await syncRoute.handler(makeRequest(), ctx) + + expect(res.status).toBe(500) + const body = await res.json() + expect(body.error).toBe(SYNC_FAILED_MESSAGE) + expect(body.error).not.toContain('ASPSP_ERROR') + expect(updateSpy).toHaveBeenCalledWith({ error_message: SYNC_FAILED_MESSAGE }) + }) +}) diff --git a/extensions/general/enable-banking/index.ts b/extensions/general/enable-banking/index.ts index e897dde1..af379e41 100644 --- a/extensions/general/enable-banking/index.ts +++ b/extensions/general/enable-banking/index.ts @@ -7,6 +7,8 @@ import { deleteSession, isSandboxMode, SessionExpiredError, + REAUTH_REQUIRED_MESSAGE, + SYNC_FAILED_MESSAGE, type ASPSP, } from './lib/api-client' import { syncAccountTransactions } from './lib/sync' @@ -443,7 +445,12 @@ export const enableBankingExtension: Extension = { return NextResponse.json({ error: 'Connection not found' }, { status: 404 }) } - if (connection.status !== 'active') { + // 'error' is retryable: a transient upstream failure (e.g. ASPSP_ERROR) + // parks the connection in 'error' while the PSD2 session is still + // alive, so the UI's "Försök igen" must be allowed through; a + // successful sync below restores 'active'. 'expired' stays rejected: + // a dead consent needs re-authorization via /connect, not a retry. + if (connection.status !== 'active' && connection.status !== 'error') { return NextResponse.json({ error: 'Connection is not active' }, { status: 400 }) } @@ -556,6 +563,13 @@ export const enableBankingExtension: Extension = { .update({ accounts_data: allAccounts, last_synced_at: syncedAt, + // A successful sync proves the session works again: recover an + // 'error' connection to 'active' (so the cron picks it up again) + // and clear any stale failure message from the settings panel. + ...(connection.status === 'error' ? { status: 'active' } : {}), + ...(connection.status === 'error' || connection.error_message + ? { error_message: null } + : {}), }) .eq('id', connection.id) @@ -601,15 +615,14 @@ export const enableBankingExtension: Extension = { // one-click "Förnya anslutning" instead of a dead-end error. No // disconnect needed: /connect reconnects this same connection in place. if (error instanceof SessionExpiredError) { - const reauthMessage = 'Bankanslutningen har löpt ut. Förnya anslutningen för att fortsätta synka.' await supabase .from('bank_connections') - .update({ status: 'expired', error_message: reauthMessage }) + .update({ status: 'expired', error_message: REAUTH_REQUIRED_MESSAGE }) .eq('id', connection.id) .eq('company_id', companyId) return NextResponse.json( { - error: reauthMessage, + error: REAUTH_REQUIRED_MESSAGE, code: 'SESSION_EXPIRED', reauth_required: true, connection_id: connection.id, @@ -618,10 +631,21 @@ export const enableBankingExtension: Extension = { ) } - return NextResponse.json( - { error: error instanceof Error ? error.message : 'Sync failed' }, - { status: 500 } - ) + // Non-session failure: the settings panel toasts this error verbatim + // and renders error_message on the connection card, so both must be + // the short Swedish message: the raw Enable Banking body (an English + // JSON envelope) is already in the server log above. Refresh the + // stored error_message on rows already in 'error' so a failed retry + // replaces any stale raw body persisted by older code. + if (connection.status === 'error') { + await supabase + .from('bank_connections') + .update({ error_message: SYNC_FAILED_MESSAGE }) + .eq('id', connection.id) + .eq('company_id', companyId) + } + + return NextResponse.json({ error: SYNC_FAILED_MESSAGE }, { status: 500 }) } }, }, diff --git a/extensions/general/enable-banking/lib/__tests__/api-client.test.ts b/extensions/general/enable-banking/lib/__tests__/api-client.test.ts index 980c4a97..6f5a7e61 100644 --- a/extensions/general/enable-banking/lib/__tests__/api-client.test.ts +++ b/extensions/general/enable-banking/lib/__tests__/api-client.test.ts @@ -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) diff --git a/extensions/general/enable-banking/lib/__tests__/sync.test.ts b/extensions/general/enable-banking/lib/__tests__/sync.test.ts index 9e3d30a0..f3404660 100644 --- a/extensions/general/enable-banking/lib/__tests__/sync.test.ts +++ b/extensions/general/enable-banking/lib/__tests__/sync.test.ts @@ -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' }], diff --git a/extensions/general/enable-banking/lib/api-client.ts b/extensions/general/enable-banking/lib/api-client.ts index bfc3f3fb..fff57a37 100644 --- a/extensions/general/enable-banking/lib/api-client.ts +++ b/extensions/general/enable-banking/lib/api-client.ts @@ -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, diff --git a/extensions/general/enable-banking/lib/sync.ts b/extensions/general/enable-banking/lib/sync.ts index 999df93e..1514baa4 100644 --- a/extensions/general/enable-banking/lib/sync.ts +++ b/extensions/general/enable-banking/lib/sync.ts @@ -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 { diff --git a/scripts/checks/eslint-baseline.json b/scripts/checks/eslint-baseline.json index e4a938ab..fa9a6bd7 100644 --- a/scripts/checks/eslint-baseline.json +++ b/scripts/checks/eslint-baseline.json @@ -1,8 +1,8 @@ { - "totalErrors": 60, + "totalErrors": 59, "perRule": { "@next/next/no-assign-module-variable": 1, - "@typescript-eslint/no-explicit-any": 15, + "@typescript-eslint/no-explicit-any": 14, "prefer-const": 3, "react-hooks/preserve-manual-memoization": 6, "react-hooks/purity": 1,