From 0999bc0b8ba177f357c7485508a5389252d3f0cd Mon Sep 17 00:00:00 2001 From: Hagernesh Date: Mon, 17 Aug 2026 11:53:12 +0000 Subject: [PATCH] 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 {