From 0464a44de470923f2dbd9cb1bfa454c379354416 Mon Sep 17 00:00:00 2001 From: Hagernesh Date: Mon, 17 Aug 2026 11:21:39 +0000 Subject: [PATCH 1/2] fix(eims): a config error before signing must not block the whole system MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit settleFailure() treated any non-EimsApiException error as ambiguous ("might have reached MoR") and permanently blocked all further filing until manually resolved. EimsConfigException (bad/missing key, unparseable cert) is thrown by EimsSignerService before EimsClientService.send()'s try/catch is even entered — by construction it never reached the wire, so there is nothing ambiguous about it. This is exactly what happened live: a private-key parse failure during the key/cert migration work reserved a counter, failed before any HTTP call, and got treated as an unresolved in-flight submission — blocking every other invoice from filing until someone manually POSTs /eims/resolve. Fix: EimsConfigException is now deterministic in settleFailure, same treatment as a clean MoR rejection — both counters roll back, no system-wide block, invoice marked FAILED (not UNKNOWN). Added a CONFIG failure kind so the invoice's eimsLastError and the staff alert both say plainly that the request never reached MoR, instead of implying a MoR rejection. --- .../eims-invoice-registration.service.spec.ts | 25 ++++++++++++++++++- .../eims/eims-invoice-registration.service.ts | 23 +++++++++++++---- .../src/modules/eims/eims.errors.ts | 3 ++- 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.spec.ts b/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.spec.ts index 79d3204dd..5fc74f988 100644 --- a/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.spec.ts +++ b/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.spec.ts @@ -10,7 +10,7 @@ import { NotificationInboxService } from "../notification-inbox/notification-inb import { NotificationsService } from "../notifications/notifications.service"; import { EimsAuthService } from "./eims-auth.service"; import { EimsClientService } from "./eims-client.service"; -import { EimsApiException } from "./eims.errors"; +import { EimsApiException, EimsConfigException } from "./eims.errors"; import { buildEimsSeller } from "./eims-invoice-context"; import { EimsInvoiceRegistrationService } from "./eims-invoice-registration.service"; import { EimsSellerCacheService } from "./eims-seller-cache.service"; @@ -479,6 +479,29 @@ describe("EimsInvoiceRegistrationService.registerInvoiceWithEims", () => { }); }); + it("a config error (bad key, never reached MoR) rolls back both counters, no system block", async () => { + const db = new FakeDb([invoiceRow()]); + const postSigned = jest + .fn() + .mockRejectedValue(new EimsConfigException("EIMS private key ... could not be read or parsed")); + + await expect(build(db, postSigned).registerInvoiceWithEims(INVOICE_ID)).rejects.toBeInstanceOf( + EimsConfigException, + ); + + expect(db.invoices.get(INVOICE_ID)).toMatchObject({ + eimsStatus: EimsInvoiceStatus.Failed, + eimsIrn: null, + eimsLastError: expect.objectContaining({ kind: "CONFIG" }), + }); + expect(db.state).toMatchObject({ + inFlightInvoiceId: null, + blockedReason: null, + previousIrn: null, + nextInvoiceCounter: 7, + }); + }); + it("treats a success response with no IRN as a failed registration", async () => { const db = new FakeDb([invoiceRow()]); const postSigned = jest.fn().mockResolvedValue({ statusCode: 200, body: { irn: "" } }); diff --git a/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.ts b/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.ts index 2ca5f5ffd..d96c64947 100644 --- a/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.ts +++ b/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.ts @@ -26,7 +26,7 @@ import { toEimsInvoiceStatusView } from "./eims-invoice-view.util"; import { FREIGHT_PERMS } from "../../seed/freight-permissions.registry"; import { EimsAuthService } from "./eims-auth.service"; import { EimsClientService } from "./eims-client.service"; -import { EimsApiException } from "./eims.errors"; +import { EimsApiException, EimsConfigException } from "./eims.errors"; import { EimsSellerCacheService } from "./eims-seller-cache.service"; import { EimsSystemState } from "./entities/eims-system-state.entity"; import { assertEimsInvoiceConfig, buildEimsContext } from "./eims-invoice-context"; @@ -469,6 +469,14 @@ export class EimsInvoiceRegistrationService { * when two rejected self-test attempts deadlocked the sequence until a manual DB reset. * * An ambiguous result keeps both: MoR may have counted and stored the document. + * + * `EimsConfigException` is also deterministic, for a different reason: it's thrown by + * `EimsSignerService`/`EimsCredentialsProvider` *before* `EimsClientService.send()`'s own + * try/catch is even entered (see `send()` — signing happens above its try block), so by + * construction no HTTP call was ever made. There is nothing to be ambiguous about — a bad key or + * missing config can't have reached MoR. Every error that *did* touch the wire is normalized to + * `EimsApiException` before it gets here (`toEimsApiException`), so this check is exhaustive: + * config errors are the only other kind `submit()` can throw. */ private async settleFailure( invoiceId: string, @@ -476,10 +484,11 @@ export class EimsInvoiceRegistrationService { err: unknown, ): Promise { const api = err instanceof EimsApiException ? err : null; - const deterministic = api ? DETERMINISTIC_KINDS.has(api.kind) : false; + const isConfigError = err instanceof EimsConfigException; + const deterministic = isConfigError || (api ? DETERMINISTIC_KINDS.has(api.kind) : false); const status = deterministic ? EimsInvoiceStatus.Failed : EimsInvoiceStatus.Unknown; const lastError: EimsInvoiceError = { - kind: api?.kind ?? "UNKNOWN", + kind: isConfigError ? "CONFIG" : (api?.kind ?? "UNKNOWN"), message: (err as Error)?.message ?? "unknown error", httpStatus: api?.httpStatus, details: api?.details, @@ -586,10 +595,14 @@ export class EimsInvoiceRegistrationService { type: NotificationType.GENERIC, priority: deterministic ? NotificationPriority.NORMAL : NotificationPriority.HIGH, title: deterministic - ? "EIMS rejected an invoice" + ? error.kind === "CONFIG" + ? "EIMS filing failed before reaching MoR" + : "EIMS rejected an invoice" : "EIMS filing unresolved — all further filing is blocked", body: deterministic - ? `MoR rejected the filing (${error.kind}): ${error.message}. The invoice is marked FAILED; correct it and file again.` + ? error.kind === "CONFIG" + ? `EIMS is misconfigured: ${error.message}. Nothing was sent to MoR; fix the config and file again.` + : `MoR rejected the filing (${error.kind}): ${error.message}. The invoice is marked FAILED; correct it and file again.` : `A submission was sent but never acknowledged (${error.kind}). Its IRN is unknown, so no further invoice can be filed until it is resolved with MoR.`, link: `/dashboard/invoices/${invoiceId}`, data: { invoiceId, eimsStatus: status, kind: error.kind, action: "EIMS_FILING_FAILED" }, diff --git a/apps/edr-freight-api/src/modules/eims/eims.errors.ts b/apps/edr-freight-api/src/modules/eims/eims.errors.ts index 3be21fdd3..75c513824 100644 --- a/apps/edr-freight-api/src/modules/eims/eims.errors.ts +++ b/apps/edr-freight-api/src/modules/eims/eims.errors.ts @@ -10,7 +10,8 @@ export type EimsFailureKind = | "FORBIDDEN" | "RULE_VALIDATION" | "SERVER" - | "UNKNOWN"; + | "UNKNOWN" + | "CONFIG"; /** Raised when EIMS is disabled or its credential files are unusable. */ export class EimsConfigException extends ServiceUnavailableException { From 0999bc0b8ba177f357c7485508a5389252d3f0cd Mon Sep 17 00:00:00 2001 From: Hagernesh Date: Mon, 17 Aug 2026 11:53:12 +0000 Subject: [PATCH 2/2] fix(eims): a mapper failure after reservation also orphaned the block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit toEimsInvoice/buildEimsContext sat outside the try/catch that calls settleFailure — reservation happens (TX1), then request-building ran unguarded, then submit() was the only thing actually wrapped. Any exception during mapping (a validation error like an unmapped buyer country, or a bug) skipped settleFailure entirely and left the reservation permanently held: exactly the live incident just seen — register 500'd, and every subsequent attempt on any invoice 409'd 'already in flight' until manually resolved. Fix: the try block now starts right after reserve(), covering request-building and submit() both. settleFailure's determinism check is generalized to match — any error that is not an EimsApiException is pre-wire and safe to release, not just EimsConfigException (still labeled CONFIG; everything else pre-wire is now labeled the new LOCAL kind). This is exhaustive by construction: every error that actually touches the wire is already normalized to EimsApiException inside EimsClientService.send()'s own catch, so nothing outside that can be ambiguous. --- .../eims-invoice-registration.service.spec.ts | 30 +++++++++ .../eims/eims-invoice-registration.service.ts | 64 ++++++++++--------- .../src/modules/eims/eims.errors.ts | 3 +- 3 files changed, 66 insertions(+), 31 deletions(-) diff --git a/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.spec.ts b/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.spec.ts index 5fc74f988..f560ff917 100644 --- a/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.spec.ts +++ b/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.spec.ts @@ -502,6 +502,36 @@ describe("EimsInvoiceRegistrationService.registerInvoiceWithEims", () => { }); }); + it("a mapper failure after reservation (e.g. unmapped buyer country) also releases the reservation", async () => { + // Regression: toEimsInvoice/buildEimsContext used to sit outside the try/catch that calls + // settleFailure — a throw here left the reservation permanently orphaned (a real live incident: + // 500 on register, then every subsequent attempt 409'd "already in flight" until manually + // resolved). This never reaches postSigned at all — the mapper throws before submit() is called. + const db = new FakeDb([ + invoiceRow({ company: { ...invoiceRow().company, country: "France" } as never }), + ]); + const postSigned = jest.fn(); + + // The mapper throws a plain Error (it's a pure function, not a NestJS layer) — that's the + // point: settleFailure must treat *any* non-EimsApiException as pre-wire, not just its own + // known exception types. + await expect(build(db, postSigned).registerInvoiceWithEims(INVOICE_ID)).rejects.toThrow( + /no MoR country code mapping/, + ); + + expect(postSigned).not.toHaveBeenCalled(); + expect(db.invoices.get(INVOICE_ID)).toMatchObject({ + eimsStatus: EimsInvoiceStatus.Failed, + eimsLastError: expect.objectContaining({ kind: "LOCAL" }), + }); + expect(db.state).toMatchObject({ + inFlightInvoiceId: null, + blockedReason: null, + previousIrn: null, + nextInvoiceCounter: 7, + }); + }); + it("treats a success response with no IRN as a failed registration", async () => { const db = new FakeDb([invoiceRow()]); const postSigned = jest.fn().mockResolvedValue({ statusCode: 200, body: { irn: "" } }); diff --git a/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.ts b/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.ts index d96c64947..a823e5ddf 100644 --- a/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.ts +++ b/apps/edr-freight-api/src/modules/eims/eims-invoice-registration.service.ts @@ -123,27 +123,29 @@ export class EimsInvoiceRegistrationService { const reservation = await this.reserve(invoiceId, session.systemNumber); if (!reservation) return this.getEimsStatus(invoiceId); - // The request can only be built now: InvoiceCounter and PreviousIrn come from the reservation. - const request = toEimsInvoice( - invoice, - this.sellerCache.getSellerDetails(cfg), - buildEimsContext(cfg, { - // Allocated from the system state, not our invoiceNumber: MoR validates DocumentNumber - // against ^(0|[1-9][0-9]{0,8})$, which "INV-20260807-00006" can never satisfy. - documentNumber: reservation.documentNumber, - invoiceCounter: reservation.invoiceCounter, - previousIrn: reservation.previousIrn, - session, - documentType, - reason: invoice.eimsReason, - relatedDocument, - }), - ); - let irn: string; let ackDate: string | undefined; let signedQR: string | undefined; try { + // The request can only be built now: InvoiceCounter and PreviousIrn come from the + // reservation. Building it — and everything after — stays inside this try: a reservation is + // held from here on, and *any* failure past this point, mapper or wire, must release it + // through settleFailure rather than leave it orphaned as a permanent system-wide block. + const request = toEimsInvoice( + invoice, + this.sellerCache.getSellerDetails(cfg), + buildEimsContext(cfg, { + // Allocated from the system state, not our invoiceNumber: MoR validates DocumentNumber + // against ^(0|[1-9][0-9]{0,8})$, which "INV-20260807-00006" can never satisfy. + documentNumber: reservation.documentNumber, + invoiceCounter: reservation.invoiceCounter, + previousIrn: reservation.previousIrn, + session, + documentType, + reason: invoice.eimsReason, + relatedDocument, + }), + ); // Deliberately outside every transaction — no DB lock is held across the wire. const result = await this.submit(request); irn = result.irn; @@ -470,13 +472,14 @@ export class EimsInvoiceRegistrationService { * * An ambiguous result keeps both: MoR may have counted and stored the document. * - * `EimsConfigException` is also deterministic, for a different reason: it's thrown by - * `EimsSignerService`/`EimsCredentialsProvider` *before* `EimsClientService.send()`'s own - * try/catch is even entered (see `send()` — signing happens above its try block), so by - * construction no HTTP call was ever made. There is nothing to be ambiguous about — a bad key or - * missing config can't have reached MoR. Every error that *did* touch the wire is normalized to - * `EimsApiException` before it gets here (`toEimsApiException`), so this check is exhaustive: - * config errors are the only other kind `submit()` can throw. + * Any error that is *not* an `EimsApiException` is also deterministic, on a different basis: + * every error that actually touches the wire is normalized to `EimsApiException` before it gets + * here (`EimsClientService.send()`'s catch calls `toEimsApiException` on whatever the HTTP call + * threw). The try block this feeds covers request-building (`toEimsInvoice`/`buildEimsContext` — + * pure, no I/O) and `submit()`; nothing in that span can produce another exception shape by + * touching MoR. So a non-`EimsApiException` here — a mapper validation error (unmapped buyer + * country, say), `EimsConfigException` from a bad signing key, or a bug — failed strictly before + * any HTTP call went out, and releasing the reservation is always safe, never a guess. */ private async settleFailure( invoiceId: string, @@ -484,11 +487,12 @@ export class EimsInvoiceRegistrationService { err: unknown, ): Promise { const api = err instanceof EimsApiException ? err : null; - const isConfigError = err instanceof EimsConfigException; - const deterministic = isConfigError || (api ? DETERMINISTIC_KINDS.has(api.kind) : false); + // Never touched the wire (see the doc comment above) — always safe to release, whatever it is. + const deterministic = api ? DETERMINISTIC_KINDS.has(api.kind) : true; const status = deterministic ? EimsInvoiceStatus.Failed : EimsInvoiceStatus.Unknown; + const localKind = err instanceof EimsConfigException ? "CONFIG" : "LOCAL"; const lastError: EimsInvoiceError = { - kind: isConfigError ? "CONFIG" : (api?.kind ?? "UNKNOWN"), + kind: api?.kind ?? localKind, message: (err as Error)?.message ?? "unknown error", httpStatus: api?.httpStatus, details: api?.details, @@ -595,13 +599,13 @@ export class EimsInvoiceRegistrationService { type: NotificationType.GENERIC, priority: deterministic ? NotificationPriority.NORMAL : NotificationPriority.HIGH, title: deterministic - ? error.kind === "CONFIG" + ? error.kind === "CONFIG" || error.kind === "LOCAL" ? "EIMS filing failed before reaching MoR" : "EIMS rejected an invoice" : "EIMS filing unresolved — all further filing is blocked", body: deterministic - ? error.kind === "CONFIG" - ? `EIMS is misconfigured: ${error.message}. Nothing was sent to MoR; fix the config and file again.` + ? error.kind === "CONFIG" || error.kind === "LOCAL" + ? `${error.kind === "CONFIG" ? "EIMS is misconfigured" : "Filing failed locally"}: ${error.message}. Nothing was sent to MoR; fix it and file again.` : `MoR rejected the filing (${error.kind}): ${error.message}. The invoice is marked FAILED; correct it and file again.` : `A submission was sent but never acknowledged (${error.kind}). Its IRN is unknown, so no further invoice can be filed until it is resolved with MoR.`, link: `/dashboard/invoices/${invoiceId}`, diff --git a/apps/edr-freight-api/src/modules/eims/eims.errors.ts b/apps/edr-freight-api/src/modules/eims/eims.errors.ts index 75c513824..853b32c29 100644 --- a/apps/edr-freight-api/src/modules/eims/eims.errors.ts +++ b/apps/edr-freight-api/src/modules/eims/eims.errors.ts @@ -11,7 +11,8 @@ export type EimsFailureKind = | "RULE_VALIDATION" | "SERVER" | "UNKNOWN" - | "CONFIG"; + | "CONFIG" + | "LOCAL"; /** Raised when EIMS is disabled or its credential files are unusable. */ export class EimsConfigException extends ServiceUnavailableException {