From 0fdb95f3dfea7e4dfa61b842b5cdeaf121b8b944 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 21:18:17 +0000 Subject: [PATCH 1/9] fix(browser-pay): report why an intercept arm failed Callers only saw intercept_unavailable when a target could not be armed. The failed event now carries the CDP method, numeric code, timeout flag, target type, and origin. closeTarget still runs. Remote error text is not included. A nested setAutoAttach failure, and an attachToTarget failure on an iframe that would have been armed, now close that target too. Captcha, extension, and worker targets stay open. Co-authored-by: Rimmely Ewan --- .../browser-pay-intercept-diagnostics.md | 5 + packages/browser-pay/README.md | 4 + packages/browser-pay/src/cdp.ts | 42 +- .../src/interceptor.event-redaction.test.ts | 15 +- .../src/interceptor.protocol.test.ts | 409 +++++++++++++++++- packages/browser-pay/src/interceptor.ts | 163 +++++-- 6 files changed, 592 insertions(+), 46 deletions(-) create mode 100644 .changeset/browser-pay-intercept-diagnostics.md diff --git a/.changeset/browser-pay-intercept-diagnostics.md b/.changeset/browser-pay-intercept-diagnostics.md new file mode 100644 index 000000000..80c9fa3a1 --- /dev/null +++ b/.changeset/browser-pay-intercept-diagnostics.md @@ -0,0 +1,5 @@ +--- +"@sapiom/browser-pay": patch +--- + +`failed` events with reason `intercept_unavailable` include the CDP method, numeric error code, timeout flag, target type, and origin. The target is still closed. A nested `Target.setAutoAttach` failure, and a `Target.attachToTarget` failure on an iframe that would have been armed, now close that target too. Captcha widgets, extension pages, and workers stay open. Remote error text is not included. diff --git a/packages/browser-pay/README.md b/packages/browser-pay/README.md index d8727932d..034b454b6 100644 --- a/packages/browser-pay/README.md +++ b/packages/browser-pay/README.md @@ -87,6 +87,10 @@ const handle = await attachInterceptor(cdpUrl, { Call `handle.on` after `attachInterceptor` resolves. Events emitted while attach was running, including a target that could not arm Fetch, are delivered to that first subscriber. +A `failed` event with reason `intercept_unavailable` means that target could not be armed. The interceptor closes it with `Target.closeTarget` and does not resume it. That close also runs when `Target.attachToTarget` fails for an existing iframe that would have been armed, and when nested `Target.setAutoAttach` fails on a page or iframe that would have been armed. Captcha widgets, extension pages, and workers stay open. + +`method` is the CDP method that failed: `Fetch.enable`, `Target.attachToTarget`, or nested `Target.setAutoAttach`. `code` is the numeric CDP error code when the browser sent one, and is absent when it did not. `timedOut` is true when that command hit its timeout, and false when it did not, including a dropped connection with no `code`. `targetType` is `page` or `iframe`. `origin` is the scheme, host, and port only, the same cut as `url`, with no path, query, fragment, or userinfo. The browser's error text is not on the event. + A request is relayed when the held placeholder number appears in the URL or body. An exact match counts, and so do digits split by spaces or dashes, including the URL-encoded and JSON-escaped forms of those separators. `placeholder` is `{ number, injectionRef }`. `injectionRef` is a UUID. The backend's `checkPlaceholderUse` decides whether the number is valid. A paused request that does not hold the placeholder is continued. A relayed response fails the paused request only when that response contains the held placeholder number. diff --git a/packages/browser-pay/src/cdp.ts b/packages/browser-pay/src/cdp.ts index f8ae53cc9..d66e1b792 100644 --- a/packages/browser-pay/src/cdp.ts +++ b/packages/browser-pay/src/cdp.ts @@ -18,11 +18,33 @@ export interface CdpEvent { } type Pending = { + method: string; resolve: (value: unknown) => void; reject: (error: Error) => void; timer: NodeJS.Timeout; }; +export class CdpCommandError extends Error { + readonly method: string; + readonly code: number | undefined; + readonly timedOut: boolean; + + constructor(method: string, details: { code?: number; timedOut: boolean }) { + super(details.timedOut ? "CDP command timed out" : "CDP command failed"); + this.name = "CdpCommandError"; + this.method = method; + this.code = details.code; + this.timedOut = details.timedOut; + } +} + +function readErrorCode(error: unknown): number | undefined { + if (!isRecord(error)) return undefined; + const code = error.code; + if (typeof code !== "number" || !Number.isFinite(code)) return undefined; + return code; +} + function parseCdpMessage(value: unknown): | { id?: number; @@ -31,6 +53,7 @@ function parseCdpMessage(value: unknown): sessionId?: string; result?: unknown; failed?: true; + code?: number; } | undefined { if (!isRecord(value)) return undefined; @@ -40,14 +63,19 @@ function parseCdpMessage(value: unknown): const params = isRecord(value.params) ? value.params : {}; const sessionId = typeof value.sessionId === "string" ? value.sessionId : undefined; - // The remote error text is dropped here so it can never reach a caller. + const code = value.error ? readErrorCode(value.error) : undefined; return { ...(id !== undefined ? { id } : {}), ...(method !== undefined ? { method } : {}), params, ...(sessionId ? { sessionId } : {}), result: value.result, - ...(value.error ? { failed: true as const } : {}), + ...(value.error + ? { + failed: true as const, + ...(code !== undefined ? { code } : {}), + } + : {}), }; } @@ -339,7 +367,12 @@ export class CdpConnection { this.pending.delete(message.id); clearTimeout(pending.timer); if (message.failed) { - pending.reject(new Error("CDP command failed")); + pending.reject( + new CdpCommandError(pending.method, { + ...(message.code !== undefined ? { code: message.code } : {}), + timedOut: false, + }), + ); } else { pending.resolve(message.result); } @@ -390,9 +423,10 @@ export class CdpConnection { } const timer = setTimeout(() => { this.pending.delete(id); - reject(new Error("CDP command timed out")); + reject(new CdpCommandError(method, { timedOut: true })); }, timeoutMs); this.pending.set(id, { + method, resolve, reject, timer, diff --git a/packages/browser-pay/src/interceptor.event-redaction.test.ts b/packages/browser-pay/src/interceptor.event-redaction.test.ts index 1464c3cfd..c8db6466b 100644 --- a/packages/browser-pay/src/interceptor.event-redaction.test.ts +++ b/packages/browser-pay/src/interceptor.event-redaction.test.ts @@ -2,7 +2,7 @@ import crypto from "node:crypto"; import http from "node:http"; import type { AddressInfo } from "node:net"; import type { Duplex } from "node:stream"; -import { CdpConnection } from "./cdp.js"; +import { CdpCommandError, CdpConnection } from "./cdp.js"; import { textHoldsPlaceholderNumber } from "./detect"; import type { PayLogger } from "./logger"; import { @@ -602,8 +602,13 @@ describe("interceptor event redaction", () => { type: "failed", url: ORIGIN, reason: "intercept_unavailable", + method: "Fetch.enable", + timedOut: false, + targetType: "page", + origin: ORIGIN, }, ]); + expect(JSON.stringify(events)).not.toContain("Fetch is unsupported"); assertNoMarkers(events); await handle.detach(); } finally { @@ -702,6 +707,14 @@ describe("interceptor event redaction", () => { ); expect(error.message).toBe("CDP command failed"); expect(error.message).not.toContain("SYNTHETIC_METHOD_SECRET"); + expect(error).toBeInstanceOf(CdpCommandError); + const encoded = JSON.stringify(error, Object.getOwnPropertyNames(error)); + expect(encoded).not.toContain("peer rejected"); + if (error instanceof CdpCommandError) { + expect(error.method).toBe(method); + expect(error.timedOut).toBe(false); + expect(error.code).toBeUndefined(); + } } finally { errorConnection.close(); await errorStub.close(); diff --git a/packages/browser-pay/src/interceptor.protocol.test.ts b/packages/browser-pay/src/interceptor.protocol.test.ts index 1bbe10b5e..c8448bf42 100644 --- a/packages/browser-pay/src/interceptor.protocol.test.ts +++ b/packages/browser-pay/src/interceptor.protocol.test.ts @@ -27,9 +27,18 @@ interface Command { sessionId?: string; } +interface CommandError { + message: string; + code?: number; +} + interface StubOptions { failAutoAttach?: boolean; failFetch?: boolean; + fetchError?: CommandError; + hangFetch?: boolean; + failNestedAutoAttach?: CommandError; + failAttach?: CommandError; failFulfill?: boolean; holdFulfill?: boolean; postData?: string; @@ -63,7 +72,7 @@ async function startStub(options: StubOptions = {}): Promise<{ const chunks: Buffer[] = []; let buffered = 0; - const reply = (id: number, result: unknown, error?: { message: string }) => { + const reply = (id: number, result: unknown, error?: CommandError) => { const message = error ? { id, error } : { id, result }; socket?.write(serverFrame(JSON.stringify(message))); }; @@ -117,18 +126,38 @@ async function startStub(options: StubOptions = {}): Promise<{ params: message.params ?? {}, ...(message.sessionId ? { sessionId: message.sessionId } : {}), }); + if ( + message.method === "Target.setAutoAttach" && + message.sessionId && + options.failNestedAutoAttach + ) { + reply(message.id, undefined, options.failNestedAutoAttach); + return; + } if (message.method === "Target.setAutoAttach" && options.failAutoAttach) { reply(message.id, undefined, { message: "auto-attach refused" }); return; } - if (message.method === "Fetch.enable" && options.failFetch) { - reply(message.id, undefined, { message: "Fetch is unsupported" }); + if (message.method === "Fetch.enable" && options.hangFetch) return; + if ( + message.method === "Fetch.enable" && + (options.failFetch || options.fetchError) + ) { + reply( + message.id, + undefined, + options.fetchError ?? { message: "Fetch is unsupported" }, + ); return; } if (message.method === "Target.getTargets") { reply(message.id, { targetInfos: options.targets ?? [] }); return; } + if (message.method === "Target.attachToTarget" && options.failAttach) { + reply(message.id, undefined, options.failAttach); + return; + } if (message.method === "Target.attachToTarget") { reply(message.id, { sessionId: options.attachedSessionId ?? "iframe-existing", @@ -237,8 +266,12 @@ function awaitListen(server: http.Server): Promise { }); } -async function waitFor(predicate: () => boolean, label: string): Promise { - const deadline = Date.now() + 3_000; +async function waitFor( + predicate: () => boolean, + label: string, + timeoutMs = 3_000, +): Promise { + const deadline = Date.now() + timeoutMs; while (!predicate()) { if (Date.now() > deadline) throw new Error(`timed out waiting for ${label}`); @@ -348,7 +381,369 @@ describe("interceptor CDP decisions", () => { type: "failed", url: "https://shop.example", reason: "intercept_unavailable", + method: "Fetch.enable", + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }); + await handle.detach(); + } finally { + await stub.close(); + } + }); + + it("reports the CDP code when Fetch.enable fails and still closes the target", async () => { + const remote = "REMOTE_CDP_TEXT_SHOULD_NOT_LEAK"; + const pageUrl = + "https://user:secret@shop.example:8443/checkout?pan=4242424242424242#cvv"; + const stub = await startStub({ + fetchError: { code: -32000, message: remote }, + }); + const events: InterceptorEvent[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + }); + handle.on((event) => events.push(event)); + stub.emit({ + method: "Target.attachedToTarget", + params: { + sessionId: "page-1", + waitingForDebugger: true, + targetInfo: { + targetId: "target-page", + type: "page", + url: pageUrl, + }, + }, + }); + await waitFor( + () => + stub.commands.some( + (command) => command.method === "Target.closeTarget", + ), + "closeTarget", + ); + expect(stub.commands).toContainEqual( + expect.objectContaining({ + method: "Target.closeTarget", + params: { targetId: "target-page" }, + }), + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Runtime.runIfWaitingForDebugger", + ); + const failed = events.find((event) => event.type === "failed"); + expect(failed).toEqual({ + type: "failed", + url: "https://shop.example:8443", + reason: "intercept_unavailable", + method: "Fetch.enable", + code: -32000, + timedOut: false, + targetType: "page", + origin: "https://shop.example:8443", + }); + const encoded = JSON.stringify(events); + expect(encoded).not.toContain(remote); + expect(encoded).not.toContain("secret"); + expect(encoded).not.toContain("4242424242424242"); + expect(encoded).not.toContain("checkout"); + expect(encoded).not.toContain("cvv"); + await handle.detach(); + } finally { + await stub.close(); + } + }); + + it("reports a Fetch.enable timeout and still closes the target", async () => { + const pageUrl = + "https://user:secret@shop.example:8443/checkout?pan=4242424242424242#cvv"; + const stub = await startStub({ hangFetch: true }); + const events: InterceptorEvent[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + }); + handle.on((event) => events.push(event)); + stub.emit({ + method: "Target.attachedToTarget", + params: { + sessionId: "page-1", + waitingForDebugger: true, + targetInfo: { + targetId: "target-page", + type: "page", + url: pageUrl, + }, + }, + }); + await waitFor( + () => + stub.commands.some( + (command) => command.method === "Target.closeTarget", + ), + "closeTarget", + 25_000, + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Runtime.runIfWaitingForDebugger", + ); + expect(stub.commands).toContainEqual( + expect.objectContaining({ + method: "Target.closeTarget", + params: { targetId: "target-page" }, + }), + ); + const failed = events.find((event) => event.type === "failed"); + expect(failed).toEqual({ + type: "failed", + url: "https://shop.example:8443", + reason: "intercept_unavailable", + method: "Fetch.enable", + timedOut: true, + targetType: "page", + origin: "https://shop.example:8443", }); + expect(failed).not.toHaveProperty("code"); + const encoded = JSON.stringify(events); + expect(encoded).not.toContain("CDP command timed out"); + expect(encoded).not.toContain("secret"); + expect(encoded).not.toContain("4242424242424242"); + expect(encoded).not.toContain("checkout"); + expect(encoded).not.toContain("cvv"); + await handle.detach(); + } finally { + await stub.close(); + } + }, 40_000); + + it("closes the page when nested Target.setAutoAttach fails", async () => { + const remote = "NESTED_REMOTE_TEXT_SHOULD_NOT_LEAK"; + const stub = await startStub({ + failNestedAutoAttach: { code: -32601, message: remote }, + }); + const events: InterceptorEvent[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + }); + handle.on((event) => events.push(event)); + attachPage(stub.emit); + await waitFor( + () => + stub.commands.some( + (command) => command.method === "Target.closeTarget", + ), + "closeTarget", + ); + expect(stub.commands).toContainEqual( + expect.objectContaining({ + method: "Target.closeTarget", + params: { targetId: "target-page" }, + }), + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Fetch.enable", + ); + stub.emit({ + method: "Target.targetInfoChanged", + params: { + targetInfo: { + targetId: "target-page", + url: "https://shop.example/pay", + }, + }, + }); + stub.emit({ + method: "Target.attachedToTarget", + params: { + sessionId: "page-2", + waitingForDebugger: true, + targetInfo: { + targetId: "target-page", + type: "page", + url: "https://shop.example/pay", + }, + }, + }); + await waitFor( + () => + stub.commands.some( + (command) => + command.method === "Target.detachFromTarget" && + command.params.sessionId === "page-2", + ), + "detach duplicate", + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Fetch.enable", + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Runtime.runIfWaitingForDebugger", + ); + expect(events).toContainEqual({ + type: "failed", + url: "https://shop.example", + reason: "intercept_unavailable", + method: "Target.setAutoAttach", + code: -32601, + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }); + expect(JSON.stringify(events)).not.toContain(remote); + await handle.detach(); + } finally { + await stub.close(); + } + }); + + it("closes an existing iframe when Target.attachToTarget fails", async () => { + const remote = "ATTACH_REMOTE_TEXT_SHOULD_NOT_LEAK"; + const stub = await startStub({ + failAttach: { code: -32000, message: remote }, + targets: [ + { + targetId: "iframe-existing", + type: "iframe", + url: "https://user:secret@checkout.example:8443/card?pan=4242424242424242#cvv", + }, + ], + }); + const events: InterceptorEvent[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + }); + handle.on((event) => events.push(event)); + expect(stub.commands).toContainEqual( + expect.objectContaining({ + method: "Target.closeTarget", + params: { targetId: "iframe-existing" }, + }), + ); + expect(events).toContainEqual({ + type: "failed", + url: "https://checkout.example:8443", + reason: "intercept_unavailable", + method: "Target.attachToTarget", + code: -32000, + timedOut: false, + targetType: "iframe", + origin: "https://checkout.example:8443", + }); + const encoded = JSON.stringify(events); + expect(encoded).not.toContain(remote); + expect(encoded).not.toContain("secret"); + expect(encoded).not.toContain("4242424242424242"); + expect(encoded).not.toContain("cvv"); + await handle.detach(); + } finally { + await stub.close(); + } + }); + + it("closes an existing iframe when nested Target.setAutoAttach fails", async () => { + const remote = "EXISTING_NESTED_REMOTE_TEXT_SHOULD_NOT_LEAK"; + const stub = await startStub({ + failNestedAutoAttach: { code: -32601, message: remote }, + targets: [ + { + targetId: "iframe-existing", + type: "iframe", + url: "https://checkout.example/card", + }, + ], + }); + const events: InterceptorEvent[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + }); + handle.on((event) => events.push(event)); + expect(stub.commands).toContainEqual( + expect.objectContaining({ + method: "Target.closeTarget", + params: { targetId: "iframe-existing" }, + }), + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Fetch.enable", + ); + expect(events).toContainEqual({ + type: "failed", + url: "https://checkout.example", + reason: "intercept_unavailable", + method: "Target.setAutoAttach", + code: -32601, + timedOut: false, + targetType: "iframe", + origin: "https://checkout.example", + }); + expect(JSON.stringify(events)).not.toContain(remote); + await handle.detach(); + } finally { + await stub.close(); + } + }); + + it("does not close a captcha iframe when nested auto-attach fails", async () => { + const stub = await startStub({ + failNestedAutoAttach: { + code: -32000, + message: "NESTED_CAPTCHA_SHOULD_NOT_LEAK", + }, + }); + const events: InterceptorEvent[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + }); + handle.on((event) => events.push(event)); + stub.emit({ + method: "Target.attachedToTarget", + params: { + sessionId: "captcha-1", + waitingForDebugger: true, + targetInfo: { + targetId: "target-captcha", + type: "iframe", + url: "https://hcaptcha.com/captcha", + }, + }, + }); + await waitFor( + () => + stub.commands.some( + (command) => + command.method === "Runtime.runIfWaitingForDebugger" && + command.sessionId === "captcha-1", + ), + "resume captcha", + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Target.closeTarget", + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Fetch.enable", + ); + expect(events).toEqual([]); + expect(JSON.stringify(stub.commands)).not.toContain( + "NESTED_CAPTCHA_SHOULD_NOT_LEAK", + ); await handle.detach(); } finally { await stub.close(); @@ -1561,6 +1956,10 @@ describe("interceptor CDP decisions", () => { type: "failed", url: "https://checkout.example", reason: "intercept_unavailable", + method: "Fetch.enable", + timedOut: false, + targetType: "iframe", + origin: "https://checkout.example", }); await handle.detach(); } finally { diff --git a/packages/browser-pay/src/interceptor.ts b/packages/browser-pay/src/interceptor.ts index cb435f73c..d1b7026e5 100644 --- a/packages/browser-pay/src/interceptor.ts +++ b/packages/browser-pay/src/interceptor.ts @@ -1,4 +1,4 @@ -import { CdpConnection, type CdpEvent } from "./cdp.js"; +import { CdpCommandError, CdpConnection, type CdpEvent } from "./cdp.js"; import { paymentIdempotencyKey, type RelayedHttpResponse, @@ -88,7 +88,29 @@ export type InterceptorEvent = | { type: "relayed"; url: string; status: number } | { type: "relayed_unfulfilled"; url: string; status: number } | { type: "refused"; url: string; reason: RefusalReason } - | { type: "failed"; url: string; reason: FailureReason }; + | { + type: "failed"; + url: string; + reason: FailureReason; + method?: + | "Fetch.enable" + | "Target.attachToTarget" + | "Target.setAutoAttach"; + code?: number; + timedOut?: boolean; + targetType?: "page" | "iframe"; + origin?: string; + }; + +type ArmMethod = NonNullable< + Extract["method"] +>; + +interface ArmFailure { + method: ArmMethod; + code: number | undefined; + timedOut: boolean; +} export interface InterceptorHandle { detach(): Promise; @@ -192,6 +214,7 @@ interface Session { url: string; parentSessionId?: string; fetchEnabled: boolean; + unarmed?: boolean; } // Blank frames stay armed. Disarming about:blank and arming again on the @@ -341,12 +364,16 @@ class PaymentInterceptor { const info = params.targetInfo; if (this.byTarget.has(info.targetId)) { // setAutoAttach already owns this target. A second session would pause - // every request twice. - void this.resumeIfWaiting(sessionId, params.waitingForDebugger).then(() => - this.connection + // every request twice. A target that failed closed stays paused. + const owned = this.byTarget.get(info.targetId); + void (async () => { + if (!owned?.unarmed) { + await this.resumeIfWaiting(sessionId, params.waitingForDebugger); + } + await this.connection .send("Target.detachFromTarget", { sessionId }) - .catch(() => {}), - ); + .catch(() => {}); + })(); return; } const session = this.trackSession({ @@ -358,10 +385,7 @@ class PaymentInterceptor { ...(event.sessionId ? { parentSessionId: event.sessionId } : {}), }); const arm = (async () => { - if (info.type === "page" || info.type === "iframe") { - await this.autoAttachChildren(sessionId); - } - await this.enableSession(session.sessionId, params.waitingForDebugger); + await this.armAttached(session, params.waitingForDebugger); })(); this.pendingArms.add(arm); void arm.finally(() => { @@ -385,7 +409,7 @@ class PaymentInterceptor { const info = parseTargetInfoChange(event.params); if (!info) return; const session = this.byTarget.get(info.targetId); - if (!session) return; + if (!session || session.unarmed) return; session.url = info.url; if (!shouldArm(session)) { this.disarm(session); @@ -425,24 +449,58 @@ class PaymentInterceptor { await this.resumeIfWaiting(sessionId, waitingForDebugger); } + private async armAttached( + session: Session, + waitingForDebugger: boolean, + ): Promise { + if (session.type === "page" || session.type === "iframe") { + const nested = await this.autoAttachChildren(session.sessionId); + if (nested && shouldArm(session)) { + await this.closeUnarmed(session, nested); + return; + } + } + await this.enableSession(session.sessionId, waitingForDebugger); + } + private async armOrClose(session: Session): Promise { - const armed = await armFetch(this.connection, session); - if (armed) return true; + const cause = await armFetch(this.connection, session); + if (!cause) return true; + return this.closeUnarmed(session, cause); + } + + private async closeUnarmed( + target: Pick, + cause: ArmFailure, + ): Promise { + const tracked = this.byTarget.get(target.targetId); + if (tracked) tracked.unarmed = true; if (this.closed) return false; + const targetType = + target.type === "page" || target.type === "iframe" + ? target.type + : undefined; this.emit({ type: "failed", - url: session.url, + url: target.url, reason: "intercept_unavailable", + method: cause.method, + ...(cause.code !== undefined ? { code: cause.code } : {}), + timedOut: cause.timedOut, + ...(targetType ? { targetType } : {}), + origin: target.url, }); await this.connection - .send("Target.closeTarget", { targetId: session.targetId }) + .send("Target.closeTarget", { targetId: target.targetId }) .catch(() => {}); return false; } - private async autoAttachChildren(sessionId: string): Promise { - await this.connection - .send( + private async autoAttachChildren( + sessionId: string, + ): Promise { + try { + await this.connection.send( "Target.setAutoAttach", { autoAttach: true, @@ -451,8 +509,11 @@ class PaymentInterceptor { filter: CHILD_FILTER, }, sessionId, - ) - .catch(() => {}); + ); + return undefined; + } catch (error) { + return armFailure(error, "Target.setAutoAttach"); + } } private async resumeIfWaiting( @@ -472,12 +533,21 @@ class PaymentInterceptor { for (const info of parseTargetInfos(listed)) { if (info.type !== "iframe") continue; if (this.byTarget.has(info.targetId)) continue; - const attached = await this.connection - .send("Target.attachToTarget", { + let attached: unknown; + try { + attached = await this.connection.send("Target.attachToTarget", { targetId: info.targetId, flatten: true, - }) - .catch(() => undefined); + }); + } catch (error) { + if (!this.byTarget.has(info.targetId) && armable(info.type, info.url)) { + await this.closeUnarmed( + { targetId: info.targetId, type: info.type, url: info.url }, + armFailure(error, "Target.attachToTarget"), + ); + } + continue; + } const sessionId = readAttachedSessionId(attached); if (!sessionId || this.sessions.has(sessionId)) continue; const session = this.trackSession({ @@ -487,8 +557,7 @@ class PaymentInterceptor { url: info.url, fetchEnabled: false, }); - await this.autoAttachChildren(sessionId); - await this.enableSession(session.sessionId, false); + await this.armAttached(session, false); } } @@ -878,10 +947,16 @@ class PaymentInterceptor { }; } - // Callers pass the raw request URL. It is cut to the origin here, once, so - // no event leaves with a path, query, or held number. private emit(event: InterceptorEvent): void { - const safe = { ...event, url: eventUrl(event.url, this.number) }; + const number = this.number; + const safe: InterceptorEvent = + event.type === "failed" && event.origin !== undefined + ? { + ...event, + url: eventUrl(event.url, number), + origin: eventUrl(event.origin, number), + } + : { ...event, url: eventUrl(event.url, number) }; if (this.buffering && this.backlog) { this.backlog.push(safe); return; @@ -1254,16 +1329,32 @@ function isMultipart(headers: unknown): boolean { .includes("multipart/"); } -// The retrieved body keeps a file part's filename header and drops its bytes. function multipartOmitsFileBytes(body: string): boolean { return /content-disposition:[^\r\n]*[;\s]filename\s*\*?=/i.test(body); } +function armable(type: string, url: string): boolean { + return shouldArm({ + sessionId: "", + targetId: "", + type, + url, + fetchEnabled: false, + }); +} + +function armFailure(error: unknown, method: ArmMethod): ArmFailure { + if (error instanceof CdpCommandError) { + return { method, code: error.code, timedOut: error.timedOut }; + } + return { method, code: undefined, timedOut: false }; +} + async function armFetch( connection: CdpConnection, session: Session, -): Promise { - if (session.fetchEnabled) return true; +): Promise { + if (session.fetchEnabled) return undefined; try { await connection .send("Network.enable", {}, session.sessionId) @@ -1273,11 +1364,11 @@ async function armFetch( { patterns: [{ urlPattern: "*", requestStage: "Request" }] }, session.sessionId, ); - } catch { - return false; + } catch (error) { + return armFailure(error, "Fetch.enable"); } session.fetchEnabled = true; - return true; + return undefined; } async function continueRequest( From d21a932c7ce9921485d52da267edddcadcab5f9e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 21:25:33 +0000 Subject: [PATCH 2/9] fix(browser-pay): limit intercept diagnostics to armFetch The failed event reports only the CDP methods armFetch can fail on, plus the integer code, timeout flag, target type, and origin. closeTarget is unchanged. Nested attach failures stay swallowed. Co-authored-by: Rimmely Ewan --- .../browser-pay-intercept-diagnostics.md | 2 +- packages/browser-pay/README.md | 4 +- packages/browser-pay/src/cdp.ts | 33 +- .../src/interceptor.event-redaction.test.ts | 11 +- .../src/interceptor.protocol.test.ts | 352 ++---------------- packages/browser-pay/src/interceptor.ts | 161 +++----- 6 files changed, 98 insertions(+), 465 deletions(-) diff --git a/.changeset/browser-pay-intercept-diagnostics.md b/.changeset/browser-pay-intercept-diagnostics.md index 80c9fa3a1..5fa33cf01 100644 --- a/.changeset/browser-pay-intercept-diagnostics.md +++ b/.changeset/browser-pay-intercept-diagnostics.md @@ -2,4 +2,4 @@ "@sapiom/browser-pay": patch --- -`failed` events with reason `intercept_unavailable` include the CDP method, numeric error code, timeout flag, target type, and origin. The target is still closed. A nested `Target.setAutoAttach` failure, and a `Target.attachToTarget` failure on an iframe that would have been armed, now close that target too. Captcha widgets, extension pages, and workers stay open. Remote error text is not included. +`failed` events with reason `intercept_unavailable` include the CDP method, integer error code, timeout flag, target type, and origin. The target is still closed. Remote error text is not included. diff --git a/packages/browser-pay/README.md b/packages/browser-pay/README.md index 034b454b6..08f18fc3a 100644 --- a/packages/browser-pay/README.md +++ b/packages/browser-pay/README.md @@ -87,9 +87,7 @@ const handle = await attachInterceptor(cdpUrl, { Call `handle.on` after `attachInterceptor` resolves. Events emitted while attach was running, including a target that could not arm Fetch, are delivered to that first subscriber. -A `failed` event with reason `intercept_unavailable` means that target could not be armed. The interceptor closes it with `Target.closeTarget` and does not resume it. That close also runs when `Target.attachToTarget` fails for an existing iframe that would have been armed, and when nested `Target.setAutoAttach` fails on a page or iframe that would have been armed. Captcha widgets, extension pages, and workers stay open. - -`method` is the CDP method that failed: `Fetch.enable`, `Target.attachToTarget`, or nested `Target.setAutoAttach`. `code` is the numeric CDP error code when the browser sent one, and is absent when it did not. `timedOut` is true when that command hit its timeout, and false when it did not, including a dropped connection with no `code`. `targetType` is `page` or `iframe`. `origin` is the scheme, host, and port only, the same cut as `url`, with no path, query, fragment, or userinfo. The browser's error text is not on the event. +A `failed` event with reason `intercept_unavailable` closes that target with `Target.closeTarget`. `method` is `Fetch.enable` or `Network.enable`, `code` is the integer CDP error code when the browser sent one, `timedOut` is true when the command timed out, `targetType` is `page` or `iframe`, and `origin` uses the same cut as `url`. An opaque origin such as `about:blank` is emitted as `about:`. A request is relayed when the held placeholder number appears in the URL or body. An exact match counts, and so do digits split by spaces or dashes, including the URL-encoded and JSON-escaped forms of those separators. `placeholder` is `{ number, injectionRef }`. `injectionRef` is a UUID. The backend's `checkPlaceholderUse` decides whether the number is valid. A paused request that does not hold the placeholder is continued. diff --git a/packages/browser-pay/src/cdp.ts b/packages/browser-pay/src/cdp.ts index d66e1b792..b7c47187d 100644 --- a/packages/browser-pay/src/cdp.ts +++ b/packages/browser-pay/src/cdp.ts @@ -18,31 +18,26 @@ export interface CdpEvent { } type Pending = { - method: string; resolve: (value: unknown) => void; reject: (error: Error) => void; timer: NodeJS.Timeout; }; export class CdpCommandError extends Error { - readonly method: string; readonly code: number | undefined; readonly timedOut: boolean; - constructor(method: string, details: { code?: number; timedOut: boolean }) { - super(details.timedOut ? "CDP command timed out" : "CDP command failed"); + constructor(timedOut: boolean, code?: number) { + super(timedOut ? "CDP command timed out" : "CDP command failed"); this.name = "CdpCommandError"; - this.method = method; - this.code = details.code; - this.timedOut = details.timedOut; + this.timedOut = timedOut; + this.code = code; } } function readErrorCode(error: unknown): number | undefined { - if (!isRecord(error)) return undefined; - const code = error.code; - if (typeof code !== "number" || !Number.isFinite(code)) return undefined; - return code; + if (!isRecord(error) || typeof error.code !== "number") return undefined; + return Number.isInteger(error.code) ? error.code : undefined; } function parseCdpMessage(value: unknown): @@ -64,6 +59,7 @@ function parseCdpMessage(value: unknown): const sessionId = typeof value.sessionId === "string" ? value.sessionId : undefined; const code = value.error ? readErrorCode(value.error) : undefined; + // The remote error text is dropped here so it can never reach a caller. return { ...(id !== undefined ? { id } : {}), ...(method !== undefined ? { method } : {}), @@ -71,10 +67,7 @@ function parseCdpMessage(value: unknown): ...(sessionId ? { sessionId } : {}), result: value.result, ...(value.error - ? { - failed: true as const, - ...(code !== undefined ? { code } : {}), - } + ? { failed: true as const, ...(code !== undefined ? { code } : {}) } : {}), }; } @@ -367,12 +360,7 @@ export class CdpConnection { this.pending.delete(message.id); clearTimeout(pending.timer); if (message.failed) { - pending.reject( - new CdpCommandError(pending.method, { - ...(message.code !== undefined ? { code: message.code } : {}), - timedOut: false, - }), - ); + pending.reject(new CdpCommandError(false, message.code)); } else { pending.resolve(message.result); } @@ -423,10 +411,9 @@ export class CdpConnection { } const timer = setTimeout(() => { this.pending.delete(id); - reject(new CdpCommandError(method, { timedOut: true })); + reject(new CdpCommandError(true)); }, timeoutMs); this.pending.set(id, { - method, resolve, reject, timer, diff --git a/packages/browser-pay/src/interceptor.event-redaction.test.ts b/packages/browser-pay/src/interceptor.event-redaction.test.ts index c8db6466b..337d8f5f8 100644 --- a/packages/browser-pay/src/interceptor.event-redaction.test.ts +++ b/packages/browser-pay/src/interceptor.event-redaction.test.ts @@ -2,7 +2,7 @@ import crypto from "node:crypto"; import http from "node:http"; import type { AddressInfo } from "node:net"; import type { Duplex } from "node:stream"; -import { CdpCommandError, CdpConnection } from "./cdp.js"; +import { CdpConnection } from "./cdp.js"; import { textHoldsPlaceholderNumber } from "./detect"; import type { PayLogger } from "./logger"; import { @@ -608,7 +608,6 @@ describe("interceptor event redaction", () => { origin: ORIGIN, }, ]); - expect(JSON.stringify(events)).not.toContain("Fetch is unsupported"); assertNoMarkers(events); await handle.detach(); } finally { @@ -707,14 +706,6 @@ describe("interceptor event redaction", () => { ); expect(error.message).toBe("CDP command failed"); expect(error.message).not.toContain("SYNTHETIC_METHOD_SECRET"); - expect(error).toBeInstanceOf(CdpCommandError); - const encoded = JSON.stringify(error, Object.getOwnPropertyNames(error)); - expect(encoded).not.toContain("peer rejected"); - if (error instanceof CdpCommandError) { - expect(error.method).toBe(method); - expect(error.timedOut).toBe(false); - expect(error.code).toBeUndefined(); - } } finally { errorConnection.close(); await errorStub.close(); diff --git a/packages/browser-pay/src/interceptor.protocol.test.ts b/packages/browser-pay/src/interceptor.protocol.test.ts index c8448bf42..1d356af5e 100644 --- a/packages/browser-pay/src/interceptor.protocol.test.ts +++ b/packages/browser-pay/src/interceptor.protocol.test.ts @@ -27,18 +27,11 @@ interface Command { sessionId?: string; } -interface CommandError { - message: string; - code?: number; -} - interface StubOptions { failAutoAttach?: boolean; failFetch?: boolean; - fetchError?: CommandError; - hangFetch?: boolean; - failNestedAutoAttach?: CommandError; - failAttach?: CommandError; + fetchErrorCode?: number; + fetchErrorMessage?: string; failFulfill?: boolean; holdFulfill?: boolean; postData?: string; @@ -72,7 +65,11 @@ async function startStub(options: StubOptions = {}): Promise<{ const chunks: Buffer[] = []; let buffered = 0; - const reply = (id: number, result: unknown, error?: CommandError) => { + const reply = ( + id: number, + result: unknown, + error?: { message: string; code?: number }, + ) => { const message = error ? { id, error } : { id, result }; socket?.write(serverFrame(JSON.stringify(message))); }; @@ -126,38 +123,23 @@ async function startStub(options: StubOptions = {}): Promise<{ params: message.params ?? {}, ...(message.sessionId ? { sessionId: message.sessionId } : {}), }); - if ( - message.method === "Target.setAutoAttach" && - message.sessionId && - options.failNestedAutoAttach - ) { - reply(message.id, undefined, options.failNestedAutoAttach); - return; - } if (message.method === "Target.setAutoAttach" && options.failAutoAttach) { reply(message.id, undefined, { message: "auto-attach refused" }); return; } - if (message.method === "Fetch.enable" && options.hangFetch) return; - if ( - message.method === "Fetch.enable" && - (options.failFetch || options.fetchError) - ) { - reply( - message.id, - undefined, - options.fetchError ?? { message: "Fetch is unsupported" }, - ); + if (message.method === "Fetch.enable" && options.failFetch) { + reply(message.id, undefined, { + message: options.fetchErrorMessage ?? "Fetch is unsupported", + ...(options.fetchErrorCode !== undefined + ? { code: options.fetchErrorCode } + : {}), + }); return; } if (message.method === "Target.getTargets") { reply(message.id, { targetInfos: options.targets ?? [] }); return; } - if (message.method === "Target.attachToTarget" && options.failAttach) { - reply(message.id, undefined, options.failAttach); - return; - } if (message.method === "Target.attachToTarget") { reply(message.id, { sessionId: options.attachedSessionId ?? "iframe-existing", @@ -266,12 +248,8 @@ function awaitListen(server: http.Server): Promise { }); } -async function waitFor( - predicate: () => boolean, - label: string, - timeoutMs = 3_000, -): Promise { - const deadline = Date.now() + timeoutMs; +async function waitFor(predicate: () => boolean, label: string): Promise { + const deadline = Date.now() + 3_000; while (!predicate()) { if (Date.now() > deadline) throw new Error(`timed out waiting for ${label}`); @@ -394,10 +372,10 @@ describe("interceptor CDP decisions", () => { it("reports the CDP code when Fetch.enable fails and still closes the target", async () => { const remote = "REMOTE_CDP_TEXT_SHOULD_NOT_LEAK"; - const pageUrl = - "https://user:secret@shop.example:8443/checkout?pan=4242424242424242#cvv"; const stub = await startStub({ - fetchError: { code: -32000, message: remote }, + failFetch: true, + fetchErrorCode: -32000, + fetchErrorMessage: remote, }); const events: InterceptorEvent[] = []; try { @@ -415,7 +393,7 @@ describe("interceptor CDP decisions", () => { targetInfo: { targetId: "target-page", type: "page", - url: pageUrl, + url: "https://shop.example/checkout?q=1#frag", }, }, }); @@ -432,101 +410,27 @@ describe("interceptor CDP decisions", () => { params: { targetId: "target-page" }, }), ); - expect(stub.commands.map((command) => command.method)).not.toContain( - "Runtime.runIfWaitingForDebugger", - ); const failed = events.find((event) => event.type === "failed"); expect(failed).toEqual({ type: "failed", - url: "https://shop.example:8443", + url: "https://shop.example", reason: "intercept_unavailable", method: "Fetch.enable", code: -32000, timedOut: false, targetType: "page", - origin: "https://shop.example:8443", - }); - const encoded = JSON.stringify(events); - expect(encoded).not.toContain(remote); - expect(encoded).not.toContain("secret"); - expect(encoded).not.toContain("4242424242424242"); - expect(encoded).not.toContain("checkout"); - expect(encoded).not.toContain("cvv"); - await handle.detach(); - } finally { - await stub.close(); - } - }); - - it("reports a Fetch.enable timeout and still closes the target", async () => { - const pageUrl = - "https://user:secret@shop.example:8443/checkout?pan=4242424242424242#cvv"; - const stub = await startStub({ hangFetch: true }); - const events: InterceptorEvent[] = []; - try { - const handle = await attachInterceptor(stub.url, { - relay: stubPaymentRelay(async () => success()), - placeholder: HELD, - profileId: "shopify-card-vault", - }); - handle.on((event) => events.push(event)); - stub.emit({ - method: "Target.attachedToTarget", - params: { - sessionId: "page-1", - waitingForDebugger: true, - targetInfo: { - targetId: "target-page", - type: "page", - url: pageUrl, - }, - }, + origin: "https://shop.example", }); - await waitFor( - () => - stub.commands.some( - (command) => command.method === "Target.closeTarget", - ), - "closeTarget", - 25_000, - ); - expect(stub.commands.map((command) => command.method)).not.toContain( - "Runtime.runIfWaitingForDebugger", - ); - expect(stub.commands).toContainEqual( - expect.objectContaining({ - method: "Target.closeTarget", - params: { targetId: "target-page" }, - }), - ); - const failed = events.find((event) => event.type === "failed"); - expect(failed).toEqual({ - type: "failed", - url: "https://shop.example:8443", - reason: "intercept_unavailable", - method: "Fetch.enable", - timedOut: true, - targetType: "page", - origin: "https://shop.example:8443", - }); - expect(failed).not.toHaveProperty("code"); - const encoded = JSON.stringify(events); - expect(encoded).not.toContain("CDP command timed out"); - expect(encoded).not.toContain("secret"); - expect(encoded).not.toContain("4242424242424242"); - expect(encoded).not.toContain("checkout"); - expect(encoded).not.toContain("cvv"); + expect(JSON.stringify(failed)).not.toContain(remote); + expect(JSON.stringify(failed)).not.toContain("checkout"); await handle.detach(); } finally { await stub.close(); } - }, 40_000); + }); - it("closes the page when nested Target.setAutoAttach fails", async () => { - const remote = "NESTED_REMOTE_TEXT_SHOULD_NOT_LEAK"; - const stub = await startStub({ - failNestedAutoAttach: { code: -32601, message: remote }, - }); + it("emits about: for an about:blank target that cannot arm Fetch", async () => { + const stub = await startStub({ failFetch: true }); const events: InterceptorEvent[] = []; try { const handle = await attachInterceptor(stub.url, { @@ -535,215 +439,31 @@ describe("interceptor CDP decisions", () => { profileId: "shopify-card-vault", }); handle.on((event) => events.push(event)); - attachPage(stub.emit); - await waitFor( - () => - stub.commands.some( - (command) => command.method === "Target.closeTarget", - ), - "closeTarget", - ); - expect(stub.commands).toContainEqual( - expect.objectContaining({ - method: "Target.closeTarget", - params: { targetId: "target-page" }, - }), - ); - expect(stub.commands.map((command) => command.method)).not.toContain( - "Fetch.enable", - ); - stub.emit({ - method: "Target.targetInfoChanged", - params: { - targetInfo: { - targetId: "target-page", - url: "https://shop.example/pay", - }, - }, - }); stub.emit({ method: "Target.attachedToTarget", params: { - sessionId: "page-2", + sessionId: "blank-1", waitingForDebugger: true, targetInfo: { - targetId: "target-page", - type: "page", - url: "https://shop.example/pay", + targetId: "target-blank", + type: "iframe", + url: "about:blank", }, }, }); await waitFor( - () => - stub.commands.some( - (command) => - command.method === "Target.detachFromTarget" && - command.params.sessionId === "page-2", - ), - "detach duplicate", - ); - expect(stub.commands.map((command) => command.method)).not.toContain( - "Fetch.enable", - ); - expect(stub.commands.map((command) => command.method)).not.toContain( - "Runtime.runIfWaitingForDebugger", - ); - expect(events).toContainEqual({ - type: "failed", - url: "https://shop.example", - reason: "intercept_unavailable", - method: "Target.setAutoAttach", - code: -32601, - timedOut: false, - targetType: "page", - origin: "https://shop.example", - }); - expect(JSON.stringify(events)).not.toContain(remote); - await handle.detach(); - } finally { - await stub.close(); - } - }); - - it("closes an existing iframe when Target.attachToTarget fails", async () => { - const remote = "ATTACH_REMOTE_TEXT_SHOULD_NOT_LEAK"; - const stub = await startStub({ - failAttach: { code: -32000, message: remote }, - targets: [ - { - targetId: "iframe-existing", - type: "iframe", - url: "https://user:secret@checkout.example:8443/card?pan=4242424242424242#cvv", - }, - ], - }); - const events: InterceptorEvent[] = []; - try { - const handle = await attachInterceptor(stub.url, { - relay: stubPaymentRelay(async () => success()), - placeholder: HELD, - profileId: "shopify-card-vault", - }); - handle.on((event) => events.push(event)); - expect(stub.commands).toContainEqual( - expect.objectContaining({ - method: "Target.closeTarget", - params: { targetId: "iframe-existing" }, - }), - ); - expect(events).toContainEqual({ - type: "failed", - url: "https://checkout.example:8443", - reason: "intercept_unavailable", - method: "Target.attachToTarget", - code: -32000, - timedOut: false, - targetType: "iframe", - origin: "https://checkout.example:8443", - }); - const encoded = JSON.stringify(events); - expect(encoded).not.toContain(remote); - expect(encoded).not.toContain("secret"); - expect(encoded).not.toContain("4242424242424242"); - expect(encoded).not.toContain("cvv"); - await handle.detach(); - } finally { - await stub.close(); - } - }); - - it("closes an existing iframe when nested Target.setAutoAttach fails", async () => { - const remote = "EXISTING_NESTED_REMOTE_TEXT_SHOULD_NOT_LEAK"; - const stub = await startStub({ - failNestedAutoAttach: { code: -32601, message: remote }, - targets: [ - { - targetId: "iframe-existing", - type: "iframe", - url: "https://checkout.example/card", - }, - ], - }); - const events: InterceptorEvent[] = []; - try { - const handle = await attachInterceptor(stub.url, { - relay: stubPaymentRelay(async () => success()), - placeholder: HELD, - profileId: "shopify-card-vault", - }); - handle.on((event) => events.push(event)); - expect(stub.commands).toContainEqual( - expect.objectContaining({ - method: "Target.closeTarget", - params: { targetId: "iframe-existing" }, - }), - ); - expect(stub.commands.map((command) => command.method)).not.toContain( - "Fetch.enable", + () => events.some((event) => event.type === "failed"), + "about blank", ); expect(events).toContainEqual({ type: "failed", - url: "https://checkout.example", + url: "about:", reason: "intercept_unavailable", - method: "Target.setAutoAttach", - code: -32601, + method: "Fetch.enable", timedOut: false, targetType: "iframe", - origin: "https://checkout.example", - }); - expect(JSON.stringify(events)).not.toContain(remote); - await handle.detach(); - } finally { - await stub.close(); - } - }); - - it("does not close a captcha iframe when nested auto-attach fails", async () => { - const stub = await startStub({ - failNestedAutoAttach: { - code: -32000, - message: "NESTED_CAPTCHA_SHOULD_NOT_LEAK", - }, - }); - const events: InterceptorEvent[] = []; - try { - const handle = await attachInterceptor(stub.url, { - relay: stubPaymentRelay(async () => success()), - placeholder: HELD, - profileId: "shopify-card-vault", + origin: "about:", }); - handle.on((event) => events.push(event)); - stub.emit({ - method: "Target.attachedToTarget", - params: { - sessionId: "captcha-1", - waitingForDebugger: true, - targetInfo: { - targetId: "target-captcha", - type: "iframe", - url: "https://hcaptcha.com/captcha", - }, - }, - }); - await waitFor( - () => - stub.commands.some( - (command) => - command.method === "Runtime.runIfWaitingForDebugger" && - command.sessionId === "captcha-1", - ), - "resume captcha", - ); - expect(stub.commands.map((command) => command.method)).not.toContain( - "Target.closeTarget", - ); - expect(stub.commands.map((command) => command.method)).not.toContain( - "Fetch.enable", - ); - expect(events).toEqual([]); - expect(JSON.stringify(stub.commands)).not.toContain( - "NESTED_CAPTCHA_SHOULD_NOT_LEAK", - ); await handle.detach(); } finally { await stub.close(); diff --git a/packages/browser-pay/src/interceptor.ts b/packages/browser-pay/src/interceptor.ts index d1b7026e5..f3704d39f 100644 --- a/packages/browser-pay/src/interceptor.ts +++ b/packages/browser-pay/src/interceptor.ts @@ -92,26 +92,13 @@ export type InterceptorEvent = type: "failed"; url: string; reason: FailureReason; - method?: - | "Fetch.enable" - | "Target.attachToTarget" - | "Target.setAutoAttach"; + method?: "Fetch.enable" | "Network.enable"; code?: number; timedOut?: boolean; targetType?: "page" | "iframe"; origin?: string; }; -type ArmMethod = NonNullable< - Extract["method"] ->; - -interface ArmFailure { - method: ArmMethod; - code: number | undefined; - timedOut: boolean; -} - export interface InterceptorHandle { detach(): Promise; /** Subscribe to outcomes. Returns an unsubscribe function. */ @@ -214,7 +201,6 @@ interface Session { url: string; parentSessionId?: string; fetchEnabled: boolean; - unarmed?: boolean; } // Blank frames stay armed. Disarming about:blank and arming again on the @@ -364,16 +350,12 @@ class PaymentInterceptor { const info = params.targetInfo; if (this.byTarget.has(info.targetId)) { // setAutoAttach already owns this target. A second session would pause - // every request twice. A target that failed closed stays paused. - const owned = this.byTarget.get(info.targetId); - void (async () => { - if (!owned?.unarmed) { - await this.resumeIfWaiting(sessionId, params.waitingForDebugger); - } - await this.connection + // every request twice. + void this.resumeIfWaiting(sessionId, params.waitingForDebugger).then(() => + this.connection .send("Target.detachFromTarget", { sessionId }) - .catch(() => {}); - })(); + .catch(() => {}), + ); return; } const session = this.trackSession({ @@ -385,7 +367,10 @@ class PaymentInterceptor { ...(event.sessionId ? { parentSessionId: event.sessionId } : {}), }); const arm = (async () => { - await this.armAttached(session, params.waitingForDebugger); + if (info.type === "page" || info.type === "iframe") { + await this.autoAttachChildren(sessionId); + } + await this.enableSession(session.sessionId, params.waitingForDebugger); })(); this.pendingArms.add(arm); void arm.finally(() => { @@ -409,7 +394,7 @@ class PaymentInterceptor { const info = parseTargetInfoChange(event.params); if (!info) return; const session = this.byTarget.get(info.targetId); - if (!session || session.unarmed) return; + if (!session) return; session.url = info.url; if (!shouldArm(session)) { this.disarm(session); @@ -449,58 +434,33 @@ class PaymentInterceptor { await this.resumeIfWaiting(sessionId, waitingForDebugger); } - private async armAttached( - session: Session, - waitingForDebugger: boolean, - ): Promise { - if (session.type === "page" || session.type === "iframe") { - const nested = await this.autoAttachChildren(session.sessionId); - if (nested && shouldArm(session)) { - await this.closeUnarmed(session, nested); - return; - } - } - await this.enableSession(session.sessionId, waitingForDebugger); - } - private async armOrClose(session: Session): Promise { - const cause = await armFetch(this.connection, session); - if (!cause) return true; - return this.closeUnarmed(session, cause); - } - - private async closeUnarmed( - target: Pick, - cause: ArmFailure, - ): Promise { - const tracked = this.byTarget.get(target.targetId); - if (tracked) tracked.unarmed = true; + const failure = await armFetch(this.connection, session); + if (!failure) return true; if (this.closed) return false; const targetType = - target.type === "page" || target.type === "iframe" - ? target.type + session.type === "page" || session.type === "iframe" + ? session.type : undefined; this.emit({ type: "failed", - url: target.url, + url: session.url, reason: "intercept_unavailable", - method: cause.method, - ...(cause.code !== undefined ? { code: cause.code } : {}), - timedOut: cause.timedOut, + method: failure.method, + ...(failure.code !== undefined ? { code: failure.code } : {}), + timedOut: failure.timedOut, ...(targetType ? { targetType } : {}), - origin: target.url, + origin: eventUrl(session.url, this.number), }); await this.connection - .send("Target.closeTarget", { targetId: target.targetId }) + .send("Target.closeTarget", { targetId: session.targetId }) .catch(() => {}); return false; } - private async autoAttachChildren( - sessionId: string, - ): Promise { - try { - await this.connection.send( + private async autoAttachChildren(sessionId: string): Promise { + await this.connection + .send( "Target.setAutoAttach", { autoAttach: true, @@ -509,11 +469,8 @@ class PaymentInterceptor { filter: CHILD_FILTER, }, sessionId, - ); - return undefined; - } catch (error) { - return armFailure(error, "Target.setAutoAttach"); - } + ) + .catch(() => {}); } private async resumeIfWaiting( @@ -533,21 +490,12 @@ class PaymentInterceptor { for (const info of parseTargetInfos(listed)) { if (info.type !== "iframe") continue; if (this.byTarget.has(info.targetId)) continue; - let attached: unknown; - try { - attached = await this.connection.send("Target.attachToTarget", { + const attached = await this.connection + .send("Target.attachToTarget", { targetId: info.targetId, flatten: true, - }); - } catch (error) { - if (!this.byTarget.has(info.targetId) && armable(info.type, info.url)) { - await this.closeUnarmed( - { targetId: info.targetId, type: info.type, url: info.url }, - armFailure(error, "Target.attachToTarget"), - ); - } - continue; - } + }) + .catch(() => undefined); const sessionId = readAttachedSessionId(attached); if (!sessionId || this.sessions.has(sessionId)) continue; const session = this.trackSession({ @@ -557,7 +505,8 @@ class PaymentInterceptor { url: info.url, fetchEnabled: false, }); - await this.armAttached(session, false); + await this.autoAttachChildren(sessionId); + await this.enableSession(session.sessionId, false); } } @@ -947,16 +896,10 @@ class PaymentInterceptor { }; } + // Callers pass the raw request URL. It is cut to the origin here, once, so + // no event leaves with a path, query, or held number. private emit(event: InterceptorEvent): void { - const number = this.number; - const safe: InterceptorEvent = - event.type === "failed" && event.origin !== undefined - ? { - ...event, - url: eventUrl(event.url, number), - origin: eventUrl(event.origin, number), - } - : { ...event, url: eventUrl(event.url, number) }; + const safe = { ...event, url: eventUrl(event.url, this.number) }; if (this.buffering && this.backlog) { this.backlog.push(safe); return; @@ -1329,32 +1272,22 @@ function isMultipart(headers: unknown): boolean { .includes("multipart/"); } +// The retrieved body keeps a file part's filename header and drops its bytes. function multipartOmitsFileBytes(body: string): boolean { return /content-disposition:[^\r\n]*[;\s]filename\s*\*?=/i.test(body); } -function armable(type: string, url: string): boolean { - return shouldArm({ - sessionId: "", - targetId: "", - type, - url, - fetchEnabled: false, - }); -} - -function armFailure(error: unknown, method: ArmMethod): ArmFailure { - if (error instanceof CdpCommandError) { - return { method, code: error.code, timedOut: error.timedOut }; - } - return { method, code: undefined, timedOut: false }; -} +type ArmMiss = { + method: "Fetch.enable" | "Network.enable"; + code?: number; + timedOut: boolean; +}; async function armFetch( connection: CdpConnection, session: Session, -): Promise { - if (session.fetchEnabled) return undefined; +): Promise { + if (session.fetchEnabled) return; try { await connection .send("Network.enable", {}, session.sessionId) @@ -1365,10 +1298,14 @@ async function armFetch( session.sessionId, ); } catch (error) { - return armFailure(error, "Fetch.enable"); + const failed = error instanceof CdpCommandError ? error : undefined; + return { + method: "Fetch.enable", + timedOut: failed?.timedOut ?? false, + ...(failed?.code !== undefined ? { code: failed.code } : {}), + }; } session.fetchEnabled = true; - return undefined; } async function continueRequest( From 77d9b554352eb9c1664b038ae24540c5c89dcd94 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 21:40:28 +0000 Subject: [PATCH 3/9] fix(browser-pay): narrow arm-failure method to Fetch.enable Network.enable is swallowed, so the event only names Fetch.enable. Cover a short command timeout and a non-integer CDP code that omits code. Co-authored-by: Rimmely Ewan --- packages/browser-pay/README.md | 2 +- packages/browser-pay/src/cdp.test.ts | 16 +++++++++- .../src/interceptor.protocol.test.ts | 31 +++++++++++++++++++ packages/browser-pay/src/interceptor.ts | 4 +-- 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/packages/browser-pay/README.md b/packages/browser-pay/README.md index 08f18fc3a..65ae7bb99 100644 --- a/packages/browser-pay/README.md +++ b/packages/browser-pay/README.md @@ -87,7 +87,7 @@ const handle = await attachInterceptor(cdpUrl, { Call `handle.on` after `attachInterceptor` resolves. Events emitted while attach was running, including a target that could not arm Fetch, are delivered to that first subscriber. -A `failed` event with reason `intercept_unavailable` closes that target with `Target.closeTarget`. `method` is `Fetch.enable` or `Network.enable`, `code` is the integer CDP error code when the browser sent one, `timedOut` is true when the command timed out, `targetType` is `page` or `iframe`, and `origin` uses the same cut as `url`. An opaque origin such as `about:blank` is emitted as `about:`. +A `failed` event with reason `intercept_unavailable` closes that target with `Target.closeTarget`. `method` is `Fetch.enable`, `code` is the integer CDP error code when the browser sent one, `timedOut` is true when the command timed out, `targetType` is `page` or `iframe`, and `origin` uses the same cut as `url`. An opaque origin such as `about:blank` is emitted as `about:`. A request is relayed when the held placeholder number appears in the URL or body. An exact match counts, and so do digits split by spaces or dashes, including the URL-encoded and JSON-escaped forms of those separators. `placeholder` is `{ number, injectionRef }`. `injectionRef` is a UUID. The backend's `checkPlaceholderUse` decides whether the number is valid. A paused request that does not hold the placeholder is continued. diff --git a/packages/browser-pay/src/cdp.test.ts b/packages/browser-pay/src/cdp.test.ts index ab4b7d331..8137d7ef2 100644 --- a/packages/browser-pay/src/cdp.test.ts +++ b/packages/browser-pay/src/cdp.test.ts @@ -2,7 +2,7 @@ import crypto from "node:crypto"; import http from "node:http"; import type { AddressInfo } from "node:net"; import type { Duplex } from "node:stream"; -import { CdpConnection } from "./cdp"; +import { CdpCommandError, CdpConnection } from "./cdp"; function acceptKey(key: string): string { return crypto @@ -109,6 +109,20 @@ describe("CdpConnection", () => { } }); + it("rejects Fetch.enable with timedOut when the browser never replies", async () => { + const stub = await listen(); + try { + const connection = await CdpConnection.connect(stub.url); + await stub.onceSocket; + const pending = connection.send("Fetch.enable", {}, undefined, 20); + await expect(pending).rejects.toBeInstanceOf(CdpCommandError); + await expect(pending).rejects.toHaveProperty("timedOut", true); + connection.close(); + } finally { + await stub.close(); + } + }); + it("rejects pending calls when a frame claims to exceed the size cap", async () => { const stub = await listen(); try { diff --git a/packages/browser-pay/src/interceptor.protocol.test.ts b/packages/browser-pay/src/interceptor.protocol.test.ts index 1d356af5e..8df67338a 100644 --- a/packages/browser-pay/src/interceptor.protocol.test.ts +++ b/packages/browser-pay/src/interceptor.protocol.test.ts @@ -429,6 +429,37 @@ describe("interceptor CDP decisions", () => { } }); + it("omits code when Fetch.enable returns a non-integer CDP code", async () => { + const stub = await startStub({ failFetch: true, fetchErrorCode: 1.5 }); + const events: InterceptorEvent[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + }); + handle.on((event) => events.push(event)); + attachPage(stub.emit); + await waitFor( + () => events.some((event) => event.type === "failed"), + "non-integer code", + ); + const failed = events.find((event) => event.type === "failed"); + expect(failed).toStrictEqual({ + type: "failed", + url: "https://shop.example", + reason: "intercept_unavailable", + method: "Fetch.enable", + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }); + await handle.detach(); + } finally { + await stub.close(); + } + }); + it("emits about: for an about:blank target that cannot arm Fetch", async () => { const stub = await startStub({ failFetch: true }); const events: InterceptorEvent[] = []; diff --git a/packages/browser-pay/src/interceptor.ts b/packages/browser-pay/src/interceptor.ts index f3704d39f..9e0533f91 100644 --- a/packages/browser-pay/src/interceptor.ts +++ b/packages/browser-pay/src/interceptor.ts @@ -92,7 +92,7 @@ export type InterceptorEvent = type: "failed"; url: string; reason: FailureReason; - method?: "Fetch.enable" | "Network.enable"; + method?: "Fetch.enable"; code?: number; timedOut?: boolean; targetType?: "page" | "iframe"; @@ -1278,7 +1278,7 @@ function multipartOmitsFileBytes(body: string): boolean { } type ArmMiss = { - method: "Fetch.enable" | "Network.enable"; + method: "Fetch.enable"; code?: number; timedOut: boolean; }; From 62ac3a9ba61724608fdbfa87c9d12298fa5becfd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 22:56:44 +0000 Subject: [PATCH 4/9] fix(browser-pay): name the CDP step when a swallowed command fails CdpCommandError includes the local method. Nested auto-attach, resume, existing-frame attach, and close log through the existing logger without changing control flow. Co-authored-by: Rimmely Ewan --- .../browser-pay-intercept-diagnostics.md | 2 +- packages/browser-pay/README.md | 4 +- packages/browser-pay/examples/attach.ts | 5 +- packages/browser-pay/src/cdp.test.ts | 5 + packages/browser-pay/src/cdp.ts | 19 +- .../src/interceptor.event-redaction.test.ts | 14 +- .../src/interceptor.protocol.test.ts | 249 +++++++++++++++++- packages/browser-pay/src/interceptor.ts | 76 ++++-- packages/browser-pay/src/logger.ts | 2 +- 9 files changed, 335 insertions(+), 41 deletions(-) diff --git a/.changeset/browser-pay-intercept-diagnostics.md b/.changeset/browser-pay-intercept-diagnostics.md index 5fa33cf01..64136875b 100644 --- a/.changeset/browser-pay-intercept-diagnostics.md +++ b/.changeset/browser-pay-intercept-diagnostics.md @@ -2,4 +2,4 @@ "@sapiom/browser-pay": patch --- -`failed` events with reason `intercept_unavailable` include the CDP method, integer error code, timeout flag, target type, and origin. The target is still closed. Remote error text is not included. +`failed` events with reason `intercept_unavailable` include the CDP method, integer error code, timeout flag, target type, and origin. The target is still closed. Remote error text is not included. An optional `logger` receives these swallowed steps, and CDP errors name the method. diff --git a/packages/browser-pay/README.md b/packages/browser-pay/README.md index 65ae7bb99..c8b67f8a4 100644 --- a/packages/browser-pay/README.md +++ b/packages/browser-pay/README.md @@ -55,7 +55,7 @@ Refusal, nothing forwarded: The `refused` event and logger pass through only these known reasons: `unknown_injection`, `injection_expired`, `payment_released`, `key_rotated`, `already_relayed`, `relay_in_progress`, `card_removed`, `payment_not_authorized`, `total_changed`, `destination_not_allowed`, `card_fields_invalid`, `merchant_mismatch`, and `provider_unavailable`. Any other reason becomes `unrecognized`. `relay_in_progress` is not terminal. The paused request stays open and the same idempotency key is posted again until the outcome is terminal or the deadline passes. -Event `url` is origin-only (`scheme://host[:port]`), with any held placeholder masked. An opaque origin uses its scheme (for example, `about:`); an unparseable URL is `""`. Paths, queries, fragments, and userinfo are never included. Attach errors use fixed messages and do not include transport details. CDP command failures and timeouts use the generic messages `CDP command failed` and `CDP command timed out`. +Event `url` is origin-only (`scheme://host[:port]`), with any held placeholder masked. An opaque origin uses its scheme (for example, `about:`); an unparseable URL is `""`. Paths, queries, fragments, and userinfo are never included. Attach errors use fixed messages and do not include transport details. CDP command failures and timeouts name the method that was sent (`CDP command failed: Target.setAutoAttach`, `CDP command timed out: Target.setAutoAttach`) and do not include the browser's error text. No answer after the send: @@ -87,7 +87,7 @@ const handle = await attachInterceptor(cdpUrl, { Call `handle.on` after `attachInterceptor` resolves. Events emitted while attach was running, including a target that could not arm Fetch, are delivered to that first subscriber. -A `failed` event with reason `intercept_unavailable` closes that target with `Target.closeTarget`. `method` is `Fetch.enable`, `code` is the integer CDP error code when the browser sent one, `timedOut` is true when the command timed out, `targetType` is `page` or `iframe`, and `origin` uses the same cut as `url`. An opaque origin such as `about:blank` is emitted as `about:`. +A `failed` event with reason `intercept_unavailable` closes that target with `Target.closeTarget`. `method` is `Fetch.enable`, `code` is the integer CDP error code when the browser sent one, `timedOut` is true when the command timed out, `targetType` is `page` or `iframe`, and `origin` uses the same cut as `url`. An opaque origin such as `about:blank` is emitted as `about:`. An optional `logger` receives `browser step failed` when a nested auto-attach, resume, existing-frame attach, or close fails; CDP errors name the method. A request is relayed when the held placeholder number appears in the URL or body. An exact match counts, and so do digits split by spaces or dashes, including the URL-encoded and JSON-escaped forms of those separators. `placeholder` is `{ number, injectionRef }`. `injectionRef` is a UUID. The backend's `checkPlaceholderUse` decides whether the number is valid. A paused request that does not hold the placeholder is continued. diff --git a/packages/browser-pay/examples/attach.ts b/packages/browser-pay/examples/attach.ts index 8ee27f2a0..0e5aa5386 100644 --- a/packages/browser-pay/examples/attach.ts +++ b/packages/browser-pay/examples/attach.ts @@ -21,7 +21,10 @@ const log = { info(message: string, fields?: Readonly>) { process.stdout.write(`${message} ${JSON.stringify(fields ?? {})}\n`); }, - warn(message: string, fields?: Readonly>) { + warn( + message: string, + fields?: Readonly>, + ) { process.stderr.write(`${message} ${JSON.stringify(fields ?? {})}\n`); }, }; diff --git a/packages/browser-pay/src/cdp.test.ts b/packages/browser-pay/src/cdp.test.ts index 8137d7ef2..c4783ba72 100644 --- a/packages/browser-pay/src/cdp.test.ts +++ b/packages/browser-pay/src/cdp.test.ts @@ -117,6 +117,11 @@ describe("CdpConnection", () => { const pending = connection.send("Fetch.enable", {}, undefined, 20); await expect(pending).rejects.toBeInstanceOf(CdpCommandError); await expect(pending).rejects.toHaveProperty("timedOut", true); + await expect(pending).rejects.toHaveProperty("method", "Fetch.enable"); + await expect(pending).rejects.toHaveProperty( + "message", + "CDP command timed out: Fetch.enable", + ); connection.close(); } finally { await stub.close(); diff --git a/packages/browser-pay/src/cdp.ts b/packages/browser-pay/src/cdp.ts index b7c47187d..e68b24344 100644 --- a/packages/browser-pay/src/cdp.ts +++ b/packages/browser-pay/src/cdp.ts @@ -21,17 +21,23 @@ type Pending = { resolve: (value: unknown) => void; reject: (error: Error) => void; timer: NodeJS.Timeout; + method: string; }; export class CdpCommandError extends Error { + readonly method: string; readonly code: number | undefined; readonly timedOut: boolean; - constructor(timedOut: boolean, code?: number) { - super(timedOut ? "CDP command timed out" : "CDP command failed"); + constructor(method: string, timedOut: boolean, code?: number) { + super( + `${timedOut ? "CDP command timed out" : "CDP command failed"}: ${method}`, + ); this.name = "CdpCommandError"; + this.method = method; this.timedOut = timedOut; - this.code = code; + this.code = + typeof code === "number" && Number.isInteger(code) ? code : undefined; } } @@ -360,7 +366,9 @@ export class CdpConnection { this.pending.delete(message.id); clearTimeout(pending.timer); if (message.failed) { - pending.reject(new CdpCommandError(false, message.code)); + pending.reject( + new CdpCommandError(pending.method, false, message.code), + ); } else { pending.resolve(message.result); } @@ -411,12 +419,13 @@ export class CdpConnection { } const timer = setTimeout(() => { this.pending.delete(id); - reject(new CdpCommandError(true)); + reject(new CdpCommandError(method, true)); }, timeoutMs); this.pending.set(id, { resolve, reject, timer, + method, }); try { this.socket.send( diff --git a/packages/browser-pay/src/interceptor.event-redaction.test.ts b/packages/browser-pay/src/interceptor.event-redaction.test.ts index 337d8f5f8..637e8224e 100644 --- a/packages/browser-pay/src/interceptor.event-redaction.test.ts +++ b/packages/browser-pay/src/interceptor.event-redaction.test.ts @@ -684,14 +684,16 @@ describe("interceptor event redaction", () => { attachOptions(stubPaymentRelay(async () => success())), ), ); - expect(protocolError.message).toBe("CDP command failed"); + expect(protocolError.message).toBe( + "CDP command failed: Target.setAutoAttach", + ); assertSafeError(protocolError); } finally { await stub.close(); } }); - it("uses generic errors for secret-bearing CDP methods", async () => { + it("names the local CDP method and drops remote error text", async () => { const method = "Synthetic.method?token=SYNTHETIC_METHOD_SECRET_1"; const errorStub = await startCdp({ errorOnMethod: { @@ -704,8 +706,8 @@ describe("interceptor event redaction", () => { const error = await captureError( errorConnection.send(method, {}, undefined, 1_000), ); - expect(error.message).toBe("CDP command failed"); - expect(error.message).not.toContain("SYNTHETIC_METHOD_SECRET"); + expect(error.message).toBe(`CDP command failed: ${method}`); + expect(error.message).not.toContain("peer rejected"); } finally { errorConnection.close(); await errorStub.close(); @@ -717,8 +719,8 @@ describe("interceptor event redaction", () => { const error = await captureError( timeoutConnection.send(method, {}, undefined, 10), ); - expect(error.message).toBe("CDP command timed out"); - expect(error.message).not.toContain("SYNTHETIC_METHOD_SECRET"); + expect(error.message).toBe(`CDP command timed out: ${method}`); + expect(error.message).not.toContain("peer rejected"); } finally { timeoutConnection.close(); await timeoutStub.close(); diff --git a/packages/browser-pay/src/interceptor.protocol.test.ts b/packages/browser-pay/src/interceptor.protocol.test.ts index 8df67338a..c5d49fc1e 100644 --- a/packages/browser-pay/src/interceptor.protocol.test.ts +++ b/packages/browser-pay/src/interceptor.protocol.test.ts @@ -2,6 +2,7 @@ import crypto from "node:crypto"; import http from "node:http"; import type { AddressInfo } from "node:net"; import type { Duplex } from "node:stream"; +import { CdpCommandError } from "./cdp"; import { attachInterceptor, type InterceptorEvent } from "./interceptor"; import type { HeldPlaceholder, RelayRequest, RelayResponse } from "./index"; import { stubPaymentRelay } from "../test/stub-relay"; @@ -27,8 +28,14 @@ interface Command { sessionId?: string; } +const REMOTE_STEP = "REMOTE_STEP_TEXT"; + interface StubOptions { failAutoAttach?: boolean; + failChildAutoAttach?: boolean; + failResume?: boolean; + failAttachToTarget?: boolean; + failCloseTarget?: boolean; failFetch?: boolean; fetchErrorCode?: number; fetchErrorMessage?: string; @@ -127,6 +134,25 @@ async function startStub(options: StubOptions = {}): Promise<{ reply(message.id, undefined, { message: "auto-attach refused" }); return; } + if ( + message.method === "Target.setAutoAttach" && + message.sessionId && + options.failChildAutoAttach + ) { + reply(message.id, undefined, { message: REMOTE_STEP, code: -32000 }); + return; + } + if ( + message.method === "Runtime.runIfWaitingForDebugger" && + options.failResume + ) { + reply(message.id, undefined, { message: REMOTE_STEP, code: -32000 }); + return; + } + if (message.method === "Target.closeTarget" && options.failCloseTarget) { + reply(message.id, undefined, { message: REMOTE_STEP, code: -32000 }); + return; + } if (message.method === "Fetch.enable" && options.failFetch) { reply(message.id, undefined, { message: options.fetchErrorMessage ?? "Fetch is unsupported", @@ -141,6 +167,10 @@ async function startStub(options: StubOptions = {}): Promise<{ return; } if (message.method === "Target.attachToTarget") { + if (options.failAttachToTarget) { + reply(message.id, undefined, { message: REMOTE_STEP, code: -32000 }); + return; + } reply(message.id, { sessionId: options.attachedSessionId ?? "iframe-existing", }); @@ -311,23 +341,228 @@ function header( return headers[name.toLowerCase()]; } +type StepWarning = { + message: string; + fields?: Readonly>; +}; + +function stepLogger(warnings: StepWarning[]) { + return { + warn( + message: string, + fields?: Readonly>, + ) { + warnings.push(fields ? { message, fields } : { message }); + }, + }; +} + describe("interceptor CDP decisions", () => { it("closes the CDP connection when start fails", async () => { const stub = await startStub({ failAutoAttach: true }); try { - await expect( - attachInterceptor(stub.url, { - relay: stubPaymentRelay(async () => success()), - placeholder: HELD, - profileId: "shopify-card-vault", - }), - ).rejects.toThrow("CDP command failed"); + const caught = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + }).catch((error: unknown) => error); + expect(caught).toBeInstanceOf(CdpCommandError); + expect(caught).toMatchObject({ + method: "Target.setAutoAttach", + message: "CDP command failed: Target.setAutoAttach", + }); + expect( + JSON.stringify(caught, Object.getOwnPropertyNames(caught as object)), + ).not.toContain("auto-attach refused"); await waitFor(() => stub.sawClose(), "websocket close"); } finally { await stub.close(); } }); + it("logs a nested setAutoAttach failure and still arms Fetch", async () => { + const stub = await startStub({ failChildAutoAttach: true }); + const events: InterceptorEvent[] = []; + const warnings: StepWarning[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + logger: stepLogger(warnings), + }); + handle.on((event) => events.push(event)); + attachPage(stub.emit); + await waitFor( + () => + stub.commands.some((command) => command.method === "Fetch.enable"), + "Fetch.enable", + ); + expect(events).toEqual([]); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Target.closeTarget", + ); + expect(warnings).toStrictEqual([ + { + message: "browser step failed", + fields: { + method: "Target.setAutoAttach", + code: -32000, + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }, + }, + ]); + expect(JSON.stringify(warnings)).not.toContain(REMOTE_STEP); + expect(JSON.stringify(warnings)).not.toContain("checkout"); + await handle.detach(); + } finally { + await stub.close(); + } + }); + + it("logs a resume failure and still leaves the target armed", async () => { + const stub = await startStub({ failResume: true }); + const events: InterceptorEvent[] = []; + const warnings: StepWarning[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + logger: stepLogger(warnings), + }); + handle.on((event) => events.push(event)); + attachPage(stub.emit); + await waitFor(() => warnings.length === 1, "resume warning"); + expect(events).toEqual([]); + expect(stub.commands.map((command) => command.method)).toContain( + "Fetch.enable", + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Target.closeTarget", + ); + expect(warnings).toStrictEqual([ + { + message: "browser step failed", + fields: { + method: "Runtime.runIfWaitingForDebugger", + code: -32000, + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }, + }, + ]); + expect(JSON.stringify(warnings)).not.toContain(REMOTE_STEP); + await handle.detach(); + } finally { + await stub.close(); + } + }); + + it("logs an attachToTarget failure and leaves that iframe unarmed", async () => { + const stub = await startStub({ + failAttachToTarget: true, + targets: [ + { + targetId: "target-card", + type: "iframe", + url: "https://checkout.pci.shopifyinc.com/sessions?card=1", + }, + ], + }); + const events: InterceptorEvent[] = []; + const warnings: StepWarning[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + logger: stepLogger(warnings), + }); + handle.on((event) => events.push(event)); + expect(events).toEqual([]); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Fetch.enable", + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Target.closeTarget", + ); + expect(warnings).toStrictEqual([ + { + message: "browser step failed", + fields: { + method: "Target.attachToTarget", + code: -32000, + timedOut: false, + targetType: "iframe", + origin: "https://checkout.pci.shopifyinc.com", + }, + }, + ]); + expect(JSON.stringify(warnings)).not.toContain(REMOTE_STEP); + expect(JSON.stringify(warnings)).not.toContain("sessions"); + await handle.detach(); + } finally { + await stub.close(); + } + }); + + it("logs a closeTarget failure and still emits intercept_unavailable", async () => { + const stub = await startStub({ failFetch: true, failCloseTarget: true }); + const events: InterceptorEvent[] = []; + const warnings: StepWarning[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + logger: stepLogger(warnings), + }); + handle.on((event) => events.push(event)); + attachPage(stub.emit); + await waitFor(() => warnings.length === 1, "close warning"); + expect(events).toEqual([ + { + type: "failed", + url: "https://shop.example", + reason: "intercept_unavailable", + method: "Fetch.enable", + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }, + ]); + expect(stub.commands).toContainEqual( + expect.objectContaining({ + method: "Target.closeTarget", + params: { targetId: "target-page" }, + }), + ); + expect(stub.commands.map((command) => command.method)).not.toContain( + "Runtime.runIfWaitingForDebugger", + ); + expect(warnings).toStrictEqual([ + { + message: "browser step failed", + fields: { + method: "Target.closeTarget", + code: -32000, + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }, + }, + ]); + expect(JSON.stringify(warnings)).not.toContain(REMOTE_STEP); + await handle.detach(); + } finally { + await stub.close(); + } + }); + it("does not resume a target whose Fetch.enable failed", async () => { const stub = await startStub({ failFetch: true }); const events: InterceptorEvent[] = []; diff --git a/packages/browser-pay/src/interceptor.ts b/packages/browser-pay/src/interceptor.ts index 9e0533f91..7c2c19675 100644 --- a/packages/browser-pay/src/interceptor.ts +++ b/packages/browser-pay/src/interceptor.ts @@ -452,15 +452,46 @@ class PaymentInterceptor { ...(targetType ? { targetType } : {}), origin: eventUrl(session.url, this.number), }); - await this.connection - .send("Target.closeTarget", { targetId: session.targetId }) - .catch(() => {}); + await this.swallowStep( + this.connection.send("Target.closeTarget", { + targetId: session.targetId, + }), + "Target.closeTarget", + session, + ); return false; } + private async swallowStep( + pending: Promise, + method: string, + target?: { type: string; url: string }, + ): Promise { + try { + return await pending; + } catch (error) { + if (this.closed) return undefined; + const failed = error instanceof CdpCommandError ? error : undefined; + const targetType = + target?.type === "page" || target?.type === "iframe" + ? target.type + : undefined; + const code = failed?.code; + this.logger.warn("browser step failed", { + method: failed?.method ?? method, + ...(code !== undefined ? { code } : {}), + timedOut: failed?.timedOut ?? false, + ...(targetType ? { targetType } : {}), + ...(target ? { origin: eventUrl(target.url, this.number) } : {}), + }); + return undefined; + } + } + private async autoAttachChildren(sessionId: string): Promise { - await this.connection - .send( + const session = this.sessions.get(sessionId); + await this.swallowStep( + this.connection.send( "Target.setAutoAttach", { autoAttach: true, @@ -469,8 +500,10 @@ class PaymentInterceptor { filter: CHILD_FILTER, }, sessionId, - ) - .catch(() => {}); + ), + "Target.setAutoAttach", + session, + ); } private async resumeIfWaiting( @@ -478,24 +511,30 @@ class PaymentInterceptor { waitingForDebugger: boolean, ): Promise { if (!waitingForDebugger) return; - await this.connection - .send("Runtime.runIfWaitingForDebugger", {}, sessionId) - .catch(() => {}); + const session = this.sessions.get(sessionId); + await this.swallowStep( + this.connection.send("Runtime.runIfWaitingForDebugger", {}, sessionId), + "Runtime.runIfWaitingForDebugger", + session, + ); } private async attachExistingFrames(): Promise { - const listed = await this.connection - .send("Target.getTargets") - .catch(() => undefined); + const listed = await this.swallowStep( + this.connection.send("Target.getTargets"), + "Target.getTargets", + ); for (const info of parseTargetInfos(listed)) { if (info.type !== "iframe") continue; if (this.byTarget.has(info.targetId)) continue; - const attached = await this.connection - .send("Target.attachToTarget", { + const attached = await this.swallowStep( + this.connection.send("Target.attachToTarget", { targetId: info.targetId, flatten: true, - }) - .catch(() => undefined); + }), + "Target.attachToTarget", + { type: info.type, url: info.url }, + ); const sessionId = readAttachedSessionId(attached); if (!sessionId || this.sessions.has(sessionId)) continue; const session = this.trackSession({ @@ -1300,7 +1339,8 @@ async function armFetch( } catch (error) { const failed = error instanceof CdpCommandError ? error : undefined; return { - method: "Fetch.enable", + method: + failed?.method === "Fetch.enable" ? failed.method : "Fetch.enable", timedOut: failed?.timedOut ?? false, ...(failed?.code !== undefined ? { code: failed.code } : {}), }; diff --git a/packages/browser-pay/src/logger.ts b/packages/browser-pay/src/logger.ts index 71b49ca46..6ec84fb27 100644 --- a/packages/browser-pay/src/logger.ts +++ b/packages/browser-pay/src/logger.ts @@ -1,7 +1,7 @@ export interface PayLogger { warn( message: string, - fields?: Readonly>, + fields?: Readonly>, ): void; } From fac35cf1504fe0a2399139fce0742a32457e1069 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 22:59:24 +0000 Subject: [PATCH 5/9] fix(browser-pay): keep the arm miss method as Fetch.enable The catch only runs after Network.enable is swallowed, so the method is always Fetch.enable. Co-authored-by: Rimmely Ewan --- packages/browser-pay/src/interceptor.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/browser-pay/src/interceptor.ts b/packages/browser-pay/src/interceptor.ts index 7c2c19675..2a4dac756 100644 --- a/packages/browser-pay/src/interceptor.ts +++ b/packages/browser-pay/src/interceptor.ts @@ -1339,8 +1339,7 @@ async function armFetch( } catch (error) { const failed = error instanceof CdpCommandError ? error : undefined; return { - method: - failed?.method === "Fetch.enable" ? failed.method : "Fetch.enable", + method: "Fetch.enable", timedOut: failed?.timedOut ?? false, ...(failed?.code !== undefined ? { code: failed.code } : {}), }; From cfbf149dcc8b1f75f53e03f92fbd87b17e876933 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 23:05:44 +0000 Subject: [PATCH 6/9] fix(browser-pay): ignore a logger that throws during a swallowed step A consumer logger must not reject arming or skip detach. The logged method is the literal the caller passed. Co-authored-by: Rimmely Ewan --- packages/browser-pay/README.md | 2 +- packages/browser-pay/examples/attach.ts | 5 +- .../src/interceptor.protocol.test.ts | 47 +++++++++++++++++++ packages/browser-pay/src/interceptor.ts | 18 ++++--- 4 files changed, 60 insertions(+), 12 deletions(-) diff --git a/packages/browser-pay/README.md b/packages/browser-pay/README.md index c8b67f8a4..bb1c9d7db 100644 --- a/packages/browser-pay/README.md +++ b/packages/browser-pay/README.md @@ -87,7 +87,7 @@ const handle = await attachInterceptor(cdpUrl, { Call `handle.on` after `attachInterceptor` resolves. Events emitted while attach was running, including a target that could not arm Fetch, are delivered to that first subscriber. -A `failed` event with reason `intercept_unavailable` closes that target with `Target.closeTarget`. `method` is `Fetch.enable`, `code` is the integer CDP error code when the browser sent one, `timedOut` is true when the command timed out, `targetType` is `page` or `iframe`, and `origin` uses the same cut as `url`. An opaque origin such as `about:blank` is emitted as `about:`. An optional `logger` receives `browser step failed` when a nested auto-attach, resume, existing-frame attach, or close fails; CDP errors name the method. +A `failed` event with reason `intercept_unavailable` closes that target with `Target.closeTarget`. `method` is `Fetch.enable`, `code` is the integer CDP error code when the browser sent one, `timedOut` is true when the command timed out, `targetType` is `page` or `iframe`, and `origin` uses the same cut as `url`. An opaque origin such as `about:blank` is emitted as `about:`. An optional `logger` receives `browser step failed` when a nested auto-attach, resume, `Target.getTargets`, existing-frame attach, or close fails; CDP errors name the method. A request is relayed when the held placeholder number appears in the URL or body. An exact match counts, and so do digits split by spaces or dashes, including the URL-encoded and JSON-escaped forms of those separators. `placeholder` is `{ number, injectionRef }`. `injectionRef` is a UUID. The backend's `checkPlaceholderUse` decides whether the number is valid. A paused request that does not hold the placeholder is continued. diff --git a/packages/browser-pay/examples/attach.ts b/packages/browser-pay/examples/attach.ts index 0e5aa5386..8ee27f2a0 100644 --- a/packages/browser-pay/examples/attach.ts +++ b/packages/browser-pay/examples/attach.ts @@ -21,10 +21,7 @@ const log = { info(message: string, fields?: Readonly>) { process.stdout.write(`${message} ${JSON.stringify(fields ?? {})}\n`); }, - warn( - message: string, - fields?: Readonly>, - ) { + warn(message: string, fields?: Readonly>) { process.stderr.write(`${message} ${JSON.stringify(fields ?? {})}\n`); }, }; diff --git a/packages/browser-pay/src/interceptor.protocol.test.ts b/packages/browser-pay/src/interceptor.protocol.test.ts index c5d49fc1e..85f5e05cd 100644 --- a/packages/browser-pay/src/interceptor.protocol.test.ts +++ b/packages/browser-pay/src/interceptor.protocol.test.ts @@ -422,6 +422,53 @@ describe("interceptor CDP decisions", () => { } }); + it("a throwing logger leaves a nested setAutoAttach failure unchanged", async () => { + const options: StubOptions = { + failChildAutoAttach: true, + targets: [ + { + targetId: "target-card", + type: "iframe", + url: "https://checkout.pci.shopifyinc.com/sessions", + }, + ], + attachedSessionId: "iframe-card", + }; + const run = async (logger?: { + warn(message: string): void; + }): Promise<{ events: InterceptorEvent[]; commands: Command[] }> => { + const stub = await startStub(options); + const events: InterceptorEvent[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + ...(logger ? { logger } : {}), + }); + handle.on((event) => events.push(event)); + await handle.detach(); + return { events: [...events], commands: [...stub.commands] }; + } finally { + await stub.close(); + } + }; + const silent = await run(); + let warned = 0; + const throwing = await run({ + warn() { + warned += 1; + throw new Error("logger boom"); + }, + }); + expect(warned).toBeGreaterThan(0); + expect(silent.commands.map((command) => command.method)).toContain( + "Fetch.enable", + ); + expect(throwing.events).toEqual(silent.events); + expect(throwing.commands).toEqual(silent.commands); + }); + it("logs a resume failure and still leaves the target armed", async () => { const stub = await startStub({ failResume: true }); const events: InterceptorEvent[] = []; diff --git a/packages/browser-pay/src/interceptor.ts b/packages/browser-pay/src/interceptor.ts index 2a4dac756..1ba77c835 100644 --- a/packages/browser-pay/src/interceptor.ts +++ b/packages/browser-pay/src/interceptor.ts @@ -477,13 +477,17 @@ class PaymentInterceptor { ? target.type : undefined; const code = failed?.code; - this.logger.warn("browser step failed", { - method: failed?.method ?? method, - ...(code !== undefined ? { code } : {}), - timedOut: failed?.timedOut ?? false, - ...(targetType ? { targetType } : {}), - ...(target ? { origin: eventUrl(target.url, this.number) } : {}), - }); + try { + this.logger.warn("browser step failed", { + method, + ...(code !== undefined ? { code } : {}), + timedOut: failed?.timedOut ?? false, + ...(targetType ? { targetType } : {}), + ...(target ? { origin: eventUrl(target.url, this.number) } : {}), + }); + } catch { + // A logger must not change control flow. + } return undefined; } } From dbf2d17dc51c41e23e16a6ca1ab2b05e461972a5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 23:52:12 +0000 Subject: [PATCH 7/9] fix(browser-pay): keep unsafe CDP method names out of command errors Only a plain Domain.method is named. Any other caller-supplied method keeps the generic failure and timeout messages. Co-authored-by: Rimmely Ewan --- packages/browser-pay/src/cdp.ts | 12 +++++++----- .../src/interceptor.event-redaction.test.ts | 16 +++++++++++----- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/browser-pay/src/cdp.ts b/packages/browser-pay/src/cdp.ts index e68b24344..04ae95ec6 100644 --- a/packages/browser-pay/src/cdp.ts +++ b/packages/browser-pay/src/cdp.ts @@ -24,17 +24,19 @@ type Pending = { method: string; }; +const PLAIN_CDP_METHOD = /^[A-Za-z]+\.[A-Za-z]+$/; + export class CdpCommandError extends Error { - readonly method: string; + readonly method: string | undefined; readonly code: number | undefined; readonly timedOut: boolean; constructor(method: string, timedOut: boolean, code?: number) { - super( - `${timedOut ? "CDP command timed out" : "CDP command failed"}: ${method}`, - ); + const safe = PLAIN_CDP_METHOD.test(method) ? method : undefined; + const label = timedOut ? "CDP command timed out" : "CDP command failed"; + super(safe === undefined ? label : `${label}: ${safe}`); this.name = "CdpCommandError"; - this.method = method; + this.method = safe; this.timedOut = timedOut; this.code = typeof code === "number" && Number.isInteger(code) ? code : undefined; diff --git a/packages/browser-pay/src/interceptor.event-redaction.test.ts b/packages/browser-pay/src/interceptor.event-redaction.test.ts index 637e8224e..43cb09a08 100644 --- a/packages/browser-pay/src/interceptor.event-redaction.test.ts +++ b/packages/browser-pay/src/interceptor.event-redaction.test.ts @@ -693,7 +693,7 @@ describe("interceptor event redaction", () => { } }); - it("names the local CDP method and drops remote error text", async () => { + it("uses generic errors for secret-bearing CDP methods", async () => { const method = "Synthetic.method?token=SYNTHETIC_METHOD_SECRET_1"; const errorStub = await startCdp({ errorOnMethod: { @@ -706,8 +706,11 @@ describe("interceptor event redaction", () => { const error = await captureError( errorConnection.send(method, {}, undefined, 1_000), ); - expect(error.message).toBe(`CDP command failed: ${method}`); - expect(error.message).not.toContain("peer rejected"); + expect(error.message).toBe("CDP command failed"); + expect(error.message).not.toContain("SYNTHETIC_METHOD_SECRET"); + expect( + JSON.stringify(error, Object.getOwnPropertyNames(error)), + ).not.toContain("SYNTHETIC_METHOD_SECRET"); } finally { errorConnection.close(); await errorStub.close(); @@ -719,8 +722,11 @@ describe("interceptor event redaction", () => { const error = await captureError( timeoutConnection.send(method, {}, undefined, 10), ); - expect(error.message).toBe(`CDP command timed out: ${method}`); - expect(error.message).not.toContain("peer rejected"); + expect(error.message).toBe("CDP command timed out"); + expect(error.message).not.toContain("SYNTHETIC_METHOD_SECRET"); + expect( + JSON.stringify(error, Object.getOwnPropertyNames(error)), + ).not.toContain("SYNTHETIC_METHOD_SECRET"); } finally { timeoutConnection.close(); await timeoutStub.close(); From 9a9f379ee52413a488976dce1d0a868c784bdd64 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 27 Oct 2026 00:05:58 +0000 Subject: [PATCH 8/9] fix(browser-pay): log arm failures instead of extending the event intercept_unavailable is the same event as before. An optional logger records why Fetch.enable failed, alongside the other swallowed steps. Co-authored-by: Rimmely Ewan --- .../browser-pay-intercept-diagnostics.md | 2 +- packages/browser-pay/README.md | 2 +- .../src/interceptor.event-redaction.test.ts | 6 +- .../src/interceptor.protocol.test.ts | 214 +++++++++++++----- packages/browser-pay/src/interceptor.ts | 80 +++---- 5 files changed, 190 insertions(+), 114 deletions(-) diff --git a/.changeset/browser-pay-intercept-diagnostics.md b/.changeset/browser-pay-intercept-diagnostics.md index 64136875b..993d7d9e7 100644 --- a/.changeset/browser-pay-intercept-diagnostics.md +++ b/.changeset/browser-pay-intercept-diagnostics.md @@ -2,4 +2,4 @@ "@sapiom/browser-pay": patch --- -`failed` events with reason `intercept_unavailable` include the CDP method, integer error code, timeout flag, target type, and origin. The target is still closed. Remote error text is not included. An optional `logger` receives these swallowed steps, and CDP errors name the method. +Better debug logging for browser-pay: an optional logger now receives browser step failures, and CDP errors name the method. diff --git a/packages/browser-pay/README.md b/packages/browser-pay/README.md index bb1c9d7db..c3c6ca0e1 100644 --- a/packages/browser-pay/README.md +++ b/packages/browser-pay/README.md @@ -87,7 +87,7 @@ const handle = await attachInterceptor(cdpUrl, { Call `handle.on` after `attachInterceptor` resolves. Events emitted while attach was running, including a target that could not arm Fetch, are delivered to that first subscriber. -A `failed` event with reason `intercept_unavailable` closes that target with `Target.closeTarget`. `method` is `Fetch.enable`, `code` is the integer CDP error code when the browser sent one, `timedOut` is true when the command timed out, `targetType` is `page` or `iframe`, and `origin` uses the same cut as `url`. An opaque origin such as `about:blank` is emitted as `about:`. An optional `logger` receives `browser step failed` when a nested auto-attach, resume, `Target.getTargets`, existing-frame attach, or close fails; CDP errors name the method. +An optional `logger` receives `browser step failed` when arming `Fetch.enable` fails, and when a nested auto-attach, resume, `Target.getTargets`, existing-frame attach, or close fails. A request is relayed when the held placeholder number appears in the URL or body. An exact match counts, and so do digits split by spaces or dashes, including the URL-encoded and JSON-escaped forms of those separators. `placeholder` is `{ number, injectionRef }`. `injectionRef` is a UUID. The backend's `checkPlaceholderUse` decides whether the number is valid. A paused request that does not hold the placeholder is continued. diff --git a/packages/browser-pay/src/interceptor.event-redaction.test.ts b/packages/browser-pay/src/interceptor.event-redaction.test.ts index 43cb09a08..5b3f74833 100644 --- a/packages/browser-pay/src/interceptor.event-redaction.test.ts +++ b/packages/browser-pay/src/interceptor.event-redaction.test.ts @@ -597,15 +597,11 @@ describe("interceptor event redaction", () => { attachOptions(stubPaymentRelay(async () => success())), ); handle.on((event) => events.push(event)); - expect(events).toEqual([ + expect(events).toStrictEqual([ { type: "failed", url: ORIGIN, reason: "intercept_unavailable", - method: "Fetch.enable", - timedOut: false, - targetType: "page", - origin: ORIGIN, }, ]); assertNoMarkers(events); diff --git a/packages/browser-pay/src/interceptor.protocol.test.ts b/packages/browser-pay/src/interceptor.protocol.test.ts index 85f5e05cd..7d4643dce 100644 --- a/packages/browser-pay/src/interceptor.protocol.test.ts +++ b/packages/browser-pay/src/interceptor.protocol.test.ts @@ -570,16 +570,12 @@ describe("interceptor CDP decisions", () => { }); handle.on((event) => events.push(event)); attachPage(stub.emit); - await waitFor(() => warnings.length === 1, "close warning"); - expect(events).toEqual([ + await waitFor(() => warnings.length === 2, "close warning"); + expect(events).toStrictEqual([ { type: "failed", url: "https://shop.example", reason: "intercept_unavailable", - method: "Fetch.enable", - timedOut: false, - targetType: "page", - origin: "https://shop.example", }, ]); expect(stub.commands).toContainEqual( @@ -592,6 +588,15 @@ describe("interceptor CDP decisions", () => { "Runtime.runIfWaitingForDebugger", ); expect(warnings).toStrictEqual([ + { + message: "browser step failed", + fields: { + method: "Fetch.enable", + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }, + }, { message: "browser step failed", fields: { @@ -613,11 +618,13 @@ describe("interceptor CDP decisions", () => { it("does not resume a target whose Fetch.enable failed", async () => { const stub = await startStub({ failFetch: true }); const events: InterceptorEvent[] = []; + const warnings: StepWarning[] = []; try { const handle = await attachInterceptor(stub.url, { relay: stubPaymentRelay(async () => success()), placeholder: HELD, profileId: "shopify-card-vault", + logger: stepLogger(warnings), }); handle.on((event) => events.push(event)); attachPage(stub.emit); @@ -637,15 +644,24 @@ describe("interceptor CDP decisions", () => { params: { targetId: "target-page" }, }), ); - expect(events).toContainEqual({ - type: "failed", - url: "https://shop.example", - reason: "intercept_unavailable", - method: "Fetch.enable", - timedOut: false, - targetType: "page", - origin: "https://shop.example", - }); + expect(events).toStrictEqual([ + { + type: "failed", + url: "https://shop.example", + reason: "intercept_unavailable", + }, + ]); + expect(warnings).toStrictEqual([ + { + message: "browser step failed", + fields: { + method: "Fetch.enable", + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }, + }, + ]); await handle.detach(); } finally { await stub.close(); @@ -660,11 +676,13 @@ describe("interceptor CDP decisions", () => { fetchErrorMessage: remote, }); const events: InterceptorEvent[] = []; + const warnings: StepWarning[] = []; try { const handle = await attachInterceptor(stub.url, { relay: stubPaymentRelay(async () => success()), placeholder: HELD, profileId: "shopify-card-vault", + logger: stepLogger(warnings), }); handle.on((event) => events.push(event)); stub.emit({ @@ -692,19 +710,28 @@ describe("interceptor CDP decisions", () => { params: { targetId: "target-page" }, }), ); - const failed = events.find((event) => event.type === "failed"); - expect(failed).toEqual({ - type: "failed", - url: "https://shop.example", - reason: "intercept_unavailable", - method: "Fetch.enable", - code: -32000, - timedOut: false, - targetType: "page", - origin: "https://shop.example", - }); - expect(JSON.stringify(failed)).not.toContain(remote); - expect(JSON.stringify(failed)).not.toContain("checkout"); + expect(events).toStrictEqual([ + { + type: "failed", + url: "https://shop.example", + reason: "intercept_unavailable", + }, + ]); + expect(warnings).toStrictEqual([ + { + message: "browser step failed", + fields: { + method: "Fetch.enable", + code: -32000, + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }, + }, + ]); + expect(JSON.stringify(events)).not.toContain(remote); + expect(JSON.stringify(warnings)).not.toContain(remote); + expect(JSON.stringify(warnings)).not.toContain("checkout"); await handle.detach(); } finally { await stub.close(); @@ -714,11 +741,13 @@ describe("interceptor CDP decisions", () => { it("omits code when Fetch.enable returns a non-integer CDP code", async () => { const stub = await startStub({ failFetch: true, fetchErrorCode: 1.5 }); const events: InterceptorEvent[] = []; + const warnings: StepWarning[] = []; try { const handle = await attachInterceptor(stub.url, { relay: stubPaymentRelay(async () => success()), placeholder: HELD, profileId: "shopify-card-vault", + logger: stepLogger(warnings), }); handle.on((event) => events.push(event)); attachPage(stub.emit); @@ -726,30 +755,40 @@ describe("interceptor CDP decisions", () => { () => events.some((event) => event.type === "failed"), "non-integer code", ); - const failed = events.find((event) => event.type === "failed"); - expect(failed).toStrictEqual({ - type: "failed", - url: "https://shop.example", - reason: "intercept_unavailable", - method: "Fetch.enable", - timedOut: false, - targetType: "page", - origin: "https://shop.example", - }); + expect(events).toStrictEqual([ + { + type: "failed", + url: "https://shop.example", + reason: "intercept_unavailable", + }, + ]); + expect(warnings).toStrictEqual([ + { + message: "browser step failed", + fields: { + method: "Fetch.enable", + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }, + }, + ]); await handle.detach(); } finally { await stub.close(); } }); - it("emits about: for an about:blank target that cannot arm Fetch", async () => { + it("logs about: for an about:blank target that cannot arm Fetch", async () => { const stub = await startStub({ failFetch: true }); const events: InterceptorEvent[] = []; + const warnings: StepWarning[] = []; try { const handle = await attachInterceptor(stub.url, { relay: stubPaymentRelay(async () => success()), placeholder: HELD, profileId: "shopify-card-vault", + logger: stepLogger(warnings), }); handle.on((event) => events.push(event)); stub.emit({ @@ -768,21 +807,84 @@ describe("interceptor CDP decisions", () => { () => events.some((event) => event.type === "failed"), "about blank", ); - expect(events).toContainEqual({ - type: "failed", - url: "about:", - reason: "intercept_unavailable", - method: "Fetch.enable", - timedOut: false, - targetType: "iframe", - origin: "about:", - }); + expect(events).toStrictEqual([ + { + type: "failed", + url: "about:", + reason: "intercept_unavailable", + }, + ]); + expect(warnings).toStrictEqual([ + { + message: "browser step failed", + fields: { + method: "Fetch.enable", + timedOut: false, + targetType: "iframe", + origin: "about:", + }, + }, + ]); await handle.detach(); } finally { await stub.close(); } }); + it("a throwing logger does not change a Fetch.enable failure", async () => { + const run = async (logger?: { + warn(message: string): void; + }): Promise<{ events: InterceptorEvent[]; commands: Command[] }> => { + const stub = await startStub({ failFetch: true }); + const events: InterceptorEvent[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + ...(logger ? { logger } : {}), + }); + handle.on((event) => events.push(event)); + attachPage(stub.emit); + await waitFor( + () => + stub.commands.some( + (command) => command.method === "Target.closeTarget", + ), + "closeTarget", + ); + await handle.detach(); + return { events: [...events], commands: [...stub.commands] }; + } finally { + await stub.close(); + } + }; + const silent = await run(); + let warned = 0; + const throwing = await run({ + warn() { + warned += 1; + throw new Error("logger boom"); + }, + }); + expect(warned).toBeGreaterThan(0); + expect(silent.events).toStrictEqual([ + { + type: "failed", + url: "https://shop.example", + reason: "intercept_unavailable", + }, + ]); + expect(throwing.events).toStrictEqual(silent.events); + expect(throwing.commands).toEqual(silent.commands); + expect(throwing.commands).toContainEqual( + expect.objectContaining({ + method: "Target.closeTarget", + params: { targetId: "target-page" }, + }), + ); + }); + it("reads a body with missing entry bytes via getRequestPostData and relays it", async () => { const stub = await startStub({ postData: CARD }); const calls: RelayRequest[] = []; @@ -1985,15 +2087,13 @@ describe("interceptor CDP decisions", () => { ), ).toBe(true); handle.on((event) => events.push(event)); - expect(events).toContainEqual({ - type: "failed", - url: "https://checkout.example", - reason: "intercept_unavailable", - method: "Fetch.enable", - timedOut: false, - targetType: "iframe", - origin: "https://checkout.example", - }); + expect(events).toStrictEqual([ + { + type: "failed", + url: "https://checkout.example", + reason: "intercept_unavailable", + }, + ]); await handle.detach(); } finally { await stub.close(); diff --git a/packages/browser-pay/src/interceptor.ts b/packages/browser-pay/src/interceptor.ts index 1ba77c835..183b9df74 100644 --- a/packages/browser-pay/src/interceptor.ts +++ b/packages/browser-pay/src/interceptor.ts @@ -88,16 +88,7 @@ export type InterceptorEvent = | { type: "relayed"; url: string; status: number } | { type: "relayed_unfulfilled"; url: string; status: number } | { type: "refused"; url: string; reason: RefusalReason } - | { - type: "failed"; - url: string; - reason: FailureReason; - method?: "Fetch.enable"; - code?: number; - timedOut?: boolean; - targetType?: "page" | "iframe"; - origin?: string; - }; + | { type: "failed"; url: string; reason: FailureReason }; export interface InterceptorHandle { detach(): Promise; @@ -438,20 +429,12 @@ class PaymentInterceptor { const failure = await armFetch(this.connection, session); if (!failure) return true; if (this.closed) return false; - const targetType = - session.type === "page" || session.type === "iframe" - ? session.type - : undefined; this.emit({ type: "failed", url: session.url, reason: "intercept_unavailable", - method: failure.method, - ...(failure.code !== undefined ? { code: failure.code } : {}), - timedOut: failure.timedOut, - ...(targetType ? { targetType } : {}), - origin: eventUrl(session.url, this.number), }); + this.logStepFailure("Fetch.enable", failure, session); await this.swallowStep( this.connection.send("Target.closeTarget", { targetId: session.targetId, @@ -462,6 +445,31 @@ class PaymentInterceptor { return false; } + private logStepFailure( + method: string, + error: unknown, + target?: { type: string; url: string }, + ): void { + if (this.closed) return; + const failed = error instanceof CdpCommandError ? error : undefined; + const targetType = + target?.type === "page" || target?.type === "iframe" + ? target.type + : undefined; + const code = failed?.code; + try { + this.logger.warn("browser step failed", { + method, + ...(code !== undefined ? { code } : {}), + timedOut: failed?.timedOut ?? false, + ...(targetType ? { targetType } : {}), + ...(target ? { origin: eventUrl(target.url, this.number) } : {}), + }); + } catch { + // A logger must not change control flow. + } + } + private async swallowStep( pending: Promise, method: string, @@ -470,24 +478,7 @@ class PaymentInterceptor { try { return await pending; } catch (error) { - if (this.closed) return undefined; - const failed = error instanceof CdpCommandError ? error : undefined; - const targetType = - target?.type === "page" || target?.type === "iframe" - ? target.type - : undefined; - const code = failed?.code; - try { - this.logger.warn("browser step failed", { - method, - ...(code !== undefined ? { code } : {}), - timedOut: failed?.timedOut ?? false, - ...(targetType ? { targetType } : {}), - ...(target ? { origin: eventUrl(target.url, this.number) } : {}), - }); - } catch { - // A logger must not change control flow. - } + this.logStepFailure(method, error, target); return undefined; } } @@ -1320,16 +1311,10 @@ function multipartOmitsFileBytes(body: string): boolean { return /content-disposition:[^\r\n]*[;\s]filename\s*\*?=/i.test(body); } -type ArmMiss = { - method: "Fetch.enable"; - code?: number; - timedOut: boolean; -}; - async function armFetch( connection: CdpConnection, session: Session, -): Promise { +): Promise { if (session.fetchEnabled) return; try { await connection @@ -1341,12 +1326,7 @@ async function armFetch( session.sessionId, ); } catch (error) { - const failed = error instanceof CdpCommandError ? error : undefined; - return { - method: "Fetch.enable", - timedOut: failed?.timedOut ?? false, - ...(failed?.code !== undefined ? { code: failed.code } : {}), - }; + return error instanceof Error ? error : new Error("CDP command failed"); } session.fetchEnabled = true; } From 0fe204268f225f51d3dab81884004df9b9ff7e79 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 27 Oct 2026 16:15:16 +0000 Subject: [PATCH 9/9] fix(browser-pay): swallow async step logs and record them before detach Co-authored-by: Rimmely Ewan --- .../src/interceptor.protocol.test.ts | 97 +++++++++++++++++++ packages/browser-pay/src/interceptor.ts | 18 ++-- 2 files changed, 107 insertions(+), 8 deletions(-) diff --git a/packages/browser-pay/src/interceptor.protocol.test.ts b/packages/browser-pay/src/interceptor.protocol.test.ts index 7d4643dce..976a71fe3 100644 --- a/packages/browser-pay/src/interceptor.protocol.test.ts +++ b/packages/browser-pay/src/interceptor.protocol.test.ts @@ -885,6 +885,103 @@ describe("interceptor CDP decisions", () => { ); }); + it("swallows a rejected async logger and still closes the target", async () => { + const stub = await startStub({ failFetch: true }); + const events: InterceptorEvent[] = []; + const unhandled: unknown[] = []; + const onUnhandled = (error: unknown) => { + unhandled.push(error); + }; + process.on("unhandledRejection", onUnhandled); + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + logger: { + async warn() { + throw new Error("ASYNC_LOGGER_REJECTION"); + }, + }, + }); + handle.on((event) => events.push(event)); + attachPage(stub.emit); + await waitFor( + () => + stub.commands.some( + (command) => command.method === "Target.closeTarget", + ), + "closeTarget", + ); + await new Promise((resolve) => setImmediate(resolve)); + expect(unhandled).toEqual([]); + expect(events).toStrictEqual([ + { + type: "failed", + url: "https://shop.example", + reason: "intercept_unavailable", + }, + ]); + expect(stub.commands).toContainEqual( + expect.objectContaining({ + method: "Target.closeTarget", + params: { targetId: "target-page" }, + }), + ); + await handle.detach(); + } finally { + process.off("unhandledRejection", onUnhandled); + await stub.close(); + } + }); + + it("logs Fetch.enable before a failed handler detaches", async () => { + const stub = await startStub({ failFetch: true }); + const events: InterceptorEvent[] = []; + const warnings: StepWarning[] = []; + try { + const handle = await attachInterceptor(stub.url, { + relay: stubPaymentRelay(async () => success()), + placeholder: HELD, + profileId: "shopify-card-vault", + logger: stepLogger(warnings), + }); + handle.on((event) => { + events.push(event); + if (event.type === "failed") void handle.detach(); + }); + attachPage(stub.emit); + await waitFor(() => warnings.length === 1, "Fetch.enable warning"); + expect(events).toStrictEqual([ + { + type: "failed", + url: "https://shop.example", + reason: "intercept_unavailable", + }, + ]); + expect(warnings).toStrictEqual([ + { + message: "browser step failed", + fields: { + method: "Fetch.enable", + timedOut: false, + targetType: "page", + origin: "https://shop.example", + }, + }, + ]); + await waitFor( + () => + stub.commands.some( + (command) => command.method === "Target.closeTarget", + ), + "closeTarget", + ); + } finally { + await stub.close(); + } + }); + it("reads a body with missing entry bytes via getRequestPostData and relays it", async () => { const stub = await startStub({ postData: CARD }); const calls: RelayRequest[] = []; diff --git a/packages/browser-pay/src/interceptor.ts b/packages/browser-pay/src/interceptor.ts index 183b9df74..1bcd4faae 100644 --- a/packages/browser-pay/src/interceptor.ts +++ b/packages/browser-pay/src/interceptor.ts @@ -429,12 +429,12 @@ class PaymentInterceptor { const failure = await armFetch(this.connection, session); if (!failure) return true; if (this.closed) return false; + this.logStepFailure("Fetch.enable", failure, session); this.emit({ type: "failed", url: session.url, reason: "intercept_unavailable", }); - this.logStepFailure("Fetch.enable", failure, session); await this.swallowStep( this.connection.send("Target.closeTarget", { targetId: session.targetId, @@ -458,13 +458,15 @@ class PaymentInterceptor { : undefined; const code = failed?.code; try { - this.logger.warn("browser step failed", { - method, - ...(code !== undefined ? { code } : {}), - timedOut: failed?.timedOut ?? false, - ...(targetType ? { targetType } : {}), - ...(target ? { origin: eventUrl(target.url, this.number) } : {}), - }); + void Promise.resolve( + this.logger.warn("browser step failed", { + method, + ...(code !== undefined ? { code } : {}), + timedOut: failed?.timedOut ?? false, + ...(targetType ? { targetType } : {}), + ...(target ? { origin: eventUrl(target.url, this.number) } : {}), + }), + ).catch(() => {}); } catch { // A logger must not change control flow. }