From 88f53350de7c90d089ec67e40d23907087233d10 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:51:07 +0200 Subject: [PATCH] fix(errors): translate typed engine errors instead of leaking raw messages as journal_entry_error (#1048) Typed bookkeeping Error instances passed to getErrorMessage() matched the bare-envelope branch (any object with string code + message) and returned their raw English message verbatim, so the categorize and match-invoice routes surfaced strings like DB check-constraint violations directly in the user's toast (issue #337). - get-error-message.ts: when the bare-envelope shape is an Error instance, normalize it into the structured envelope ({ error: { code, message, account_numbers, details } }) so the existing per-code Swedish branches own the translation; plain forwarded envelopes keep the passthrough. - get-error-message.ts: structured-path final fallback now prefers the registry's message_sv for known codes whose message is not Swedish, so typed codes without a dynamic branch (e.g. CANNOT_REVERSE_STORNO) cannot surface English either. - categorize + match-invoice routes: always map the caught error through getErrorMessage (the raw error is already logged); untyped errors fall to the Swedish context fallback instead of leaking err.message. - Tests: new instance-translation suite in lib/errors, typed-error case in the categorize route suite, and deliberate updates of the two tests that pinned raw 'Period locked' passthrough. Fixes #337 Co-authored-by: Claude Fable 5 --- .../[id]/categorize/__tests__/route.test.ts | 43 +++++++++++++- app/api/transactions/[id]/categorize/route.ts | 15 ++--- .../match-invoice/__tests__/route.test.ts | 4 +- .../transactions/[id]/match-invoice/route.ts | 11 ++-- .../__tests__/get-error-message.test.ts | 57 +++++++++++++++++++ lib/errors/get-error-message.ts | 29 ++++++++++ 6 files changed, 141 insertions(+), 18 deletions(-) diff --git a/app/api/transactions/[id]/categorize/__tests__/route.test.ts b/app/api/transactions/[id]/categorize/__tests__/route.test.ts index a76ef37d..f4c8eeae 100644 --- a/app/api/transactions/[id]/categorize/__tests__/route.test.ts +++ b/app/api/transactions/[id]/categorize/__tests__/route.test.ts @@ -7,6 +7,7 @@ import { makeTransaction, } from '@/tests/helpers' import { eventBus } from '@/lib/events' +import { JournalEntryNotBalancedError } from '@/lib/bookkeeping/errors' const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase() vi.mock('@/lib/supabase/server', () => ({ @@ -325,7 +326,47 @@ describe('POST /api/transactions/[id]/categorize', () => { expect(status).toBe(200) expect(body.success).toBe(true) expect(body.journal_entry_created).toBe(false) - expect(body.journal_entry_error).toBe('Period locked') + // Untyped errors no longer leak their raw English message (issue #337): + // they map to the Swedish transaction-context fallback. + expect(body.journal_entry_error).toBe('Kunde inte hantera transaktionen. Försök igen.') + }) + + it('translates typed engine errors to Swedish in journal_entry_error (issue #337)', async () => { + const tx = makeTransaction({ + id: 'tx-1', + amount: -500, + merchant_name: 'Test', + journal_entry_id: null, + }) + + enqueue({ data: tx, error: null }) + enqueue({ data: { entity_type: 'enskild_firma', fiscal_year_start_month: 1 }, error: null }) + enqueue({ data: [{ id: 'period-1' }], error: null }) + + mockCreateTransactionJournalEntry.mockRejectedValue(new JournalEntryNotBalancedError(100, 80)) + + // Update transaction + enqueue({ data: null, error: null }) + + const request = createMockRequest('/api/transactions/tx-1/categorize', { + method: 'POST', + body: { is_business: true, category: 'expense_software' }, + }) + const response = await POST(request, createMockRouteParams({ id: 'tx-1' })) + const { status, body } = await parseJsonResponse<{ + success: boolean + journal_entry_created: boolean + journal_entry_error: string + }>(response) + + expect(status).toBe(200) + expect(body.success).toBe(true) + expect(body.journal_entry_created).toBe(false) + expect(body.journal_entry_error).toContain('balanserar inte') + expect(body.journal_entry_error).toMatch(/100/) + expect(body.journal_entry_error).toMatch(/80/) + expect(body.journal_entry_error).not.toContain('not balanced') + expect(body.journal_entry_error).not.toContain('check constraint') }) it('returns 500 when transaction update fails', async () => { diff --git a/app/api/transactions/[id]/categorize/route.ts b/app/api/transactions/[id]/categorize/route.ts index c2626bc6..ee9649e2 100644 --- a/app/api/transactions/[id]/categorize/route.ts +++ b/app/api/transactions/[id]/categorize/route.ts @@ -18,7 +18,7 @@ import { escapeLikePattern, normalizeOcrReference, } from '@/lib/invoices/duplicate-payment-guard' -import { AccountsNotInChartError, accountsNotInChartResponse, isBookkeepingError } from '@/lib/bookkeeping/errors' +import { AccountsNotInChartError, accountsNotInChartResponse } from '@/lib/bookkeeping/errors' import { collectMappingResultAccounts, findUnresolvableAccounts } from '@/lib/bookkeeping/account-validation' import { getErrorMessage } from '@/lib/errors/get-error-message' import type { Logger } from '@/lib/logger' @@ -640,14 +640,11 @@ export const POST = withRouteContext( if (err instanceof AccountsNotInChartError) { return accountsNotInChartResponse(err) } - // Bookkeeping errors map to Swedish via the registry. Other errors get - // their raw message: the categorization is preserved either way so the - // user can still re-book the verifikation manually. - if (isBookkeepingError(err)) { - journalEntryError = getErrorMessage(err, { context: 'transaction' }) - } else { - journalEntryError = err instanceof Error ? err.message : 'Unknown error' - } + // All errors map to Swedish via getErrorMessage: the raw message is + // already logged above and must never reach the user verbatim (issue + // #337). The categorization is preserved either way so the user can + // still re-book the verifikation manually. + journalEntryError = getErrorMessage(err, { context: 'transaction' }) } // direction_mismatch = a mirrored refund/repayment booking; learning it diff --git a/app/api/transactions/[id]/match-invoice/__tests__/route.test.ts b/app/api/transactions/[id]/match-invoice/__tests__/route.test.ts index 0925068d..d3d41881 100644 --- a/app/api/transactions/[id]/match-invoice/__tests__/route.test.ts +++ b/app/api/transactions/[id]/match-invoice/__tests__/route.test.ts @@ -1048,7 +1048,9 @@ describe('POST /api/transactions/[id]/match-invoice', () => { expect(status).toBe(200) expect(body.success).toBe(true) expect(body.journal_entry_id).toBeNull() - expect(body.journal_entry_error).toBe('Period locked') + // Untyped errors no longer leak their raw English message (issue #337): + // they map to the Swedish invoice-context fallback. + expect(body.journal_entry_error).toBe('Kunde inte hantera fakturan. Försök igen.') }) // ──────────────────────────────────────────────────────────────── diff --git a/app/api/transactions/[id]/match-invoice/route.ts b/app/api/transactions/[id]/match-invoice/route.ts index 8e197bcf..0798661b 100644 --- a/app/api/transactions/[id]/match-invoice/route.ts +++ b/app/api/transactions/[id]/match-invoice/route.ts @@ -4,7 +4,7 @@ import { buildInvoicePaymentClearingLines } from '@/lib/bookkeeping/invoice-paym import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' import { fetchExchangeRate } from '@/lib/currency/riksbanken' import { reverseEntry, createJournalEntry, findFiscalPeriod } from '@/lib/bookkeeping/engine' -import { AccountsNotInChartError, isBookkeepingError } from '@/lib/bookkeeping/errors' +import { AccountsNotInChartError } from '@/lib/bookkeeping/errors' import { getErrorMessage } from '@/lib/errors/get-error-message' import { withRouteContext } from '@/lib/api/with-route-context' import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error' @@ -480,12 +480,9 @@ export const POST = withRouteContext( } txLog.error('failed to create payment journal entry', err as Error) // Other errors are recorded but don't abort the match: the user can - // re-book the verifikation manually. - if (isBookkeepingError(err)) { - journalEntryError = getErrorMessage(err, { context: 'invoice' }) - } else { - journalEntryError = err instanceof Error ? err.message : 'Unknown error' - } + // re-book the verifikation manually. All errors map to Swedish via + // getErrorMessage; the raw message must never reach the user (issue #337). + journalEntryError = getErrorMessage(err, { context: 'invoice' }) } // Underlag for the payment verifikation: re-attach the invoice PDF that diff --git a/lib/errors/__tests__/get-error-message.test.ts b/lib/errors/__tests__/get-error-message.test.ts index 639111d0..0054ffe4 100644 --- a/lib/errors/__tests__/get-error-message.test.ts +++ b/lib/errors/__tests__/get-error-message.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect } from 'vitest' import { getErrorMessage } from '../get-error-message' +import { + AccountsNotInChartError, + BookkeepingDatabaseError, + CannotReverseStornoError, + JournalEntryNotBalancedError, +} from '@/lib/bookkeeping/errors' describe('getErrorMessage: typed bookkeeping error codes', () => { it('ACCOUNTS_NOT_IN_CHART → lists accounts to activate', () => { @@ -95,6 +101,57 @@ describe('getErrorMessage: typed bookkeeping error codes', () => { }) }) +describe('getErrorMessage: typed bookkeeping Error instances (issue #337)', () => { + it('JournalEntryNotBalancedError instance → rich Swedish amount message', () => { + const msg = getErrorMessage(new JournalEntryNotBalancedError(100, 80), { context: 'transaction' }) + expect(msg).toContain('balanserar inte') + expect(msg).toMatch(/100/) + expect(msg).toMatch(/80/) + expect(msg).not.toContain('Journal entry is not balanced') + }) + + it('BookkeepingDatabaseError instance → Swedish, never the raw constraint string', () => { + const msg = getErrorMessage( + new BookkeepingDatabaseError( + 'commit_entry', + 'new row for relation "journal_entries" violates check constraint "check_balanced"', + ), + { context: 'transaction' }, + ) + expect(msg).toBe('Verifikationen kunde inte sparas. Försök igen.') + expect(msg).not.toContain('check constraint') + expect(msg).not.toContain('Database operation') + }) + + it('BookkeepingDatabaseError instance wrapping a period-lock trigger → specific Swedish message', () => { + const msg = getErrorMessage( + new BookkeepingDatabaseError('commit_entry', 'Cannot create entry in locked/closed fiscal period'), + ) + expect(msg).toBe('Perioden är låst. Verifikationen kan inte skapas i en stängd eller låst period.') + }) + + it('AccountsNotInChartError instance → Swedish account-activation message', () => { + const msg = getErrorMessage(new AccountsNotInChartError(['1930'])) + expect(msg).toBe('Följande konton behöver aktiveras: 1930') + }) + + it('CannotReverseStornoError instance → registry Swedish message (no dynamic branch)', () => { + const msg = getErrorMessage(new CannotReverseStornoError('reversal')) + expect(msg).toBe('En stornering eller rättelse kan inte stornas.') + expect(msg).not.toContain('Cannot reverse') + }) + + it('locale "en" on a typed instance → registry English message', () => { + const msg = getErrorMessage(new CannotReverseStornoError('reversal'), { locale: 'en' }) + expect(msg).toBe('A storno or correction entry cannot be reversed.') + }) + + it('regression: plain-object bare envelope with a Swedish message passes through unchanged', () => { + const msg = getErrorMessage({ code: 'SOME_CODE', message: 'Kunde inte hantera fakturan. Försök igen.' }) + expect(msg).toBe('Kunde inte hantera fakturan. Försök igen.') + }) +}) + describe('getErrorMessage: English locale uses registry English (C9)', () => { it('returns the registry English message for a known structured code instead of Swedish', () => { const code = 'FISCAL_PERIOD_NOT_FOUND' diff --git a/lib/errors/get-error-message.ts b/lib/errors/get-error-message.ts index 62491e6f..0b34f597 100644 --- a/lib/errors/get-error-message.ts +++ b/lib/errors/get-error-message.ts @@ -273,6 +273,28 @@ export function getErrorMessage( // of the whole `result`. Pick the English variant when the UI locale is // English; otherwise fall back to the Swedish `message`. if (typeof obj.code === 'string' && typeof obj.message === 'string' && obj.message.trim()) { + // Typed domain exceptions (lib/bookkeeping/errors.ts classes) also match + // this shape, but their `message` is raw English (often a DB constraint + // string) and must never reach the user verbatim. Normalize the instance + // into the structured envelope so the per-code branches below own the + // translation. Class fields are enumerable own props, so { ...obj } + // carries exactly the details those branches expect (totalDebit, + // lockDate, reason, issues, ...), while the non-enumerable Error.message + // stays out of details. Plain objects (forwarded inner envelopes, + // PostgrestError-shaped literals) keep the passthrough behavior. + if (error instanceof Error) { + return getErrorMessage( + { + error: { + code: obj.code, + message: obj.message, + account_numbers: (obj as { accountNumbers?: unknown }).accountNumbers, + details: { ...obj }, + }, + }, + options + ) + } if (locale === 'en' && typeof obj.message_en === 'string' && obj.message_en.trim()) { return obj.message_en } @@ -407,6 +429,13 @@ export function getErrorMessage( return structured.message_en } if (typeof structured.message === 'string' && structured.message.trim()) { + // Known codes without a dynamic branch above (e.g. CANNOT_REVERSE_STORNO) + // carry raw English engine messages: prefer the registry's Swedish + // message so no typed code surfaces English in a Swedish UI. + if (locale === 'sv' && typeof structured.code === 'string' && !isSwedishUserMessage(structured.message)) { + const entry = getErrorEntry(structured.code) + if (entry?.message_sv) return entry.message_sv + } return structured.message } }