mirror of
https://github.com/Tria-plc/edr-platform.git
synced 2026-08-26 18:42:49 +00:00
fix(eims): a mapper failure after reservation also orphaned the block
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.
This commit is contained in:
@@ -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: "" } });
|
||||
|
||||
@@ -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<void> {
|
||||
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}`,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user