Skip to content

Commit 94e1a4f

Browse files
committed
fix: contain routed preview fetch failures
1 parent 95c3bb3 commit 94e1a4f

7 files changed

Lines changed: 249 additions & 36 deletions

File tree

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@
108108
"generate:cloudflare-canonical-mdi-seed": "npm run generate:cloudflare-mdi-runtime-bundle && php scripts/build-cloudflare-canonical-mdi-seed.php",
109109
"smoke": "tsx scripts/run-smoke.ts",
110110
"test:redaction": "tsx tests/redaction.test.ts",
111+
"test:browser-preview-routing": "tsx --test tests/browser-preview-routing.test.ts",
111112
"test:cloudflare-runtime": "node --test tests/cloudflare-d1-provisioner.test.mjs && tsx tests/cloudflare-site-context.test.ts && tsx tests/cloudflare-coordinator-site-partitioning.test.ts && tsx tests/cloudflare-d1-operation-repository.test.ts && tsx tests/cloudflare-provisioning-api.test.ts && tsx tests/cloudflare-runtime.test.ts && node ./node_modules/typescript/bin/tsc -p packages/runtime-cloudflare --noEmit",
112113
"test:cloudflare-administrator-claim": "tsx tests/cloudflare-provisioning-api.test.ts",
113114
"test:cloudflare-wordpress-auth": "tsx tests/cloudflare-wordpress-auth.test.ts",

