fix(orders): cover the after() fallback path and log when it engages
The mock for next/server's after() detaches the callback regardless of what production code does, which left the outer catch's fire-and-forget fallback untested and the runQuoteMatch inner catch tested for nothing. - New test 'falls back to an inline match when after() is unavailable' forces after() to throw and asserts suggestQuoteForOrder was called anyway, plus that the fallback log fired. Mutation: deleting the 'void runQuoteMatch()' line breaks it. - 'never fails order creation when the matcher throws' now spies on console.error and asserts the inner catch's swallow actually logged. Mutation: deleting the inner console.error breaks it. - The outer catch now logs [createOrder] after() unavailable... so a silent switch to fire-and-forget (after upgrade, waitUntil missing on deploy target) is impossible to miss in production logs. - Comment names error code E468, Next 16.2.6, and node_modules/next/dist/server/after/after.js, and corrects the justification: createOrder is an exported service function reachable from multiple layers, not 'a smoke script or another background job'. Verified against the documented baseline — same single pre-existing failure (splitOrderForContinuation > moves pending items...), no new failures introduced. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
11535fd432
commit
093bbafe76
2 changed files with 66 additions and 9 deletions
|
|
@ -2904,6 +2904,9 @@ describe("createOrder quote auto-detection", () => {
|
|||
});
|
||||
|
||||
it("never fails order creation when the matcher throws", async () => {
|
||||
const consoleErrorSpy = vi
|
||||
.spyOn(console, "error")
|
||||
.mockImplementation(() => {});
|
||||
const { suggestQuoteForOrder } = await import("./order-quote-link-service");
|
||||
vi.mocked(suggestQuoteForOrder).mockClear();
|
||||
vi.mocked(suggestQuoteForOrder).mockRejectedValueOnce(new Error("PB down"));
|
||||
|
|
@ -2918,5 +2921,50 @@ describe("createOrder quote auto-detection", () => {
|
|||
|
||||
expect(result.order.id).toBeDefined();
|
||||
expect(result.items_created.length).toBe(1);
|
||||
// Verifies the inner catch swallowed the matcher failure and
|
||||
// logged it — without this assertion the test passes purely
|
||||
// because the `after` mock detaches the promise regardless of
|
||||
// what production code does.
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("[createOrder] quote auto-detection failed for order"),
|
||||
expect.stringContaining("PB down"),
|
||||
);
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("falls back to an inline match when after() is unavailable", async () => {
|
||||
const consoleErrorSpy = vi
|
||||
.spyOn(console, "error")
|
||||
.mockImplementation(() => {});
|
||||
const { after } = await import("next/server");
|
||||
const { suggestQuoteForOrder } = await import("./order-quote-link-service");
|
||||
vi.mocked(suggestQuoteForOrder).mockClear();
|
||||
vi.mocked(after).mockClear();
|
||||
vi.mocked(after).mockImplementationOnce(() => {
|
||||
throw new Error("`after` was called outside a request scope.");
|
||||
});
|
||||
await pbMock.collection("customers").create({ id: "cust-x", name: "Acme" });
|
||||
await pbMock.collection("companies").create({ id: "comp-x", name: "Acme (Pty) Ltd" });
|
||||
|
||||
const r = await createOrder({
|
||||
customer_id: "cust-x",
|
||||
company_id: "comp-x",
|
||||
items: [{ description: "M16 Bolt", quantity: 50 }],
|
||||
});
|
||||
|
||||
expect(r.order.id).toBeDefined();
|
||||
// Proves the catch fired AND ran the inline fallback — without
|
||||
// the call assertion the test would pass on the spy alone even
|
||||
// if `void runQuoteMatch()` was deleted.
|
||||
expect(suggestQuoteForOrder).toHaveBeenCalledWith(r.order.id);
|
||||
// The fallback logs its engagement so a silent switch to
|
||||
// fire-and-forget is impossible to miss in production logs.
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
"[createOrder] after() unavailable, running quote match inline",
|
||||
),
|
||||
expect.anything(),
|
||||
);
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2235,16 +2235,25 @@ export async function createOrder(input: CreateOrderInput): Promise<CreateOrderR
|
|||
};
|
||||
try {
|
||||
after(runQuoteMatch);
|
||||
} catch {
|
||||
} catch (err) {
|
||||
// `after()` throws synchronously when called outside a request
|
||||
// scope — `workAsyncStorage.getStore()` comes back empty, which
|
||||
// happens for any caller that isn't inside a Route Handler, Server
|
||||
// Action, or Server Component (e.g. a smoke script or another
|
||||
// background job invoking `createOrder` directly). Since that
|
||||
// throw is synchronous, letting it escape would turn "matcher
|
||||
// unavailable" into "order creation failed" — exactly the failure
|
||||
// mode this whole block exists to avoid. Fall back to firing the
|
||||
// match without blocking the return.
|
||||
// scope — `workAsyncStorage.getStore()` comes back empty, throwing
|
||||
// error code E468. Next 16.2.6,
|
||||
// node_modules/next/dist/server/after/after.js.
|
||||
//
|
||||
// `createOrder` is an exported service function reachable from
|
||||
// multiple layers (route handlers, server components, agents),
|
||||
// not only from in-scope callers. Letting the throw escape would
|
||||
// turn "matcher unavailable" into "order creation failed" —
|
||||
// exactly the failure mode this whole block exists to avoid. Fall
|
||||
// back to firing the match without blocking the return.
|
||||
//
|
||||
// Logged because a silent fallback here is invisible: orders
|
||||
// would lose the latency guarantee permanently with no trace.
|
||||
console.error(
|
||||
`[createOrder] after() unavailable, running quote match inline for ${order.id}:`,
|
||||
err,
|
||||
);
|
||||
void runQuoteMatch();
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue