Merge PR #8569: fix(browser): probe CDP endpoints over raw TCP to bypass HTTP_PROXY (@roboomp)
This commit is contained in:
@@ -78,6 +78,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed `lsp reload` crashing non-rust-analyzer language servers (e.g. Roslyn/`roslyn-language-server`) by sending the rust-analyzer-specific `rust-analyzer/reloadWorkspace` request to every server; the request is now gated on the server being rust-analyzer, and all other servers reload via `workspace/didChangeConfiguration` directly ([#8571](https://github.com/can1357/oh-my-pi/issues/8571)).
|
||||
### Fixed
|
||||
|
||||
- Fixed `browser open` failing with "Shared browser daemon unavailable" when `HTTP_PROXY`/`HTTPS_PROXY` is set (e.g. a local Clash proxy), because the shared-browser CDP liveness probes routed loopback `127.0.0.1` requests through the proxy, which 502'd them and killed the healthy daemon. The probes now talk to the endpoint over raw TCP and never touch a proxy ([#8567](https://github.com/can1357/oh-my-pi/issues/8567)).
|
||||
|
||||
## [17.3.4] - 2026-08-14
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import * as net from "node:net";
|
||||
import { Process, ProcessStatus } from "@oh-my-pi/pi-natives";
|
||||
import type { Socket } from "bun";
|
||||
import type { Browser, Page } from "puppeteer-core";
|
||||
import { ToolError, throwIfAborted } from "../tool-errors";
|
||||
|
||||
@@ -29,31 +30,92 @@ export async function findFreeCdpPort(): Promise<number> {
|
||||
return promise;
|
||||
}
|
||||
|
||||
/**
|
||||
* Loopback HTTP/1.1 GET that never routes through a proxy, resolving to the
|
||||
* response status code (or null when the endpoint is unreachable, aborted,
|
||||
* malformed, or slow past `timeoutMs`).
|
||||
*
|
||||
* Chrome's DevTools endpoint listens on loopback and speaks plain HTTP/1.1.
|
||||
* Both `fetch` and Bun's `node:http` honor `HTTP_PROXY`/`HTTPS_PROXY` and
|
||||
* forward even `127.0.0.1` requests to the proxy unless `NO_PROXY` covers them,
|
||||
* so a local proxy that 502s internal addresses makes a healthy daemon look
|
||||
* dead and the CDP readiness checks tear it down (issue #8567). Talking to the
|
||||
* socket over raw TCP sidesteps proxy env entirely.
|
||||
*/
|
||||
export async function probeCdpStatus(
|
||||
url: string,
|
||||
opts: { timeoutMs: number; signal?: AbortSignal },
|
||||
): Promise<number | null> {
|
||||
let target: URL;
|
||||
try {
|
||||
target = new URL(url);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
if (opts.signal?.aborted) return null;
|
||||
const port = target.port ? Number(target.port) : 80;
|
||||
const requestPath = `${target.pathname}${target.search}` || "/";
|
||||
const { promise, resolve } = Promise.withResolvers<number | null>();
|
||||
let socket: Socket<undefined> | undefined;
|
||||
let settled = false;
|
||||
const finish = (status: number | null) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearTimeout(timer);
|
||||
opts.signal?.removeEventListener("abort", onAbort);
|
||||
try {
|
||||
socket?.end();
|
||||
} catch {
|
||||
// socket already torn down
|
||||
}
|
||||
resolve(status);
|
||||
};
|
||||
const onAbort = () => finish(null);
|
||||
const timer = setTimeout(() => finish(null), opts.timeoutMs);
|
||||
opts.signal?.addEventListener("abort", onAbort, { once: true });
|
||||
let buffered = "";
|
||||
try {
|
||||
socket = await Bun.connect({
|
||||
hostname: target.hostname,
|
||||
port,
|
||||
socket: {
|
||||
open(s) {
|
||||
s.write(`GET ${requestPath} HTTP/1.1\r\nHost: ${target.hostname}:${port}\r\nConnection: close\r\n\r\n`);
|
||||
},
|
||||
data(_s, chunk) {
|
||||
buffered += chunk.toString("latin1");
|
||||
const match = /^HTTP\/\d(?:\.\d)? (\d{3})/.exec(buffered);
|
||||
if (match) finish(Number(match[1]));
|
||||
},
|
||||
error() {
|
||||
finish(null);
|
||||
},
|
||||
close() {
|
||||
finish(null);
|
||||
},
|
||||
},
|
||||
});
|
||||
} catch {
|
||||
finish(null);
|
||||
}
|
||||
return promise;
|
||||
}
|
||||
|
||||
/** Poll `${cdpUrl}/json/version` until it responds with 200, with abort + timeout support. */
|
||||
export async function waitForCdp(cdpUrl: string, timeoutMs: number, signal?: AbortSignal): Promise<void> {
|
||||
const deadline = Date.now() + timeoutMs;
|
||||
let lastErr: unknown;
|
||||
const probeUrl = `${cdpUrl.replace(/\/+$/, "")}/json/version`;
|
||||
let lastStatus: number | null = null;
|
||||
while (Date.now() < deadline) {
|
||||
throwIfAborted(signal);
|
||||
const probeTimeout = AbortSignal.timeout(2000);
|
||||
const probeSignal = signal ? AbortSignal.any([signal, probeTimeout]) : probeTimeout;
|
||||
try {
|
||||
const res = await fetch(probeUrl, { signal: probeSignal });
|
||||
if (res.ok) {
|
||||
await res.body?.cancel();
|
||||
return;
|
||||
}
|
||||
lastErr = new Error(`HTTP ${res.status}`);
|
||||
await res.body?.cancel();
|
||||
} catch (err) {
|
||||
if (signal?.aborted) throwIfAborted(signal);
|
||||
lastErr = err;
|
||||
}
|
||||
const status = await probeCdpStatus(probeUrl, { timeoutMs: 2000, signal });
|
||||
if (status !== null && status >= 200 && status < 300) return;
|
||||
lastStatus = status;
|
||||
await Bun.sleep(150);
|
||||
}
|
||||
throwIfAborted(signal);
|
||||
throw new ToolError(
|
||||
`Timed out waiting for CDP endpoint ${cdpUrl}${lastErr instanceof Error ? `: ${lastErr.message}` : ""}`,
|
||||
`Timed out waiting for CDP endpoint ${cdpUrl}${lastStatus !== null ? `: HTTP ${lastStatus}` : ""}`,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -81,15 +143,8 @@ function findCdpPortInArgs(args: string[]): number | null {
|
||||
|
||||
/** One-shot probe: returns true when `/json/version` answers 200 within the timeout. */
|
||||
async function probeCdpAt(port: number, signal?: AbortSignal): Promise<boolean> {
|
||||
const probeTimeout = AbortSignal.timeout(1500);
|
||||
const probeSignal = signal ? AbortSignal.any([signal, probeTimeout]) : probeTimeout;
|
||||
try {
|
||||
const res = await fetch(`http://127.0.0.1:${port}/json/version`, { signal: probeSignal });
|
||||
await res.body?.cancel();
|
||||
return res.ok;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
const status = await probeCdpStatus(`http://127.0.0.1:${port}/json/version`, { timeoutMs: 1500, signal });
|
||||
return status !== null && status >= 200 && status < 300;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -16,6 +16,7 @@ import { describeQuietly, stopQuietly, waitReady } from "../../launch/ensure";
|
||||
import { daemonRuntimeDir } from "../../launch/paths";
|
||||
import type { DaemonSnapshot } from "../../launch/protocol";
|
||||
import { throwIfAborted } from "../tool-errors";
|
||||
import { probeCdpStatus } from "./attach";
|
||||
import { resolveSharedBrowserLaunchSpec } from "./launch";
|
||||
|
||||
/** Chrome prints this on stderr once the CDP listener is up; the broker's ready probe captures the line. */
|
||||
@@ -50,13 +51,8 @@ async function probeEndpoint(wsEndpoint: string): Promise<boolean> {
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
const res = await fetch(`http://${host}/json/version`, { signal: AbortSignal.timeout(PROBE_TIMEOUT_MS) });
|
||||
await res.body?.cancel();
|
||||
return res.ok;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
const status = await probeCdpStatus(`http://${host}/json/version`, { timeoutMs: PROBE_TIMEOUT_MS });
|
||||
return status !== null && status >= 200 && status < 300;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -3,7 +3,9 @@ import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import type { ToolSession } from "@oh-my-pi/pi-coding-agent/sdk";
|
||||
import { BrowserTool } from "@oh-my-pi/pi-coding-agent/tools/browser";
|
||||
import {
|
||||
findFreeCdpPort,
|
||||
pickElectronTarget,
|
||||
probeCdpStatus,
|
||||
shouldPreserveConnectedBrowserFocus,
|
||||
} from "@oh-my-pi/pi-coding-agent/tools/browser/attach";
|
||||
import {
|
||||
@@ -206,3 +208,56 @@ describe("pickElectronTarget", () => {
|
||||
30_000,
|
||||
);
|
||||
});
|
||||
|
||||
describe("probeCdpStatus", () => {
|
||||
// Regression for #8567: a local proxy (Clash, corporate) 502s internal
|
||||
// loopback addresses, so a bare fetch()/node:http probe misreports a healthy
|
||||
// CDP daemon as dead. The raw-TCP probe must ignore HTTP_PROXY entirely.
|
||||
test("returns the loopback status even when HTTP_PROXY 502s the request", async () => {
|
||||
const cdp = Bun.serve({ port: 0, fetch: () => new Response("{}", { status: 200 }) });
|
||||
const proxy = Bun.serve({ port: 0, fetch: () => new Response("Bad Gateway", { status: 502 }) });
|
||||
const saved = { HTTP_PROXY: process.env.HTTP_PROXY, http_proxy: process.env.http_proxy };
|
||||
process.env.HTTP_PROXY = `http://127.0.0.1:${proxy.port}`;
|
||||
process.env.http_proxy = `http://127.0.0.1:${proxy.port}`;
|
||||
try {
|
||||
const status = await probeCdpStatus(`http://127.0.0.1:${cdp.port}/json/version`, { timeoutMs: 1500 });
|
||||
expect(status).toBe(200);
|
||||
} finally {
|
||||
process.env.HTTP_PROXY = saved.HTTP_PROXY;
|
||||
process.env.http_proxy = saved.http_proxy;
|
||||
if (saved.HTTP_PROXY === undefined) delete process.env.HTTP_PROXY;
|
||||
if (saved.http_proxy === undefined) delete process.env.http_proxy;
|
||||
await cdp.stop(true);
|
||||
await proxy.stop(true);
|
||||
}
|
||||
});
|
||||
|
||||
test("surfaces a non-2xx status from a live endpoint", async () => {
|
||||
const server = Bun.serve({ port: 0, fetch: () => new Response("nope", { status: 503 }) });
|
||||
try {
|
||||
const status = await probeCdpStatus(`http://127.0.0.1:${server.port}/json/version`, { timeoutMs: 1500 });
|
||||
expect(status).toBe(503);
|
||||
} finally {
|
||||
await server.stop(true);
|
||||
}
|
||||
});
|
||||
|
||||
test("returns null when the endpoint is unreachable", async () => {
|
||||
const port = await findFreeCdpPort();
|
||||
const status = await probeCdpStatus(`http://127.0.0.1:${port}/json/version`, { timeoutMs: 500 });
|
||||
expect(status).toBeNull();
|
||||
});
|
||||
|
||||
test("returns null when the request is already aborted", async () => {
|
||||
const server = Bun.serve({ port: 0, fetch: () => new Response("{}", { status: 200 }) });
|
||||
try {
|
||||
const status = await probeCdpStatus(`http://127.0.0.1:${server.port}/json/version`, {
|
||||
timeoutMs: 1500,
|
||||
signal: AbortSignal.abort(),
|
||||
});
|
||||
expect(status).toBeNull();
|
||||
} finally {
|
||||
await server.stop(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user