packages/runtime-core/src/redaction.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,25 @@ export function redactJsonValue(value: unknown, options: RedactJsonOptions = {},
9696

9797
export function redactString(value: string, options: RedactStringOptions = {}): string {
9898
return value
99+
.replace(/(^|\r?\n)([ \t]*([A-Za-z0-9_-]+)[ \t]*:[ \t]*)[^\r\n]*/g, (line, prefix: string, assignment: string, key: string) => (
100+
isSensitiveKey(key, options) ? `${prefix}${assignment}${REDACTED_VALUE}` : line
101+
))
99102
.replace(/https?:\/\/[^\s"'<>]+/gi, (match) => redactUrl(match, options))
100103
.replace(SECRET_LIKE_VALUE_GLOBAL_PATTERN, REDACTED_VALUE)
101104
.replace(/([?&][^=&#\s"'<>]+)=([^&#\s"'<>]+)/g, options.redactQueryAssignments ? `$1=${REDACTED_VALUE}` : "$&")
102105
.replace(/((?:[A-Za-z0-9_-]*)(?:access[_-]?token|auth|bearer|code|cookie|credential|key|login|nonce|pass|password|secret|session|state|token)(?:[A-Za-z0-9_-]*)(?:["'\s:=]+))[^&#\s"'<>]+/gi, `$1${REDACTED_VALUE}`)
103106
}
104107

108+
export function redactError(error: unknown, options: RedactStringOptions = {}): Error {
109+
const source = error instanceof Error ? error : new Error(String(error))
110+
const redacted = new Error(redactString(source.message, options))
111+
redacted.name = redactString(source.name, options)
112+
if (source.stack) {
113+
redacted.stack = redactString(source.stack, options)
114+
}
115+
return redacted
116+
}
117+
105118
export function redactUrl(value: string, options: RedactStringOptions = {}): string {
106119
try {
107120
const url = new URL(value)

packages/runtime-playground/src/browser-metrics.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createHash } from "node:crypto"
22
import { access, readFile } from "node:fs/promises"
33
import { join } from "node:path"
4-
import { redactString, type ArtifactManifest } from "@automattic/wp-codebox-core"
4+
import { redactError, redactString, type ArtifactManifest } from "@automattic/wp-codebox-core"
55
import { isPlainObject as isRecord, now } from "@automattic/wp-codebox-core/internals"
66
import type { ConsoleMessage, Request, Response } from "playwright"
77
import type {
@@ -636,11 +636,8 @@ export function serializeBrowserConsoleMessage(message: ConsoleMessage): Record<
636636
}
637637

638638
export function serializeBrowserError(type: BrowserProbeErrorRecord["type"], error: unknown): BrowserProbeErrorRecord {
639-
if (error instanceof Error) {
640-
return { type, name: error.name, message: error.message, stack: error.stack, timestamp: now() }
641-
}
642-
643-
return { type, name: "Error", message: String(error), timestamp: now() }
639+
const sanitized = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
640+
return { type, name: sanitized.name, message: sanitized.message, stack: sanitized.stack, timestamp: now() }
644641
}
645642

646643
export function jsonLines(records: unknown[]): string {

packages/runtime-playground/src/browser-preview-routing.ts

Lines changed: 61 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1-
import type { RuntimeCreateSpec } from "@automattic/wp-codebox-core"
1+
import { redactError, redactString, type RuntimeCreateSpec } from "@automattic/wp-codebox-core"
22
import type { BrowserProbeNetworkPolicySummary, BrowserProbePreviewMode, BrowserProbePreviewRouting } from "./browser-artifacts.js"
33
import { argValue, commaListArg, strictBooleanArg } from "./commands.js"
44
import type { Page, Route } from "playwright"
55

66
const BROWSER_PREVIEW_ROUTE_DRAIN_TIMEOUT_MS = 5_000
7+
const BROWSER_PREVIEW_ROUTE_DOCUMENT_FETCH_ATTEMPTS = 3
8+
const BROWSER_PREVIEW_ROUTE_SUBRESOURCE_FETCH_ATTEMPTS = 2
9+
const BROWSER_PREVIEW_ROUTE_RETRY_DELAY_MS = 25
710

811
export interface BrowserPreviewNetworkPolicy {
912
mode: "allow" | "block" | "record"
@@ -258,8 +261,7 @@ export async function drainBrowserPreviewRouteTracker(tracker: BrowserPreviewRou
258261
}
259262

260263
if (tracker.errors.length > 0) {
261-
const error = tracker.errors[0]
262-
throw error instanceof Error ? error : new Error(String(error))
264+
throw sanitizeBrowserPreviewRouteError(tracker.errors[0])
263265
}
264266
}
265267

@@ -310,8 +312,8 @@ async function routeBrowserPreviewNetwork(routePattern: (url: string, handler: (
310312
try {
311313
await task
312314
} catch (error) {
313-
tracker?.errors.push(error)
314-
throw error
315+
tracker?.errors.push(sanitizeBrowserPreviewRouteError(error))
316+
await route.abort("failed").catch(() => undefined)
315317
} finally {
316318
tracker?.pending.delete(task)
317319
}
@@ -400,25 +402,42 @@ async function fetchBrowserPreviewRoutedHost(route: Route, requestUrl: URL, poli
400402
routedUrl.hostname = origin.hostname
401403
routedUrl.port = origin.port
402404

403-
let response: Awaited<ReturnType<Route["fetch"]>>
404-
try {
405-
response = await route.fetch({
406-
url: routedUrl.toString(),
407-
headers: {
408-
...route.request().headers(),
409-
host: currentUrl.host,
410-
"x-forwarded-host": currentUrl.host,
411-
"x-forwarded-port": currentUrl.port || (currentUrl.protocol === "https:" ? "443" : "80"),
412-
"x-forwarded-proto": currentUrl.protocol.replace(":", ""),
413-
},
414-
maxRedirects: 0,
415-
})
416-
} catch (error) {
417-
if (!isBrowserPreviewRouteFetchRecoverableError(error)) {
418-
throw error
405+
let response: Awaited<ReturnType<Route["fetch"]>> | undefined
406+
const resourceType = route.request().resourceType()
407+
const maxAttempts = resourceType === "document" ? BROWSER_PREVIEW_ROUTE_DOCUMENT_FETCH_ATTEMPTS : BROWSER_PREVIEW_ROUTE_SUBRESOURCE_FETCH_ATTEMPTS
408+
for (let attempt = 1; attempt <= maxAttempts; attempt += 1) {
409+
try {
410+
response = await route.fetch({
411+
url: routedUrl.toString(),
412+
headers: {
413+
...route.request().headers(),
414+
host: currentUrl.host,
415+
"x-forwarded-host": currentUrl.host,
416+
"x-forwarded-port": currentUrl.port || (currentUrl.protocol === "https:" ? "443" : "80"),
417+
"x-forwarded-proto": currentUrl.protocol.replace(":", ""),
418+
},
419+
maxRedirects: 0,
420+
})
421+
break
422+
} catch (error) {
423+
if (!isBrowserPreviewRouteFetchRecoverableError(error)) {
424+
throw sanitizeBrowserPreviewRouteError(error)
425+
}
426+
427+
const retryable = isBrowserPreviewRouteFetchTransientTransportError(error)
428+
if (retryable && attempt < maxAttempts) {
429+
await wait(BROWSER_PREVIEW_ROUTE_RETRY_DELAY_MS * attempt)
430+
continue
431+
}
432+
433+
if (resourceType !== "document" || !retryable) {
434+
await route.abort("failed").catch(() => undefined)
435+
return undefined
436+
}
437+
throw browserPreviewRouteFetchExhaustedError(route, currentUrl, attempt, error)
419438
}
420-
421-
await route.abort("failed").catch(() => undefined)
439+
}
440+
if (!response) {
422441
return undefined
423442
}
424443

@@ -458,13 +477,31 @@ export function isBrowserPreviewRouteFetchRequestContextDisposedError(error: unk
458477
}
459478

460479
export function isBrowserPreviewRouteFetchRecoverableError(error: unknown): boolean {
461-
return isBrowserPreviewRouteFetchRequestContextDisposedError(error) || isBrowserPreviewRouteFetchContentDecodingError(error)
480+
return isBrowserPreviewRouteFetchRequestContextDisposedError(error) || isBrowserPreviewRouteFetchContentDecodingError(error) || isBrowserPreviewRouteFetchTransientTransportError(error)
462481
}
463482

464483
export function isBrowserPreviewRouteFetchContentDecodingError(error: unknown): boolean {
465484
return error instanceof Error && /\broute\.fetch:\s*failed to decompress\b/i.test(error.message)
466485
}
467486

487+
export function isBrowserPreviewRouteFetchTransientTransportError(error: unknown): boolean {
488+
return error instanceof Error && /\b(?:ECONNRESET|ECONNREFUSED|EPIPE|ETIMEDOUT|UND_ERR_SOCKET|socket (?:hang up|closed|ended)|connection (?:reset|refused|closed)|other side closed)\b/i.test(error.message)
489+
}
490+
491+
function browserPreviewRouteFetchExhaustedError(route: Route, requestUrl: URL, attempts: number, error: unknown): Error {
492+
const method = route.request().method()
493+
const resourceType = route.request().resourceType()
494+
const classification = isBrowserPreviewRouteFetchTransientTransportError(error) ? "upstream-transport" : "route-fetch"
495+
const safeUrl = redactString(requestUrl.toString(), { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
496+
const exhausted = new Error(`wordpress.browser-probe route-host fetch failed after ${attempts} attempt(s): classification=${classification} method=${method} resourceType=${resourceType} url=${safeUrl}`)
497+
exhausted.name = "BrowserPreviewRouteFetchError"
498+
return exhausted
499+
}
500+
501+
function sanitizeBrowserPreviewRouteError(error: unknown): Error {
502+
return redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
503+
}
504+
468505
function urlProtocol(url: string): string | undefined {
469506
try {
470507
return new URL(url).protocol

packages/runtime-playground/src/browser-probe-runner.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { BROWSER_PROBE_BROWSER_VALUES, BROWSER_PROBE_CAPTURE_VALUES, BROWSER_PROBE_CHROMIUM_PROFILE_IDS, BROWSER_PROBE_PROFILES, BROWSER_PROBE_THROTTLE_PROFILE_IDS, browserGeolocation, type BrowserGeolocationPermissionState, type BrowserProbeProfileDefinition, type ExecutionSpec, type RuntimeCreateSpec } from "@automattic/wp-codebox-core"
1+
import { BROWSER_PROBE_BROWSER_VALUES, BROWSER_PROBE_CAPTURE_VALUES, BROWSER_PROBE_CHROMIUM_PROFILE_IDS, BROWSER_PROBE_PROFILES, BROWSER_PROBE_THROTTLE_PROFILE_IDS, browserGeolocation, redactError, type BrowserGeolocationPermissionState, type BrowserProbeProfileDefinition, type ExecutionSpec, type RuntimeCreateSpec } from "@automattic/wp-codebox-core"
22
import { BrowserArtifactSession } from "./browser-artifact-session.js"
33
import { BrowserCommandArtifactError } from "./browser-command-artifact-error.js"
44
import type { BrowserArtifactFiles, BrowserProbeArtifact, BrowserProbeAuthSummary, BrowserProbeCapabilityDiagnostics, BrowserProbeCheckpointRecord, BrowserProbeContextDetails, BrowserProbeErrorRecord, BrowserProbeLifecycleArtifact, BrowserProbeMemoryArtifact, BrowserProbeNetworkRecord, BrowserProbePerformanceArtifact, BrowserProbeScriptMetadata, BrowserProbeViewport, BrowserProbeWebSocketRecord, BrowserWordPressDiagnosticsSummary } from "./browser-artifacts.js"
@@ -440,8 +440,9 @@ export async function runSingleBrowserProbeCommand({
440440
}
441441
finalUrl = page.url()
442442
} catch (error) {
443-
pendingError = error instanceof Error ? error : new Error(String(error))
444-
if (isBrowserCommandLivenessError(pendingError)) {
443+
const livenessError = isBrowserCommandLivenessError(error)
444+
pendingError = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
445+
if (livenessError) {
445446
await page?.close().catch(() => undefined)
446447
page = null
447448
}
@@ -456,7 +457,7 @@ export async function runSingleBrowserProbeCommand({
456457
try {
457458
await drainBrowserPreviewRouteTracker(routeTracker)
458459
} catch (error) {
459-
const routeError = error instanceof Error ? error : new Error(String(error))
460+
const routeError = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
460461
if (!pendingError && runPlan.routeHostDrain === "required") {
461462
pendingError = routeError
462463
progress.fail("probe-error", routeError)
@@ -674,7 +675,7 @@ export async function runBoundedBrowserDiagnostic<T>({
674675
})
675676
return { ok: true, value }
676677
} catch (error) {
677-
const normalized = error instanceof Error ? error : new Error(String(error))
678+
const normalized = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
678679
onError(normalized)
679680
return { ok: false, error: normalized }
680681
}
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
import assert from "node:assert/strict"
2+
import test from "node:test"
3+
4+
import type { BrowserContext, Route } from "playwright"
5+
6+
import { browserPreviewNetworkPolicy, browserPreviewRouting, createBrowserPreviewRouteTracker, drainBrowserPreviewRouteTracker, isBrowserPreviewRouteFetchContentDecodingError, isBrowserPreviewRouteFetchRecoverableError, isBrowserPreviewRouteFetchRequestContextDisposedError, isBrowserPreviewRouteFetchTransientTransportError, routeBrowserPreviewContextNetwork } from "../packages/runtime-playground/src/browser-preview-routing.js"
7+
import { jsonLines, serializeBrowserError } from "../packages/runtime-playground/src/browser-metrics.js"
8+
9+
const SENTINELS = ["SENTINEL_COOKIE_2094", "SENTINEL_AUTH_2094", "SENTINEL_NONCE_2094", "SENTINEL_TOKEN_2094"]
10+
const ROUTED_URL = "http://routed.test/wp-includes/app.js?token=SENTINEL_TOKEN_2094"
11+
12+
test("routed fetch classifiers include transport resets and preserve disposal/decompression recovery", () => {
13+
assert.equal(isBrowserPreviewRouteFetchTransientTransportError(routeFetchError("read ECONNRESET")), true)
14+
assert.equal(isBrowserPreviewRouteFetchTransientTransportError(routeFetchError("connect ECONNREFUSED")), true)
15+
assert.equal(isBrowserPreviewRouteFetchTransientTransportError(routeFetchError("socket hang up")), true)
16+
assert.equal(isBrowserPreviewRouteFetchRequestContextDisposedError(routeFetchError("Request context disposed.")), true)
17+
assert.equal(isBrowserPreviewRouteFetchContentDecodingError(routeFetchError("failed to decompress 'br' encoding")), true)
18+
assert.equal(isBrowserPreviewRouteFetchRecoverableError(routeFetchError("read ECONNRESET")), true)
19+
})
20+
21+
test("subresource resets retry once, abort safely, and never reject the route callback", async () => {
22+
const fixture = await routedFixture("script", [routeFetchError("read ECONNRESET"), routeFetchError("socket closed")])
23+
await assert.doesNotReject(fixture.run())
24+
assert.equal(fixture.fetchCalls(), 2)
25+
assert.equal(fixture.abortCalls(), 1)
26+
assert.equal(fixture.tracker.errors.length, 0)
27+
assert.equal(fixture.tracker.pending.size, 0)
28+
})
29+
30+
test("document resets retry deterministically and fulfill after recovery", async () => {
31+
const response = routedResponse()
32+
const fixture = await routedFixture("document", [routeFetchError("connect ECONNREFUSED"), routeFetchError("read ECONNRESET"), response])
33+
await assert.doesNotReject(fixture.run())
34+
await assert.doesNotReject(drainBrowserPreviewRouteTracker(fixture.tracker))
35+
assert.equal(fixture.fetchCalls(), 3)
36+
assert.equal(fixture.fulfilledResponse(), response)
37+
assert.equal(fixture.abortCalls(), 0)
38+
})
39+
40+
test("exhausted document resets fail through a sanitized tracker error without leaking to persisted surfaces", async () => {
41+
const fixture = await routedFixture("document", [routeFetchError("read ECONNRESET"), routeFetchError("read ECONNRESET"), routeFetchError("read ECONNRESET")])
42+
await assert.doesNotReject(fixture.run())
43+
assert.equal(fixture.fetchCalls(), 3)
44+
assert.equal(fixture.abortCalls(), 1)
45+
assert.equal(fixture.tracker.pending.size, 0)
46+
assert.equal(fixture.tracker.errors.length, 1)
47+
48+
const tracked = fixture.tracker.errors[0]
49+
await assert.rejects(drainBrowserPreviewRouteTracker(fixture.tracker), /classification=upstream-transport.*resourceType=document.*token=\[redacted\]/)
50+
const serialized = serializeBrowserError("probe-error", tracked)
51+
const persistedSurfaces = {
52+
stdout: JSON.stringify(serialized),
53+
stderr: tracked instanceof Error ? `${tracked.message}\n${tracked.stack}` : String(tracked),
54+
diagnostics: JSON.stringify({ errors: [serialized] }),
55+
manifest: JSON.stringify({ files: [{ diagnostics: serialized }] }),
56+
artifact: jsonLines([serialized]),
57+
tracker: JSON.stringify(fixture.tracker.errors.map((error) => serializeBrowserError("probe-error", error))),
58+
snapshot: JSON.stringify(serialized),
59+
}
60+
for (const [surface, contents] of Object.entries(persistedSurfaces)) {
61+
for (const sentinel of SENTINELS) assert.doesNotMatch(contents, new RegExp(sentinel), `${surface} must not contain ${sentinel}`)
62+
}
63+
})
64+
65+
test("concurrent routed requests drain after independent retry and cleanup", async () => {
66+
const first = await routedFixture("script", [routeFetchError("read ECONNRESET"), routedResponse()])
67+
const second = await routedFixture("image", [routeFetchError("socket ended"), routeFetchError("socket ended")], first.tracker)
68+
await Promise.all([first.run(), second.run()])
69+
await assert.doesNotReject(drainBrowserPreviewRouteTracker(first.tracker))
70+
assert.equal(first.tracker.pending.size, 0)
71+
assert.equal(first.fetchCalls(), 2)
72+
assert.equal(second.fetchCalls(), 2)
73+
assert.equal(second.abortCalls(), 1)
74+
})
75+
76+
test("disposed contexts and decompression failures abort without retrying or tracking errors", async () => {
77+
for (const message of ["Request context disposed.", "failed to decompress 'gzip' encoding"]) {
78+
const fixture = await routedFixture("document", [routeFetchError(message)])
79+
await assert.doesNotReject(fixture.run())
80+
await assert.doesNotReject(drainBrowserPreviewRouteTracker(fixture.tracker))
81+
assert.equal(fixture.fetchCalls(), 1)
82+
assert.equal(fixture.abortCalls(), 1)
83+
}
84+
})
85+
86+
async function routedFixture(resourceType: string, outcomes: unknown[], tracker = createBrowserPreviewRouteTracker()) {
87+
let handler: ((route: Route) => Promise<void>) | undefined
88+
let fetchCalls = 0
89+
let abortCalls = 0
90+
let fulfilled: unknown
91+
const context = {
92+
route: async (_pattern: string, nextHandler: (route: Route) => Promise<void>) => {
93+
handler = nextHandler
94+
},
95+
} as BrowserContext
96+
const preview = browserPreviewRouting([], undefined, "http://127.0.0.1:9400")
97+
const policy = browserPreviewNetworkPolicy([], ["routed.test"], preview)
98+
await routeBrowserPreviewContextNetwork(context, policy, preview.effectiveOrigin, tracker)
99+
100+
const route = {
101+
request: () => ({
102+
url: () => ROUTED_URL,
103+
method: () => "GET",
104+
resourceType: () => resourceType,
105+
headers: () => ({
106+
cookie: `wordpress_logged_in=${SENTINELS[0]}`,
107+
authorization: `Bearer ${SENTINELS[1]}`,
108+
"x-wp-nonce": SENTINELS[2],
109+
"x-session-token": SENTINELS[3],
110+
}),
111+
}),
112+
fetch: async () => {
113+
const outcome = outcomes[Math.min(fetchCalls, outcomes.length - 1)]
114+
fetchCalls += 1
115+
if (outcome instanceof Error) throw outcome
116+
return outcome
117+
},
118+
abort: async () => {
119+
abortCalls += 1
120+
},
121+
fulfill: async ({ response }: { response: unknown }) => {
122+
fulfilled = response
123+
},
124+
continue: async () => {},
125+
} as unknown as Route
126+
127+
return {
128+
tracker,
129+
run: async () => {
130+
assert(handler)
131+
await handler(route)
132+
},
133+
fetchCalls: () => fetchCalls,
134+
abortCalls: () => abortCalls,
135+
fulfilledResponse: () => fulfilled,
136+
}
137+
}
138+
139+
function routeFetchError(reason: string): Error {
140+
const message = `route.fetch: ${reason}\nCall log:\n - → GET ${ROUTED_URL}\n cookie: wordpress_logged_in=${SENTINELS[0]}\n authorization: Bearer ${SENTINELS[1]}\n x-wp-nonce: ${SENTINELS[2]}\n x-session-token: ${SENTINELS[3]}`
141+
const error = new Error(message)
142+
error.stack = `Error: ${message}`
143+
return error
144+
}
145+
146+
function routedResponse() {
147+
return {
148+
status: () => 200,
149+
headers: () => ({}),
150+
}
151+
}

0 commit comments

Comments
 (0)