diff --git a/.changeset/browser-pay-relay-retry.md b/.changeset/browser-pay-relay-retry.md new file mode 100644 index 000000000..fb1418cf4 --- /dev/null +++ b/.changeset/browser-pay-relay-retry.md @@ -0,0 +1,5 @@ +--- +"@sapiom/browser-pay": patch +--- + +Retry a paused card relay on attempt timeout, transport failure, and router HTTP 502, 503, or 504, using the same idempotency key, until the total deadline. One attempt defaults to 25 seconds. The total wait defaults to 55 seconds. diff --git a/packages/browser-pay/README.md b/packages/browser-pay/README.md index e19425bff..f383e182c 100644 --- a/packages/browser-pay/README.md +++ b/packages/browser-pay/README.md @@ -63,7 +63,7 @@ No answer after the send: The injection is consumed. The paused request fails and the same body is not sent again. -A transport error, a redirect, a non-2xx HTTP status, or an unreadable body fails the paused browser request. David's relay returns HTTP 200 for every outcome, so a 401 or 500 is not read as `unknown` or `refused`. It is not fulfilled with the error JSON. If fulfill fails, the paused request is failed and the agent event is `relayed_unfulfilled`. +A redirect, a non-2xx HTTP status other than 502, 503, or 504, or an unreadable body fails the paused browser request. David's relay returns HTTP 200 for every outcome, so a 401 or 500 is not read as `unknown` or `refused`. It is not fulfilled with the error JSON. A transport error, a per-attempt timeout, and a router HTTP 502, 503, or 504 are posted again with the same idempotency key. If fulfill fails, the paused request is failed and the agent event is `relayed_unfulfilled`. ## What an agent does @@ -87,7 +87,7 @@ A request is relayed when the held placeholder number appears in the URL or body A relayed response fails the paused request only when that response contains the held placeholder number. -Identical in-flight retries (same method, URL, and body) share one relay poll, including the backoff between `relay_in_progress` answers. `AttachOptions.timeoutMs` is the deadline for a terminal outcome. It defaults to 20 seconds. While the relay answers `relay_in_progress`, the same key is posted again after a bounded backoff. At the deadline the paused request fails with `timeout`. A relay that ignores abort still fails when the deadline passes, and one waiter's timeout does not abort a later waiter of the same flight. A terminal refusal is logged once. A transport failure drops the key so a later retry can resend. `outcome: "unknown"` does not. +Identical in-flight retries (same method, URL, and body) share one relay poll, including the backoff. `AttachOptions.timeoutMs` is the total deadline. It defaults to 55 seconds: two 25 second attempts plus backoff, so a first attempt cut near the router limit can still be read back by a second POST. `AttachOptions.attemptTimeoutMs` is one POST. It defaults to 25 seconds so the client aborts before a router cuts the socket at 30 seconds. While the relay answers `relay_in_progress`, or an attempt times out, or the transport fails, or the router returns 502, 503, or 504, the same key is posted again after a bounded backoff. At the deadline the paused request fails with `timeout`. A relay that ignores abort still fails when the deadline passes. One waiter's timeout does not abort a later waiter of the same flight. A terminal refusal is logged once. `outcome: "unknown"` is not sent again. HTTP 502, 503, and 504 retries are how `httpPaymentRelay` classifies a failure. A custom relay is retried when the attempt abort rejects its promise. Captcha widgets, extension pages, and workers are not paused. A blank iframe, including `about:blank`, stays paused so a navigation onto the card URL cannot slip through. A placeholder that shows up on any other page or iframe is relayed. diff --git a/packages/browser-pay/src/interceptor.protocol.test.ts b/packages/browser-pay/src/interceptor.protocol.test.ts index 9b4291787..7187b7dda 100644 --- a/packages/browser-pay/src/interceptor.protocol.test.ts +++ b/packages/browser-pay/src/interceptor.protocol.test.ts @@ -1345,6 +1345,7 @@ describe("interceptor CDP decisions", () => { placeholder: HELD, profileId: "shopify-card-vault", timeoutMs: 400, + attemptTimeoutMs: 1_000, }); handle.on((event) => events.push(event)); attachPage(stub.emit); diff --git a/packages/browser-pay/src/interceptor.relay-in-progress.test.ts b/packages/browser-pay/src/interceptor.relay-in-progress.test.ts index 37ba1d714..f7f927ab7 100644 --- a/packages/browser-pay/src/interceptor.relay-in-progress.test.ts +++ b/packages/browser-pay/src/interceptor.relay-in-progress.test.ts @@ -838,6 +838,326 @@ describe("relay_in_progress", () => { }); }); +function listenRelay( + respond: ( + index: number, + req: http.IncomingMessage, + res: http.ServerResponse, + ) => void, +): Promise<{ + origin: string; + posts: SeenPost[]; + close: () => Promise; +}> { + const posts: SeenPost[] = []; + const server = http.createServer((req, res) => { + const chunks: Buffer[] = []; + req.on("data", (chunk: Buffer) => chunks.push(chunk)); + req.on("end", () => { + const body = JSON.parse( + Buffer.concat(chunks).toString("utf8"), + ) as RelayRequest; + posts.push({ + method: req.method ?? "", + url: req.url ?? "", + idempotencyKey: body.idempotencyKey, + requestBody: body.request.body, + }); + respond(posts.length, req, res); + }); + }); + return new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => { + const address = server.address() as AddressInfo; + resolve({ + origin: `http://127.0.0.1:${address.port}`, + posts, + close: () => + new Promise((done) => { + server.closeAllConnections(); + server.close(() => done()); + }), + }); + }); + }); +} + +async function driveHeldPost(options: { + origin: string; + timeoutMs: number; + attemptTimeoutMs?: number; + until: (events: InterceptorEvent[], commands: Command[]) => boolean; + label: string; + beforeDetach?: () => Promise; +}): Promise<{ events: InterceptorEvent[]; commands: Command[] }> { + const cdp = await startCdp(); + const events: InterceptorEvent[] = []; + const handle = await attachInterceptor(cdp.url, { + relay: httpPaymentRelay({ + baseUrl: options.origin, + apiKey: "test-key", + transactionId: TRANSACTION_ID, + }), + placeholder: HELD, + profileId: "shopify-card-vault", + timeoutMs: options.timeoutMs, + ...(options.attemptTimeoutMs === undefined + ? {} + : { attemptTimeoutMs: options.attemptTimeoutMs }), + }); + handle.on((event) => events.push(event)); + try { + cdp.emit({ + method: "Target.attachedToTarget", + params: { + sessionId: "page-1", + waitingForDebugger: true, + targetInfo: { + targetId: "target-page", + type: "page", + url: "https://shop.example/checkout", + }, + }, + }); + await waitFor( + () => cdp.commands.some((command) => command.method === "Fetch.enable"), + "Fetch.enable", + ); + cdp.emit({ + method: "Fetch.requestPaused", + sessionId: "page-1", + params: { + requestId: "req-retry", + request: { + url: CHECKOUT_URL, + method: "POST", + headers: { "content-type": "application/json" }, + postData: CARD, + }, + }, + }); + await waitFor(() => options.until(events, cdp.commands), options.label); + if (options.beforeDetach) await options.beforeDetach(); + return { events, commands: cdp.commands }; + } finally { + await handle.detach(); + await cdp.close(); + } +} + +function sameKey(posts: SeenPost[], idempotencyKey: string): void { + expect(posts.length).toBeGreaterThan(0); + expect(posts.every((post) => post.idempotencyKey === idempotencyKey)).toBe( + true, + ); + expect( + posts.every((post) => post.method === "POST" && post.url === RELAY_PATH), + ).toBe(true); +} + +describe("relay retry before the deadline", () => { + const idempotencyKey = paymentIdempotencyKey("POST", CHECKOUT_URL, CARD); + + it("retries the same key after an attempt timeout and fulfills", async () => { + const relay = await listenRelay((index, _req, res) => { + if (index === 1) return; + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify(RELAYED)); + }); + try { + const settled = await driveHeldPost({ + origin: relay.origin, + timeoutMs: 2_000, + attemptTimeoutMs: 80, + until: (_events, commands) => + commands.some((command) => command.method === "Fetch.fulfillRequest"), + label: "fulfill after timeout", + }); + expect(relay.posts.map((post) => post.idempotencyKey)).toEqual([ + idempotencyKey, + idempotencyKey, + ]); + expect(count(settled.commands, "Fetch.failRequest")).toBe(0); + expect(count(settled.commands, "Fetch.fulfillRequest")).toBe(1); + } finally { + await relay.close(); + } + }); + + it("retries the same key after HTTP 503 and fulfills", async () => { + const relay = await listenRelay((index, _req, res) => { + if (index === 1) { + res.writeHead(503); + res.end("unavailable"); + return; + } + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify(RELAYED)); + }); + try { + const settled = await driveHeldPost({ + origin: relay.origin, + timeoutMs: 2_000, + until: (_events, commands) => + commands.some((command) => command.method === "Fetch.fulfillRequest"), + label: "fulfill after 503", + }); + expect(relay.posts.map((post) => post.idempotencyKey)).toEqual([ + idempotencyKey, + idempotencyKey, + ]); + expect(count(settled.commands, "Fetch.failRequest")).toBe(0); + expect(count(settled.commands, "Fetch.fulfillRequest")).toBe(1); + } finally { + await relay.close(); + } + }); + + it("retries the same key after a transport error and fulfills", async () => { + const relay = await listenRelay((index, req, res) => { + if (index === 1) { + req.socket?.destroy(); + return; + } + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify(RELAYED)); + }); + try { + const settled = await driveHeldPost({ + origin: relay.origin, + timeoutMs: 2_000, + until: (_events, commands) => + commands.some((command) => command.method === "Fetch.fulfillRequest"), + label: "fulfill after transport error", + }); + sameKey(relay.posts, idempotencyKey); + expect(relay.posts).toHaveLength(2); + expect(count(settled.commands, "Fetch.fulfillRequest")).toBe(1); + expect(count(settled.commands, "Fetch.failRequest")).toBe(0); + } finally { + await relay.close(); + } + }); + + it("fails with timeout when retries exhaust the deadline", async () => { + const relay = await listenRelay((_index, _req, res) => { + res.writeHead(503); + res.end("unavailable"); + }); + try { + const settled = await driveHeldPost({ + origin: relay.origin, + timeoutMs: 900, + until: (events) => events.some((event) => event.type === "failed"), + label: "deadline", + }); + sameKey(relay.posts, idempotencyKey); + expect(relay.posts.length).toBeGreaterThanOrEqual(2); + expect(count(settled.commands, "Fetch.failRequest")).toBe(1); + expect(count(settled.commands, "Fetch.fulfillRequest")).toBe(0); + expect(settled.events).toContainEqual( + expect.objectContaining({ type: "failed", reason: "timeout" }), + ); + } finally { + await relay.close(); + } + }); + + it("does not retry an HTTP 500", async () => { + const relay = await listenRelay((_index, _req, res) => { + res.writeHead(500); + res.end("application error"); + }); + try { + const settled = await driveHeldPost({ + origin: relay.origin, + timeoutMs: 2_000, + until: (events) => events.some((event) => event.type === "failed"), + label: "500", + beforeDetach: () => new Promise((resolve) => setTimeout(resolve, 400)), + }); + expect(relay.posts.map((post) => post.idempotencyKey)).toEqual([ + idempotencyKey, + ]); + expect(count(settled.commands, "Fetch.failRequest")).toBe(1); + expect(settled.events).toContainEqual( + expect.objectContaining({ type: "failed", reason: "network" }), + ); + } finally { + await relay.close(); + } + }); + + it("aborts one shared attempt and fulfills both pauses", async () => { + const cdp = await startCdp(); + let calls = 0; + let attemptStarted = 0; + const aborts: number[] = []; + const handle = await attachInterceptor(cdp.url, { + relay: { + relay(_request, options) { + calls += 1; + if (calls === 1) { + attemptStarted = Date.now(); + return new Promise((_resolve, reject) => { + options?.signal?.addEventListener("abort", () => { + aborts.push(Date.now() - attemptStarted); + reject(new Error("aborted")); + }); + }); + } + return Promise.resolve(RELAYED); + }, + }, + placeholder: HELD, + profileId: "shopify-card-vault", + timeoutMs: 2_000, + attemptTimeoutMs: 120, + }); + try { + armPage(cdp); + await waitFor( + () => cdp.commands.some((command) => command.method === "Fetch.enable"), + "Fetch.enable", + ); + emitPaused(cdp, "req-shared-a"); + await waitFor(() => calls === 1, "first attempt"); + await new Promise((resolve) => setTimeout(resolve, 80)); + emitPaused(cdp, "req-shared-b"); + await waitFor( + () => count(cdp.commands, "Fetch.fulfillRequest") === 2, + "both fulfilled", + ); + expect(calls).toBe(2); + expect(aborts).toHaveLength(1); + expect(aborts[0]).toBeGreaterThanOrEqual(100); + expect(aborts[0]).toBeLessThan(160); + expect(count(cdp.commands, "Fetch.failRequest")).toBe(0); + } finally { + await handle.detach(); + await cdp.close(); + } + }); + + it("rejects a timeout that is not a finite positive number", async () => { + const relay = { relay: async () => RELAYED }; + const base = { + relay, + placeholder: HELD, + profileId: "shopify-card-vault", + }; + await expect( + attachInterceptor("ws://127.0.0.1:9", { ...base, timeoutMs: Number.NaN }), + ).rejects.toThrow(/timeoutMs/); + await expect( + attachInterceptor("ws://127.0.0.1:9", { ...base, attemptTimeoutMs: 0 }), + ).rejects.toThrow(/attemptTimeoutMs/); + await expect( + attachInterceptor("ws://127.0.0.1:9", { ...base, timeoutMs: 2 ** 31 }), + ).rejects.toThrow(/timeoutMs/); + }); +}); + function armPage(cdp: { emit: (event: Record) => void; }): void { diff --git a/packages/browser-pay/src/interceptor.ts b/packages/browser-pay/src/interceptor.ts index c92d7bc1c..c15d16918 100644 --- a/packages/browser-pay/src/interceptor.ts +++ b/packages/browser-pay/src/interceptor.ts @@ -13,6 +13,7 @@ import { isRecord } from "./guards.js"; import { silentLogger, type PayLogger } from "./logger.js"; import { isRelayTimeout, + isRetryableRelayError, RelayTimeoutError, type PaymentRelay, } from "./relay.js"; @@ -25,14 +26,21 @@ export interface AttachOptions { profileId: string; /** * How long to wait for a terminal relay outcome before failing the paused - * request. `relay_in_progress` is posted again with the same idempotency - * key until this deadline. Identical pauses share that poll, including the - * backoff. Any other refusal fails the request. `outcome: "unknown"` is - * not sent again. A relay that ignores the abort signal still fails here. - * One waiter's timeout does not abort a shared flight that another waiter - * still holds. + * request. Defaults to 55 seconds. Identical pauses share one poll, + * including the backoff. `relay_in_progress`, a per-attempt timeout, a + * transport error, and HTTP 502, 503, and 504 are posted again with the + * same idempotency key until this deadline. Any other refusal fails the + * request. `outcome: "unknown"` is not sent again. A relay that ignores + * the abort signal still fails here. One waiter's timeout does not abort + * a shared flight that another waiter still holds. */ timeoutMs?: number; + /** + * How long one relay POST may run before it is aborted and tried again. + * Defaults to 25 seconds, under the 30 second router limit. The total + * wait is `timeoutMs`. + */ + attemptTimeoutMs?: number; /** Defaults to a logger that discards every line. */ logger?: PayLogger; } @@ -71,7 +79,8 @@ export type InterceptorTarget = | { browser(): { wsEndpoint(): string } | null } | { context(): { browser(): { wsEndpoint(): string } | null } | null }; -const DEFAULT_TIMEOUT_MS = 20_000; +const DEFAULT_ATTEMPT_TIMEOUT_MS = 25_000; +const DEFAULT_TIMEOUT_MS = 55_000; const RELAY_POLL_INITIAL_MS = 200; const RELAY_POLL_MAX_MS = 2_000; @@ -225,6 +234,7 @@ class PaymentInterceptor { private readonly loggedRefusals = new Set(); private readonly pendingArms = new Set>(); private readonly timeoutMs: number; + private readonly attemptTimeoutMs: number; private readonly logger: PayLogger; private backlog: InterceptorEvent[] | null = []; private buffering = true; @@ -235,6 +245,8 @@ class PaymentInterceptor { private readonly options: AttachOptions, ) { this.timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + this.attemptTimeoutMs = + options.attemptTimeoutMs ?? DEFAULT_ATTEMPT_TIMEOUT_MS; this.logger = options.logger ?? silentLogger; } @@ -568,7 +580,49 @@ class PaymentInterceptor { if (this.unknownKeys.has(idempotencyKey)) { return { outcome: "unknown" }; } - const response = await this.options.relay.relay( + try { + const response = await this.postAttempt( + request, + body, + pageOrigin, + idempotencyKey, + signal, + ); + if (response.outcome === "unknown") { + this.unknownKeys.add(idempotencyKey); + } + if (!isRelayInProgress(response)) return response; + } catch (error) { + if (signal.aborted || this.closed || !isRetryableRelayError(error)) { + throw error; + } + } + poll += 1; + await sleep(relayPollDelayMs(poll), signal); + } + } + + private async postAttempt( + request: PausedRequest, + body: string, + pageOrigin: string, + idempotencyKey: string, + signal: AbortSignal, + ): Promise { + const attempt = new AbortController(); + let timedOut = false; + const timer = setTimeout(() => { + timedOut = true; + attempt.abort(new RelayTimeoutError()); + }, this.attemptTimeoutMs); + const onAbort = () => { + clearTimeout(timer); + attempt.abort(signal.reason); + }; + if (signal.aborted) onAbort(); + else signal.addEventListener("abort", onAbort); + try { + return await this.options.relay.relay( { injectionRef: this.options.placeholder.injectionRef, profileId: this.options.profileId, @@ -581,14 +635,14 @@ class PaymentInterceptor { }, idempotencyKey, }, - { signal }, + { signal: attempt.signal }, ); - if (response.outcome === "unknown") { - this.unknownKeys.add(idempotencyKey); - } - if (!isRelayInProgress(response)) return response; - poll += 1; - await sleep(relayPollDelayMs(poll), signal); + } catch (error) { + if (timedOut && !signal.aborted) throw new RelayTimeoutError(); + throw error; + } finally { + clearTimeout(timer); + signal.removeEventListener("abort", onAbort); } } @@ -1256,10 +1310,21 @@ function readAttachedSessionId(value: unknown): string | undefined { return value.sessionId.length > 0 ? value.sessionId : undefined; } +const MAX_TIMER_MS = 2_147_483_647; + +function assertPositiveDuration(name: string, value: number | undefined): void { + if (value === undefined) return; + if (!Number.isFinite(value) || value <= 0 || value > MAX_TIMER_MS) { + throw new Error(`${name} must be a finite positive number`); + } +} + function assertAttachOptions(options: AttachOptions): void { if (typeof options.relay?.relay !== "function") { throw new Error("A payment relay is required"); } + assertPositiveDuration("timeoutMs", options.timeoutMs); + assertPositiveDuration("attemptTimeoutMs", options.attemptTimeoutMs); if (!options.profileId) { throw new Error("profileId is required"); } diff --git a/packages/browser-pay/src/relay.test.ts b/packages/browser-pay/src/relay.test.ts index 0f52a77f3..2b2b55d0f 100644 --- a/packages/browser-pay/src/relay.test.ts +++ b/packages/browser-pay/src/relay.test.ts @@ -1,7 +1,11 @@ import http from "node:http"; import type { AddressInfo } from "node:net"; import type { RelayRequest } from "./contract"; -import { httpPaymentRelay } from "./relay"; +import { + httpPaymentRelay, + isRetryableRelayError, + RelayHttpError, +} from "./relay"; const REQUEST: RelayRequest = { injectionRef: "inj_fixture_shopify", @@ -251,6 +255,34 @@ describe("httpPaymentRelay", () => { controller.abort(); await expectation; }); + + it("retries only router gateway statuses", async () => { + const statuses = [502, 503, 504, 500, 401]; + const server = await listen((_req, res) => { + const status = statuses.shift() ?? 500; + res.writeHead(status); + res.end(); + }); + try { + const relay = httpPaymentRelay({ + baseUrl: origin(server), + apiKey: "test-key", + transactionId: "tx_fixture_shopify", + }); + for (const status of [502, 503, 504, 500, 401]) { + const error = await relay + .relay(REQUEST) + .catch((caught: unknown) => caught); + expect(error).toBeInstanceOf(RelayHttpError); + expect((error as RelayHttpError).status).toBe(status); + expect(isRetryableRelayError(error)).toBe( + status === 502 || status === 503 || status === 504, + ); + } + } finally { + await new Promise((resolve) => server.close(resolve)); + } + }); }); function stalledBody( diff --git a/packages/browser-pay/src/relay.ts b/packages/browser-pay/src/relay.ts index 697aa6c75..1cd25d52b 100644 --- a/packages/browser-pay/src/relay.ts +++ b/packages/browser-pay/src/relay.ts @@ -33,6 +33,30 @@ export function isRelayTimeout(error: unknown): boolean { return error instanceof RelayTimeoutError || isAbortError(error); } +const ROUTER_STATUS = new Set([502, 503, 504]); + +export class RelayHttpError extends Error { + readonly status: number; + constructor(status: number) { + super(`Payment relay failed (HTTP ${status})`); + this.name = "RelayHttpError"; + this.status = status; + } +} + +export class RelayTransportError extends Error { + constructor() { + super("Payment relay failed"); + this.name = "RelayTransportError"; + } +} + +export function isRetryableRelayError(error: unknown): boolean { + if (isRelayTimeout(error)) return true; + if (error instanceof RelayTransportError) return true; + return error instanceof RelayHttpError && ROUTER_STATUS.has(error.status); +} + function isAbortError(error: unknown): boolean { return ( typeof error === "object" && @@ -104,7 +128,7 @@ function requireHttpOptions(options: HttpRelayOptions): { function transportError(error: unknown, signal?: AbortSignal): Error { if (error instanceof RelayTimeoutError) return error; if (signal?.aborted || isAbortError(error)) return new RelayTimeoutError(); - return new Error("Payment relay failed"); + return new RelayTransportError(); } async function discardBody(response: Response): Promise { @@ -137,7 +161,7 @@ export function httpPaymentRelay(options: HttpRelayOptions): PaymentRelay { if (response.status >= 300 && response.status < 400) { throw new Error("Payment relay refused a redirect"); } - throw new Error(`Payment relay failed (HTTP ${response.status})`); + throw new RelayHttpError(response.status); } let text: string; try {