From 3d70e0da86f1ee2b3792d0f44cb5f2fcb5a46aeb Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Tue, 20 Oct 2026 15:15:28 +0300 Subject: [PATCH] fix: announce demo browser only after CDP is ready --- electron/browser-service-readiness.cjs | 94 +++++++++++++ electron/main.cjs | 22 ++- scripts/smoke-demo-runtime.mjs | 20 ++- tests/browser-service-readiness.test.ts | 170 ++++++++++++++++++++++++ tests/demo-runtime-payload.test.ts | 15 ++- 5 files changed, 312 insertions(+), 9 deletions(-) create mode 100644 electron/browser-service-readiness.cjs create mode 100644 tests/browser-service-readiness.test.ts diff --git a/electron/browser-service-readiness.cjs b/electron/browser-service-readiness.cjs new file mode 100644 index 0000000..847328a --- /dev/null +++ b/electron/browser-service-readiness.cjs @@ -0,0 +1,94 @@ +// No Electron import: the shipping helper's readiness contract is testable +// without a display. Never announce CDP until its exact loopback endpoint works. +const http = require("node:http"); + +/** + * @param {number} port + * @param {{signal?: AbortSignal, timeoutMs?: number, requestTimeoutMs?: number, retryMs?: number}} [options] + * @returns {Promise} + */ +function waitForBrowserService(port, { + signal, + timeoutMs = 5000, + requestTimeoutMs = 500, + retryMs = 50, +} = {}) { + if (!Number.isInteger(port) || port < 1 || port > 65535) { + return Promise.reject(new Error("browser-service: invalid loopback port")); + } + return new Promise((resolve, reject) => { + let settled = false; + let activeRequest; + let attemptTimer; + let retryTimer; + let lastError; + const finish = (error, endpoint) => { + if (settled) return; + settled = true; + clearTimeout(deadline); + clearTimeout(attemptTimer); + clearTimeout(retryTimer); + signal?.removeEventListener("abort", abort); + activeRequest?.destroy(); + if (error) reject(error); + else resolve(endpoint); + }; + const abort = () => finish(signal.reason instanceof Error ? signal.reason : new Error("browser-service readiness aborted")); + const deadline = setTimeout(() => finish(new Error( + `browser-service: http://127.0.0.1:${port}/json/version not ready within ${timeoutMs}ms`, + { cause: lastError }, + )), timeoutMs); + const attempt = () => { + if (settled) return; + let done = false; + let response; + const complete = (error, endpoint) => { + if (done || settled) return; + done = true; + clearTimeout(attemptTimer); + response?.destroy(); + request.destroy(); + activeRequest = undefined; + if (!error) return finish(null, endpoint); + lastError = error; + retryTimer = setTimeout(attempt, retryMs); + }; + const request = http.get({ + hostname: "127.0.0.1", port, path: "/json/version", agent: false, + }, (res) => { + response = res; + res.on("error", (error) => complete(error)); + if (res.statusCode !== 200) return complete(new Error(`CDP HTTP status ${res.statusCode}`)); + const chunks = []; + let bytes = 0; + res.on("data", (chunk) => { + if (done || settled) return; + bytes += chunk.length; + if (bytes > 16 * 1024) return complete(new Error("CDP response exceeds 16 KiB")); + chunks.push(chunk); + }); + res.on("end", () => { + try { + const info = JSON.parse(Buffer.concat(chunks).toString("utf8")); + const endpoint = new URL(info.webSocketDebuggerUrl); + if (endpoint.protocol !== "ws:" || endpoint.hostname !== "127.0.0.1" || endpoint.port !== String(port) || + endpoint.username || endpoint.password || endpoint.search || endpoint.hash || + endpoint.href !== `ws://127.0.0.1:${port}${endpoint.pathname}` || + !/^\/devtools\/browser\/[A-Za-z0-9-]+$/.test(endpoint.pathname)) { + throw new Error("CDP response has an invalid browser WebSocket endpoint"); + } + complete(null, endpoint.href); + } catch (error) { complete(error); } + }); + }); + activeRequest = request; + request.on("error", (error) => complete(error)); + attemptTimer = setTimeout(() => complete(new Error("CDP HTTP request timed out")), requestTimeoutMs); + }; + signal?.addEventListener("abort", abort, { once: true }); + if (signal?.aborted) abort(); + else attempt(); + }); +} + +module.exports = { waitForBrowserService }; diff --git a/electron/main.cjs b/electron/main.cjs index c92671b..914eac5 100644 --- a/electron/main.cjs +++ b/electron/main.cjs @@ -69,6 +69,16 @@ function serverEntry() { // instance of this very binary, and before the LH_DEV_URL redirect — the // helper owns no user data at all. if (process.argv.includes("--browser-service")) { + const { waitForBrowserService } = require("./browser-service-readiness.cjs"); + // Own the helper's entire startup lifetime, including the readiness wait. + const lifetime = new AbortController(); + const parentClosed = () => { + lifetime.abort(new Error("browser-service parent stdin closed")); + app.quit(); + }; + process.stdin.resume(); + process.stdin.on("end", parentClosed); + process.stdin.on("close", parentClosed); (async () => { try { app.dock?.hide(); @@ -80,13 +90,13 @@ if (process.argv.includes("--browser-service")) { // platform (no window-all-closed handler is installed here). const win = new BrowserWindow({ show: false }); await win.loadURL("about:blank"); - console.log(`BROWSER_SERVICE_CDP ${port}`); - // Parent-tied lifetime: the server holds our stdin open. - process.stdin.resume(); - process.stdin.on("end", () => app.quit()); - process.stdin.on("close", () => app.quit()); + // Window load is not a DevTools HTTP readiness barrier. Confirm the + // endpoint before both production and smoke clients see the handshake. + await waitForBrowserService(port, { signal: lifetime.signal }); + if (!lifetime.signal.aborted) console.log(`BROWSER_SERVICE_CDP ${port}`); } catch (err) { - console.error(`browser-service failed: ${err?.stack ?? err}`); + if (lifetime.signal.aborted) return; // EOF is a normal shutdown. + console.error("browser-service failed:", err); app.exit(1); } })(); diff --git a/scripts/smoke-demo-runtime.mjs b/scripts/smoke-demo-runtime.mjs index 4030111..04046bb 100644 --- a/scripts/smoke-demo-runtime.mjs +++ b/scripts/smoke-demo-runtime.mjs @@ -30,6 +30,22 @@ export function probeEnv(env, home) { }; } +// Keep fetch's nested cause (including AggregateError socket failures), but +// never dump arbitrary error properties such as request headers or environment. +export function formatProbeError(error, depth = 0) { + if (depth > 5) return "[cause chain truncated]"; + if (!error || typeof error !== "object") return String(error); + const lines = [typeof error.stack === "string" ? error.stack : `${error.name ?? "Error"}: ${error.message ?? "unknown error"}`]; + for (const field of ["code", "errno", "syscall", "address", "port"]) { + if (["string", "number"].includes(typeof error[field])) lines.push(`${field}: ${error[field]}`); + } + if (error.cause !== undefined) lines.push(`cause: ${formatProbeError(error.cause, depth + 1)}`); + if (Array.isArray(error.errors)) { + for (const nested of error.errors.slice(0, 8)) lines.push(`aggregate: ${formatProbeError(nested, depth + 1)}`); + } + return lines.join("\n"); +} + export function verifyDemoRuntime(standalone, runtime = process.execPath, browser = false) { const home = fs.mkdtempSync(path.join(os.tmpdir(), "lh-demo-runtime-")); try { @@ -136,7 +152,7 @@ async function browserProbe(puppeteer) { assert.deepEqual(failures, [], "demo console/page/network errors"); console.log("[smoke-demo-runtime] packaged browser-service: JS button 0 → 1, clean console/page, page reset ok (no Chrome fallback)"); } catch (err) { - throw new Error(`${err.stack ?? err}\n--- browser-service output ---\n${output}\n${errors}`); + throw new Error(`browser-service probe failed at local CDP/browser interaction\n--- browser-service output ---\n${output}\n${errors}`, { cause: err }); } finally { browser?.disconnect(); helper.stdin.end(); // The shipping lifetime contract: EOF must quit the helper. @@ -175,5 +191,5 @@ async function main() { } if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { - main().catch((err) => { console.error(`[smoke-demo-runtime] FAIL: ${err.stack ?? err}`); process.exitCode = 1; }); + main().catch((err) => { console.error(`[smoke-demo-runtime] FAIL: ${formatProbeError(err)}`); process.exitCode = 1; }); } diff --git a/tests/browser-service-readiness.test.ts b/tests/browser-service-readiness.test.ts new file mode 100644 index 0000000..cfaef5a --- /dev/null +++ b/tests/browser-service-readiness.test.ts @@ -0,0 +1,170 @@ +/** Shipping helper readiness, with real loopback HTTP but no Electron/display. */ +import assert from "node:assert/strict"; +import * as http from "node:http"; +import type { AddressInfo, Socket } from "node:net"; +import { describe, it } from "node:test"; +import { setTimeout as delay } from "node:timers/promises"; +import { waitForBrowserService } from "../electron/browser-service-readiness.cjs"; + +const endpoint = (port: number) => `ws://127.0.0.1:${port}/devtools/browser/test-browser-id`; +const version = (port: number) => JSON.stringify({ webSocketDebuggerUrl: endpoint(port) }); +const short = { timeoutMs: 250, requestTimeoutMs: 50, retryMs: 10 }; + +async function withEndpoint( + reply: (request: http.IncomingMessage, response: http.ServerResponse, port: number) => void, + test: (port: number, idle: () => Promise) => Promise, +) { + let port = 0; + const sockets = new Set(); + const server = http.createServer((request, response) => reply(request, response, port)); + server.on("connection", (socket) => { + sockets.add(socket); + socket.once("close", () => sockets.delete(socket)); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + port = (server.address() as AddressInfo).port; + const idle = async () => { + const deadline = Date.now() + 1000; + while (sockets.size && Date.now() < deadline) await delay(10); + assert.equal(sockets.size, 0, "readiness left an HTTP connection open"); + }; + try { await test(port, idle); } + finally { + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + } +} + +describe("browser-service shipping readiness", () => { + it("waits for a TCP endpoint that starts listening after readiness began", async () => { + const server = http.createServer((_req, res) => res.end(version(port))); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const port = (server.address() as AddressInfo).port; + await new Promise((resolve) => server.close(() => resolve())); + let ready = false; + const waiting = waitForBrowserService(port, { timeoutMs: 2000, requestTimeoutMs: 500, retryMs: 10 }).then((value) => { ready = true; return value; }); + try { + await delay(40); + assert.equal(ready, false, "announced readiness before the endpoint listened"); + await new Promise((resolve) => server.listen(port, "127.0.0.1", resolve)); + assert.equal(await waiting, endpoint(port)); + } finally { + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + } + }); + + it("waits through delayed/non-200 startup and only accepts the exact CDP endpoint", async () => { + let requests = 0; + await withEndpoint((req, res, port) => { + assert.equal(req.url, "/json/version"); + assert.equal(req.headers.host, `127.0.0.1:${port}`); + if (++requests < 3) { res.writeHead(503); res.end(); } + else setTimeout(() => res.end(version(port)), 20); + }, async (port, idle) => { + assert.equal(await waitForBrowserService(port, { ...short, timeoutMs: 2000 }), endpoint(port)); + assert.equal(requests, 3); + await idle(); + await delay(40); + assert.equal(requests, 3, "polling continued after readiness"); + }); + }); + + it("retries malformed JSON and a missing browser WebSocket endpoint", async () => { + let requests = 0; + await withEndpoint((_req, res, port) => { + requests++; + res.end(requests === 1 ? "not json" : requests === 2 ? "{}" : version(port)); + }, async (port, idle) => { + assert.equal(await waitForBrowserService(port, { ...short, timeoutMs: 2000 }), endpoint(port)); + assert.equal(requests, 3); + await idle(); + }); + }); + + it("rejects arbitrary, wrong-port, credentialed, and non-browser WebSocket URLs", async () => { + const invalid = [ + "ws://example.com:1234/devtools/browser/id", + "ws://127.0.0.1:1/devtools/browser/id", + "wss://127.0.0.1:1234/devtools/browser/id", + "ws://user:password@127.0.0.1:1234/devtools/browser/id", + "ws://127.0.0.1:1234/devtools/page/id", + "ws://127.0.0.1:1234/devtools/browser/id?query=1", + "ws://127.0.0.1:1234/devtools/browser/id#fragment", + "ws://127.0.0.1:1234/devtools/browser/", + ]; + let requests = 0; + await withEndpoint((_req, res, port) => { + const bad = invalid[requests++]; + res.end(bad ? JSON.stringify({ webSocketDebuggerUrl: bad.replaceAll(":1234", `:${port}`) }) : version(port)); + }, async (port, idle) => { + assert.equal(await waitForBrowserService(port, { ...short, timeoutMs: 2000 }), endpoint(port)); + assert.equal(requests, invalid.length + 1); + await idle(); + }); + }); + + it("bounds response size and retries instead of accepting oversized data", async () => { + let requests = 0; + await withEndpoint((_req, res, port) => { + res.end(++requests === 1 ? "x".repeat(16 * 1024 + 1) : version(port)); + }, async (port, idle) => { + assert.equal(await waitForBrowserService(port, { ...short, timeoutMs: 2000 }), endpoint(port)); + assert.equal(requests, 2); + await idle(); + }); + }); + + it("fails within the readiness budget and retains the last HTTP failure", async () => { + let requests = 0; + await withEndpoint((_req, res) => { requests++; res.writeHead(503); res.end(); }, async (port, idle) => { + const start = Date.now(); + await assert.rejects(waitForBrowserService(port, short), (error: Error) => { + assert.match(error.message, /not ready within 250ms/); + assert.match(String(error.cause), /CDP HTTP status 503/); + return true; + }); + assert.ok(Date.now() - start < 2000, "readiness exceeded its bounded budget"); + await idle(); + const stopped = requests; + await delay(40); + assert.equal(requests, stopped, "polling continued after timeout"); + }); + }); + + it("destroys hung requests on the per-request and overall timeouts", async () => { + let requests = 0; + await withEndpoint(() => { requests++; }, async (port, idle) => { + await assert.rejects(waitForBrowserService(port, short), /not ready within/); + assert.ok(requests >= 2, "hung attempt was not bounded before the overall deadline"); + await idle(); + }); + }); + + it("aborts a hung request on parent EOF and leaves no polling/connection behind", async () => { + const controller = new AbortController(); + let started!: () => void; + const requestStarted = new Promise((resolve) => { started = resolve; }); + let requests = 0; + await withEndpoint(() => { requests++; started(); }, async (port, idle) => { + const waiting = waitForBrowserService(port, { signal: controller.signal }); + await requestStarted; + controller.abort(new Error("parent EOF")); + await assert.rejects(waiting, /parent EOF/); + await idle(); + await delay(80); + assert.equal(requests, 1); + }); + }); + + it("does not start any request if the parent already closed", async () => { + const controller = new AbortController(); + controller.abort(new Error("parent EOF")); + let requests = 0; + await withEndpoint(() => { requests++; }, async (port) => { + await assert.rejects(waitForBrowserService(port, { signal: controller.signal }), /parent EOF/); + await delay(40); + assert.equal(requests, 0); + }); + }); +}); diff --git a/tests/demo-runtime-payload.test.ts b/tests/demo-runtime-payload.test.ts index eeca601..2460f12 100644 --- a/tests/demo-runtime-payload.test.ts +++ b/tests/demo-runtime-payload.test.ts @@ -6,7 +6,7 @@ import * as os from "node:os"; import * as path from "node:path"; import { describe, it } from "node:test"; import { fileURLToPath, pathToFileURL } from "node:url"; -import { probeEnv } from "../scripts/smoke-demo-runtime.mjs"; +import { formatProbeError, probeEnv } from "../scripts/smoke-demo-runtime.mjs"; const script = fileURLToPath(new URL("../scripts/smoke-demo-runtime.mjs", import.meta.url)); @@ -80,6 +80,19 @@ describe("demo standalone dependency gate", () => { }); }); + it("preserves nested fetch/socket causes without dumping headers or environment", () => { + const socket = Object.assign(new Error("connect ECONNREFUSED"), { code: "ECONNREFUSED", address: "127.0.0.1", port: 54321 }); + const cause = new AggregateError([socket], "connection failed"); + const error = Object.assign(new TypeError("fetch failed", { cause }), { headers: { authorization: "private-header" }, env: { TOKEN: "private-token" } }); + const output = formatProbeError(error); + assert.match(output, /fetch failed/); + assert.match(output, /connection failed/); + assert.match(output, /ECONNREFUSED/); + assert.match(output, /127\.0\.0\.1/); + assert.match(output, /54321/); + assert.doesNotMatch(output, /private-header|private-token|authorization|TOKEN/); + }); + it("strips secrets and uses only scratch data/auth/profile locations", () => { const original = { PATH: "/bin", API_KEY: "fake", GH_TOKEN: "fake", MY_SECRET: "fake", PASSWORD: "fake", NODE_OPTIONS: "--require anything", NODE_PATH: "/outside", ELECTRON_RUN_AS_NODE: "1", LH_DEV_URL: "http://localhost", LH_DATA_DIR: "/real", LH_AUTH_FILE: "/real/auth.json", GOOGLE_APPLICATION_CREDENTIALS: "/real/google.json", AWS_SHARED_CREDENTIALS_FILE: "/real/aws", AWS_PROFILE: "real-profile", CLOUDSDK_CONFIG: "/real/gcloud" }; const env = probeEnv(original, "/scratch");