Skip to content

Commit 0bf8d55

Browse files
triuzziclaude
andauthored
fix: protocolTimeout for heavy pages + attach-mode arg cleanup
Two reliability fixes for symptoms hit on real workloads: 1. Heavy pages (e.g. Studio module dev bundles >100MB) cannot ack `Network.enable` and other auto-attached domain calls within puppeteer's default 180s timeout. Once it fires, the CDP connection is marked dead and every subsequent call throws — only daemon restart recovers. Bump `protocolTimeout` to 10min for both `puppeteer.connect` and `puppeteer.launch`, env-overridable via `BRAVE_DEVTOOLS_PROTOCOL_TIMEOUT_MS`. 2. `start --browserUrl X` was emitting `--browser-url X --headless --isolated` in the daemon args because the spawn-mode defaults (`headless: true`, `isolated: true`) were applied unconditionally after yargs parsing. The runtime resolver ignored them under attach mode but `status` showed misleading args, and the silent fallthrough would become a real bug if the resolver order ever changed. Detect attach mode (`browserUrl` / `wsEndpoint` / `autoConnect`) and strip spawn-only flags before serializing. Smoke-tested both modes: attach now reports `args=[--browser-url X, --category-..., ...]` and spawn still gets `[--headless, --isolated, ...]`. Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1cbc585 commit 0bf8d55

2 files changed

Lines changed: 36 additions & 6 deletions

File tree

src/bin/brave-devtools.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,29 @@ y.command(
9797
if (isDaemonRunning(argv.sessionId)) {
9898
await stopDaemon(argv.sessionId);
9999
}
100-
// Defaults but we do not want to affect the yargs conflict resolution.
101-
if (argv.isolated === undefined && argv.userDataDir === undefined) {
102-
argv.isolated = true;
103-
}
104-
if (argv.headless === undefined) {
105-
argv.headless = true;
100+
// Defaults only apply to *spawn* mode. Skip / clear them when
101+
// attaching to an existing browser, otherwise `status` shows
102+
// misleading args like `--browserUrl X --headless --isolated`
103+
// together (the runtime ignores headless/isolated under attach,
104+
// but storing them anyway is a footgun if the resolver order ever
105+
// changes — and confuses anyone reading `status` output).
106+
const isAttachMode =
107+
argv.browserUrl !== undefined ||
108+
argv.wsEndpoint !== undefined ||
109+
argv.autoConnect === true;
110+
if (isAttachMode) {
111+
// Strip spawn-mode flags so serializeArgs doesn't emit them.
112+
// Headless has `default: true` in the CLI override, so this is
113+
// necessary even when the user didn't pass `--headless`.
114+
delete argv.headless;
115+
delete argv.isolated;
116+
} else {
117+
if (argv.isolated === undefined && argv.userDataDir === undefined) {
118+
argv.isolated = true;
119+
}
120+
if (argv.headless === undefined) {
121+
argv.headless = true;
122+
}
106123
}
107124
const args = serializeArgs(cliOptions, argv);
108125
await start(args, argv.sessionId);

src/browser.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,17 @@ let browser: Browser | undefined;
2121

2222
export type Channel = 'release' | 'beta' | 'nightly' | 'dev';
2323

24+
// Heavy pages (e.g. Studio module dev bundles >100MB) cannot ack
25+
// `Network.enable` and other auto-attached domain calls within
26+
// puppeteer's default 180s. Once that fires, the CDP connection is
27+
// marked dead and every subsequent call throws — only daemon restart
28+
// recovers. Bumping the ceiling to 10min covers realistic loads;
29+
// override via env for power users.
30+
const PROTOCOL_TIMEOUT_MS = parseInt(
31+
process.env.BRAVE_DEVTOOLS_PROTOCOL_TIMEOUT_MS ?? '600000',
32+
10,
33+
);
34+
2435
function makeTargetFilter(enableExtensions = false) {
2536
const ignoredPrefixes = new Set([
2637
'chrome://',
@@ -189,6 +200,7 @@ export async function ensureBrowserConnected(options: {
189200
targetFilter: makeTargetFilter(enableExtensions),
190201
defaultViewport: null,
191202
handleDevToolsAsPage: true,
203+
protocolTimeout: PROTOCOL_TIMEOUT_MS,
192204
};
193205

194206
let autoConnect = false;
@@ -340,6 +352,7 @@ export async function launch(options: McpLaunchOptions): Promise<Browser> {
340352
acceptInsecureCerts: options.acceptInsecureCerts,
341353
handleDevToolsAsPage: true,
342354
enableExtensions: options.enableExtensions,
355+
protocolTimeout: PROTOCOL_TIMEOUT_MS,
343356
});
344357
if (options.logFile) {
345358
browser.process()?.stderr?.pipe(options.logFile);

0 commit comments

Comments
 (0)