fix(ocr): capture escrow_ref from the escrow_posted event, not just submitOcr's return
escrowRefStr was only assigned after submitOcr resolved, but submitOcr posts the escrow internally and THEN calls the supplier over HTTP — a SupplierError there (timeout, network failure, non-2xx, malformed response, receipt mismatch) meant the escrow was already on chain while the failure path still recorded escrow_ref: null. Given the OCR upstream's 120s timeout budget this is likely the most common paid-but-failed shape, and the exact one the reconciliation spine exists for. Subscribe to the SDK's "progress" event (type "escrow_posted") around each submitOcr attempt so the ref is captured the instant it lands, regardless of whether submitOcr goes on to resolve or reject. Listener is scoped per candidate and removed in a finally so it cannot accumulate. Adds a regression test proving the failure-path usage row and the ocr_failed structured log line both carry the real ref instead of null; verified it fails without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -38,6 +38,7 @@ import {
|
||||
ALLOWED_OCR_OUTPUT_FORMATS,
|
||||
MAX_OCR_IMAGE_B64_CHARS,
|
||||
} from "@marketplace/shared/tx";
|
||||
import type { ProgressEvent } from "@marketplace/buyer/sdk";
|
||||
import type { GatewayDeps } from "../deps.js";
|
||||
import type { GatewayStore } from "../db/store.js";
|
||||
import { requireKey } from "../middleware/apiKeyAuth.js";
|
||||
@@ -224,8 +225,24 @@ async function runOcrOneShot(
|
||||
// Submit. Fall back to the next supplier ONLY on a pre-post
|
||||
// TxConstructionError (no escrow was posted). A SupplierError means the
|
||||
// escrow is already posted — do not retry (the sweeper recovers it).
|
||||
//
|
||||
// submitOcr posts the escrow internally and only returns escrowRef when
|
||||
// it resolves. When the supplier call AFTER the post throws
|
||||
// (SupplierError: timeout, network failure, non-2xx, malformed
|
||||
// response, receipt mismatch) the escrow already landed on chain but
|
||||
// escrowRefStr below would otherwise stay null — the OCR upstream has a
|
||||
// 120s budget, so this is likely the most common paid-but-failed shape.
|
||||
// The SDK emits a "progress" event of type "escrow_posted" (carrying
|
||||
// escrow_ref) the instant the tx confirms, so listen for it around each
|
||||
// attempt and capture the ref from the event rather than only from
|
||||
// submitOcr's return value (I2 follow-up).
|
||||
let lastErr: unknown;
|
||||
for (const cand of candidates) {
|
||||
const onProgress = (ev: unknown): void => {
|
||||
const e = ev as ProgressEvent;
|
||||
if (e.type === "escrow_posted" && e.escrow_ref) escrowRefStr = e.escrow_ref;
|
||||
};
|
||||
ctx.sdk.on("progress", onProgress);
|
||||
try {
|
||||
result = await ctx.sdk.submitOcr({
|
||||
advertRef: cand.advertRef,
|
||||
@@ -240,6 +257,8 @@ async function runOcrOneShot(
|
||||
lastErr = err;
|
||||
if (err instanceof TxConstructionError) continue;
|
||||
throw err;
|
||||
} finally {
|
||||
ctx.sdk.off("progress", onProgress);
|
||||
}
|
||||
}
|
||||
if (!result || !used) {
|
||||
|
||||
@@ -21,6 +21,7 @@ import { hashApiKey } from "../src/middleware/apiKeyAuth.js";
|
||||
import { venueAttribution } from "../src/ocr/extract.js";
|
||||
import type { GatewayConfig } from "../src/config.js";
|
||||
import type { GatewayDeps } from "../src/deps.js";
|
||||
import { SupplierError } from "@marketplace/buyer/sdk";
|
||||
|
||||
// The one-shot settle step (resolveSubmittedRef → acceptAndConfirm) talks to
|
||||
// the live indexer + chain; the happy-path route test below only needs to
|
||||
@@ -325,3 +326,77 @@ describe("POST /v1/ocr/extract — happy path", () => {
|
||||
expect(res.body.x_vector.escrow_ref).toBe(`${"d".repeat(64)}#0`);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Failure path — escrow ref survives a post-escrow SupplierError (I2) ────
|
||||
//
|
||||
// submitOcr posts the escrow, THEN calls the supplier. A SupplierError
|
||||
// (timeout, network failure, non-2xx, malformed response, receipt mismatch)
|
||||
// means the escrow already landed on chain even though submitOcr rejects —
|
||||
// per the OCR route's 120s supplier budget, this is likely the MOST common
|
||||
// paid-but-failed shape. Before this fix, escrowRefStr was only assigned
|
||||
// from submitOcr's *return value*, so this exact case recorded escrow_ref:
|
||||
// null. The route now also listens for the SDK's "escrow_posted" progress
|
||||
// event around each attempt, so the ref survives regardless of whether
|
||||
// submitOcr goes on to resolve or reject.
|
||||
|
||||
describe("POST /v1/ocr/extract — escrow ref survives a post-escrow SupplierError", () => {
|
||||
it("records the escrow_ref from the escrow_posted progress event, not null, when submitOcr posts an escrow then rejects", async () => {
|
||||
const fetchFn = (async (url: unknown) => {
|
||||
const u = String(url);
|
||||
if (u.includes("/suppliers")) {
|
||||
return new Response(JSON.stringify([OCR_SUPPLIER]), { status: 200 });
|
||||
}
|
||||
return new Response("[]", { status: 200 });
|
||||
}) as unknown as typeof globalThis.fetch;
|
||||
|
||||
const deps = makeDeps(fetchFn, { chain: fundedChain });
|
||||
const rawKey = makeKey(deps, 0);
|
||||
const keyRow = deps.store.getKeyByHash(hashApiKey(rawKey))!;
|
||||
|
||||
// Same monkeypatch pattern as the happy-path test above: fetch the real
|
||||
// per-key Marketplace instance from the registry BEFORE the request so
|
||||
// its real EventEmitter (on/off/emit) stays live, then replace only
|
||||
// submitOcr. The stub mirrors what Marketplace.submitOcr actually does
|
||||
// on this failure shape: emit "escrow_posted" the instant the tx
|
||||
// confirms, then throw once the supplier call fails.
|
||||
const ctx = deps.registry.getContext(keyRow);
|
||||
const knownEscrowRef = `${"9".repeat(64)}#0`;
|
||||
const submitOcr = vi.fn(async () => {
|
||||
ctx.sdk.emit("progress", { type: "escrow_posted", escrow_ref: knownEscrowRef });
|
||||
throw new SupplierError("timeout", { message: "supplier did not respond within budget" });
|
||||
});
|
||||
(ctx.sdk as { submitOcr: unknown }).submitOcr = submitOcr;
|
||||
|
||||
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
const res = await request(createApp(deps))
|
||||
.post("/v1/ocr/extract")
|
||||
.set("Authorization", `Bearer ${rawKey}`)
|
||||
.send(validBody());
|
||||
// Snapshot the recorded calls BEFORE restoring — mockRestore() also
|
||||
// clears .mock.calls (it runs mockReset()/mockClear() first), so
|
||||
// reading logSpy.mock.calls after restore always sees [].
|
||||
const loggedLines = logSpy.mock.calls.map((args) => String(args[0]));
|
||||
logSpy.mockRestore();
|
||||
|
||||
expect(submitOcr).toHaveBeenCalledTimes(1);
|
||||
// toGatewayError maps SupplierError("timeout") to 504 supplier_timeout —
|
||||
// confirms the request actually took the failure path this test targets.
|
||||
expect(res.status).toBe(504);
|
||||
expect(res.body.error?.code).toBe("supplier_timeout");
|
||||
|
||||
// The reconciliation spine: the usage row recorded in the catch block
|
||||
// must carry the real ref, not null.
|
||||
const rows = deps.store.listUsage(keyRow.id, 10);
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(rows[0].status).toBe("failed");
|
||||
expect(rows[0].escrow_ref).toBe(knownEscrowRef);
|
||||
|
||||
// The structured ocr_failed log line (wedge-ledger emitter input) must
|
||||
// carry the same ref.
|
||||
const failedLine = loggedLines
|
||||
.filter((line) => line.includes('"evt":"ocr_failed"'))
|
||||
.map((line) => JSON.parse(line) as { escrow_ref: string | null });
|
||||
expect(failedLine).toHaveLength(1);
|
||||
expect(failedLine[0].escrow_ref).toBe(knownEscrowRef);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user