mirror of
https://github.com/Tria-plc/edr-platform.git
synced 2026-08-26 18:42:49 +00:00
fix(eims): drain pre-reservation rejections in auto-submit sweep
A BadRequestException thrown before reserve() (config assertion, DEB/CRE validation) left the invoice NOT_SUBMITTED with nothing persisted, so the same row was retried every tick forever — a permanent head-of-line block on every invoice behind it. Now marked FAILED, guarded by a fresh status re-read so a reservation's own SUBMITTING/UNKNOWN/blocked state is never clobbered. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
|||||||
|
import { BadRequestException } from "@nestjs/common";
|
||||||
import { ConfigService } from "@nestjs/config";
|
import { ConfigService } from "@nestjs/config";
|
||||||
import { DataSource } from "typeorm";
|
import { DataSource } from "typeorm";
|
||||||
|
|
||||||
@@ -12,6 +13,9 @@ const INVOICE_ID = "11111111-1111-4111-8111-111111111111";
|
|||||||
/**
|
/**
|
||||||
* `query` is answered by shape: the first call is the system-state guard, the second is the
|
* `query` is answered by shape: the first call is the system-state guard, the second is the
|
||||||
* candidate lookup. Keeps the fake honest about the order the service actually asks in.
|
* candidate lookup. Keeps the fake honest about the order the service actually asks in.
|
||||||
|
*
|
||||||
|
* `managerRow` backs `dataSource.manager.findOne`/`.update` — only exercised by the
|
||||||
|
* pre-reservation-rejection path (`failStalledCandidate`), so it defaults to the candidate itself.
|
||||||
*/
|
*/
|
||||||
const build = (
|
const build = (
|
||||||
opts: {
|
opts: {
|
||||||
@@ -19,6 +23,7 @@ const build = (
|
|||||||
state?: { in_flight_invoice_id?: string | null; blocked_reason?: string | null };
|
state?: { in_flight_invoice_id?: string | null; blocked_reason?: string | null };
|
||||||
candidate?: { id: string; invoiceNumber: string } | null;
|
candidate?: { id: string; invoiceNumber: string } | null;
|
||||||
register?: jest.Mock;
|
register?: jest.Mock;
|
||||||
|
managerRow?: { eimsStatus: EimsInvoiceStatus } | null;
|
||||||
} = {},
|
} = {},
|
||||||
) => {
|
) => {
|
||||||
const register =
|
const register =
|
||||||
@@ -34,12 +39,17 @@ const build = (
|
|||||||
return Promise.resolve(opts.candidate === undefined ? [] : opts.candidate ? [opts.candidate] : []);
|
return Promise.resolve(opts.candidate === undefined ? [] : opts.candidate ? [opts.candidate] : []);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const managerUpdate = jest.fn().mockResolvedValue(undefined);
|
||||||
|
const managerFindOne = jest
|
||||||
|
.fn()
|
||||||
|
.mockResolvedValue(opts.managerRow === undefined ? { eimsStatus: EimsInvoiceStatus.NotSubmitted } : opts.managerRow);
|
||||||
|
|
||||||
const service = new EimsAutoSubmitService(
|
const service = new EimsAutoSubmitService(
|
||||||
{ query } as unknown as DataSource,
|
{ query, manager: { findOne: managerFindOne, update: managerUpdate } } as unknown as DataSource,
|
||||||
{ get: () => eimsConfig({ autoSubmit: true, ...opts.cfg }) } as unknown as ConfigService,
|
{ get: () => eimsConfig({ autoSubmit: true, ...opts.cfg }) } as unknown as ConfigService,
|
||||||
{ registerInvoiceWithEims: register } as unknown as EimsInvoiceRegistrationService,
|
{ registerInvoiceWithEims: register } as unknown as EimsInvoiceRegistrationService,
|
||||||
);
|
);
|
||||||
return { service, register, query };
|
return { service, register, query, managerUpdate, managerFindOne };
|
||||||
};
|
};
|
||||||
|
|
||||||
const candidate = { id: INVOICE_ID, invoiceNumber: "INV-20260807-00006" };
|
const candidate = { id: INVOICE_ID, invoiceNumber: "INV-20260807-00006" };
|
||||||
@@ -121,6 +131,40 @@ describe("EimsAutoSubmitService.tick", () => {
|
|||||||
expect(register).toHaveBeenCalledTimes(1);
|
expect(register).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("drains a pre-reservation rejection so the sweep advances, without touching the DB row's own reservation state", async () => {
|
||||||
|
const register = jest
|
||||||
|
.fn()
|
||||||
|
.mockRejectedValue(new BadRequestException({ code: "EIMS_RELATED_INVOICE_REQUIRED", message: "no related invoice" }));
|
||||||
|
const { service, managerFindOne, managerUpdate } = build({ candidate, register });
|
||||||
|
|
||||||
|
await expect(service.tick()).resolves.toBeUndefined();
|
||||||
|
|
||||||
|
expect(managerFindOne).toHaveBeenCalledTimes(1);
|
||||||
|
expect(managerUpdate).toHaveBeenCalledWith(
|
||||||
|
expect.anything(),
|
||||||
|
INVOICE_ID,
|
||||||
|
expect.objectContaining({
|
||||||
|
eimsStatus: EimsInvoiceStatus.Failed,
|
||||||
|
eimsLastError: expect.objectContaining({ message: "no related invoice" }),
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves a row alone if it already moved past NOT_SUBMITTED by the time the rejection is handled", async () => {
|
||||||
|
const register = jest
|
||||||
|
.fn()
|
||||||
|
.mockRejectedValue(new BadRequestException({ code: "EIMS_RELATED_INVOICE_REQUIRED", message: "no related invoice" }));
|
||||||
|
const { service, managerUpdate } = build({
|
||||||
|
candidate,
|
||||||
|
register,
|
||||||
|
managerRow: { eimsStatus: EimsInvoiceStatus.Submitting },
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(service.tick()).resolves.toBeUndefined();
|
||||||
|
|
||||||
|
expect(managerUpdate).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it("does not start a second tick while one is still filing", async () => {
|
it("does not start a second tick while one is still filing", async () => {
|
||||||
let release: () => void = () => {};
|
let release: () => void = () => {};
|
||||||
const register = jest.fn().mockImplementation(
|
const register = jest.fn().mockImplementation(
|
||||||
|
|||||||
@@ -1,12 +1,14 @@
|
|||||||
import { Injectable, Logger } from "@nestjs/common";
|
import { BadRequestException, Injectable, Logger } from "@nestjs/common";
|
||||||
import { ConfigService } from "@nestjs/config";
|
import { ConfigService } from "@nestjs/config";
|
||||||
import { Cron } from "@nestjs/schedule";
|
import { Cron } from "@nestjs/schedule";
|
||||||
import { InjectDataSource } from "@nestjs/typeorm";
|
import { InjectDataSource } from "@nestjs/typeorm";
|
||||||
import { DataSource } from "typeorm";
|
import { DataSource } from "typeorm";
|
||||||
|
import type { QueryDeepPartialEntity } from "typeorm/query-builder/QueryPartialEntity.js";
|
||||||
|
|
||||||
import { EimsConfig } from "../../config/eims.config";
|
import { EimsConfig } from "../../config/eims.config";
|
||||||
|
import { Invoice } from "../billing/entities/invoice.entity";
|
||||||
import { EimsInvoiceRegistrationService } from "./eims-invoice-registration.service";
|
import { EimsInvoiceRegistrationService } from "./eims-invoice-registration.service";
|
||||||
import { EimsInvoiceStatus } from "./eims-registration.types";
|
import { EimsInvoiceError, EimsInvoiceStatus } from "./eims-registration.types";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Files issued invoices with MoR EIMS on a timer.
|
* Files issued invoices with MoR EIMS on a timer.
|
||||||
@@ -67,21 +69,62 @@ export class EimsAutoSubmitService {
|
|||||||
const candidate = await this.nextCandidate();
|
const candidate = await this.nextCandidate();
|
||||||
if (!candidate) return;
|
if (!candidate) return;
|
||||||
|
|
||||||
const view = await this.registration.registerInvoiceWithEims(candidate.id);
|
try {
|
||||||
this.logger.log(
|
const view = await this.registration.registerInvoiceWithEims(candidate.id);
|
||||||
`EIMS auto-submit: invoice ${candidate.invoiceNumber} -> ${view.eimsStatus}` +
|
this.logger.log(
|
||||||
(view.eimsIrn ? ` (IRN ${view.eimsIrn})` : ""),
|
`EIMS auto-submit: invoice ${candidate.invoiceNumber} -> ${view.eimsStatus}` +
|
||||||
);
|
(view.eimsIrn ? ` (IRN ${view.eimsIrn})` : ""),
|
||||||
|
);
|
||||||
|
} catch (err) {
|
||||||
|
// Every other failure path inside registerInvoiceWithEims persists FAILED/UNKNOWN itself
|
||||||
|
// (settleFailure) before throwing. A BadRequestException is the one exception: it is only
|
||||||
|
// ever thrown *before* a reservation is taken (config assertion, DEB/CRE validation), so
|
||||||
|
// nothing is persisted — left alone, this candidate is picked again next tick forever, a
|
||||||
|
// permanent head-of-line block on every invoice behind it. Drain it instead.
|
||||||
|
if (err instanceof BadRequestException) {
|
||||||
|
await this.failStalledCandidate(candidate, err);
|
||||||
|
} else {
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
}
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
// Never let a filing failure kill the job. The outcome is already persisted on the invoice
|
// Never let a filing failure kill the job. The outcome is already persisted on the invoice
|
||||||
// (FAILED or UNKNOWN with the gateway's own message), and a blocked system number stops the
|
// (FAILED or UNKNOWN with the gateway's own message, or drained by failStalledCandidate
|
||||||
// next tick at the guard above.
|
// above), and a blocked system number stops the next tick at the guard above.
|
||||||
this.logger.error(`EIMS auto-submit tick failed: ${(err as Error).message}`);
|
this.logger.error(`EIMS auto-submit tick failed: ${(err as Error).message}`);
|
||||||
} finally {
|
} finally {
|
||||||
this.running = false;
|
this.running = false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Mark a pre-reservation rejection as FAILED so the sweep advances past it — but only if the
|
||||||
|
* invoice is still exactly where this tick left it. A reservation's own transactions
|
||||||
|
* (SUBMITTING/UNKNOWN, or a system-wide block) are authoritative; this must never clobber them,
|
||||||
|
* so the status is re-read fresh rather than trusted from the stale `candidate` row.
|
||||||
|
*/
|
||||||
|
private async failStalledCandidate(
|
||||||
|
candidate: { id: string; invoiceNumber: string },
|
||||||
|
err: BadRequestException,
|
||||||
|
): Promise<void> {
|
||||||
|
const current = await this.dataSource.manager.findOne(Invoice, { where: { id: candidate.id } });
|
||||||
|
if (current?.eimsStatus !== EimsInvoiceStatus.NotSubmitted) {
|
||||||
|
this.logger.warn(
|
||||||
|
`EIMS auto-submit: invoice ${candidate.invoiceNumber} rejected before reservation, but is ` +
|
||||||
|
`no longer NOT_SUBMITTED (${current?.eimsStatus ?? "not found"}) — leaving state untouched.`,
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const lastError: EimsInvoiceError = { kind: "VALIDATION", message: err.message, at: new Date().toISOString() };
|
||||||
|
await this.dataSource.manager.update(Invoice, candidate.id, {
|
||||||
|
eimsStatus: EimsInvoiceStatus.Failed,
|
||||||
|
eimsLastError: lastError,
|
||||||
|
} as QueryDeepPartialEntity<Invoice>);
|
||||||
|
this.logger.error(
|
||||||
|
`EIMS auto-submit: invoice ${candidate.invoiceNumber} rejected before reservation: ${err.message}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/** Why filing is currently impossible for this system number, or null when it is free. */
|
/** Why filing is currently impossible for this system number, or null when it is free. */
|
||||||
private async systemBlockReason(): Promise<string | null> {
|
private async systemBlockReason(): Promise<string | null> {
|
||||||
const rows: { in_flight_invoice_id: string | null; blocked_reason: string | null }[] =
|
const rows: { in_flight_invoice_id: string | null; blocked_reason: string | null }[] =
|
||||||
|
|||||||
Reference in New Issue
Block a user