Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/bin/chrome-devtools-mcp-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import '../polyfill.js';

import process from 'node:process';

import {closeBrowser} from '../browser.js';
import {createMcpServer, logDisclaimers} from '../index.js';
import {logger, saveLogsToFile} from '../logger.js';
import {ClearcutLogger} from '../telemetry/ClearcutLogger.js';
Expand All @@ -32,6 +33,44 @@ if (process.env['CHROME_DEVTOOLS_MCP_CRASH_ON_UNCAUGHT'] !== 'true') {
});
}

// Shutdown on stdin EOF (stdio MCP convention — the client closes the
// transport to signal exit) and on standard termination signals. Without
// this, an active Chrome subprocess keeps the Node event loop ref'd after
// stdin closes and the server hangs until something else kills it.
let shuttingDown = false;
async function shutdown(reason: string): Promise<void> {
if (shuttingDown) {
return;
}
shuttingDown = true;
logger(`Shutting down (${reason})`);
// Backstop in case browser teardown hangs (e.g. unresponsive Chrome,
// slow beforeunload handlers, many tabs). Exits 0 because we still
// honored the shutdown request; the log line preserves observability.
// Unref'd so it doesn't keep the loop alive on the clean path.
setTimeout(() => {
logger('Shutdown timeout exceeded, forcing exit');
process.exit(0);
}, 10000).unref();
await closeBrowser();
process.exit(0);
}
process.stdin.on('end', () => {
void shutdown('stdin end');
});
process.stdin.on('close', () => {
void shutdown('stdin close');
});
process.on('SIGTERM', () => {
Comment thread
OrKoN marked this conversation as resolved.
void shutdown('SIGTERM');
});
process.on('SIGINT', () => {
void shutdown('SIGINT');
});
process.on('SIGHUP', () => {
void shutdown('SIGHUP');
});

logger(`Starting Chrome DevTools MCP Server v${VERSION}`);
const {server} = await createMcpServer(args, {
logFile,
Expand Down
39 changes: 37 additions & 2 deletions src/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type {
import {puppeteer} from './third_party/index.js';

let browser: Browser | undefined;
let browserMode: 'launched' | 'connected' | undefined;

function makeTargetFilter(enableExtensions = false) {
const ignoredPrefixes = new Set(['chrome://', 'chrome-untrusted://']);
Expand Down Expand Up @@ -120,7 +121,12 @@ export async function ensureBrowserConnected(options: {

logger('Connecting Puppeteer to ', JSON.stringify(connectOptions));
try {
browser = await puppeteer.connect(connectOptions);
// Assign mode before browser so a concurrent closeBrowser() never sees
// `browser` set with `browserMode` still undefined (would fall through
// to the disconnect() path and orphan a launched Chrome).
const connected = await puppeteer.connect(connectOptions);
browserMode = 'connected';
browser = connected;
} catch (err) {
throw new Error(
`Could not connect to Chrome. ${autoConnect ? `Check if Chrome is running and remote debugging is enabled by going to chrome://inspect/#remote-debugging.` : `Check if Chrome is running.`}`,
Expand Down Expand Up @@ -266,8 +272,37 @@ export async function ensureBrowserLaunched(
if (browser?.connected) {
return browser;
}
browser = await launch(options);
// Assign mode before browser; see the connect path above for rationale.
const launched = await launch(options);
browserMode = 'launched';
browser = launched;
return browser;
}

/**
* Shutdown hook for the active browser. Closes a launched browser (so the
* Chrome subprocess is reaped) or disconnects from an attached browser (so
* the user's Chrome instance stays alive). No-op if no browser is active or
* the connection has already been dropped. Called from the server entrypoint
* on stdin EOF / SIGTERM / SIGINT.
*/
export async function closeBrowser(): Promise<void> {
const b = browser;
const mode = browserMode;
browser = undefined;
browserMode = undefined;
if (!b || !b.connected) {
return;
}
if (mode === 'launched') {
await b.close().catch(err => {
logger('Failed to close browser', err);
});
return;
}
await b.disconnect().catch(err => {
logger('Failed to disconnect from browser', err);
});
}

export type Channel = 'stable' | 'canary' | 'beta' | 'dev';
180 changes: 180 additions & 0 deletions tests/shutdown.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import assert from 'node:assert';
import type {ChildProcessByStdio} from 'node:child_process';
import {spawn} from 'node:child_process';
import type {Readable, Writable} from 'node:stream';
import {describe, it} from 'node:test';

import {executablePath} from 'puppeteer';

type Server = ChildProcessByStdio<Writable, Readable, Readable>;

// Once shutdown is signalled, the server should be fully gone within this
// budget. The actual fast path is well under 500ms; the budget is set to be
// generous against CI noise without being so loose that it would hide a hang.
const SHUTDOWN_BUDGET_MS = 3000;
// Outer test timeout. If exit doesn't happen within this, treat as a hang
// (the bug we're guarding against) and SIGKILL the subprocess.
const EXIT_TIMEOUT_MS = 15000;

async function spawnServer(): Promise<Server> {
const child = spawn(
'node',
[
'build/src/bin/chrome-devtools-mcp.js',
'--headless',
'--isolated',
'--executable-path',
await executablePath(),
],
{
env: {
...process.env,
CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true',
},
stdio: ['pipe', 'pipe', 'pipe'],
},
) as Server;
// Drain stderr to avoid pipe-buffer backpressure stalling the server.
child.stderr.on('data', () => {
// discard
});
return child;
}

async function waitForExit(
child: Server,
timeoutMs: number,
): Promise<{
code: number | null;
signal: NodeJS.Signals | null;
elapsedMs: number;
}> {
const start = Date.now();
return await new Promise((resolve, reject) => {
const timer = setTimeout(() => {
child.kill('SIGKILL');
reject(new Error(`server did not exit within ${timeoutMs}ms`));
}, timeoutMs);
child.once('exit', (code, signal) => {
clearTimeout(timer);
resolve({code, signal, elapsedMs: Date.now() - start});
});
});
}

async function rpc(
child: Server,
msg: {method: string; params?: unknown},
): Promise<unknown> {
const id = Math.floor(Math.random() * 1e9);
const payload = JSON.stringify({jsonrpc: '2.0', id, ...msg}) + '\n';
return await new Promise((resolve, reject) => {
let buf = '';
const onData = (chunk: Buffer) => {
buf += chunk.toString();
const lines = buf.split('\n');
buf = lines.pop() ?? '';
for (const line of lines) {
if (!line.trim()) {
continue;
}
try {
const parsed = JSON.parse(line) as {id?: number};
if (parsed.id === id) {
child.stdout.off('data', onData);
resolve(parsed);
return;
}
} catch {
// Not a JSON message; ignore.
}
}
};
child.stdout.on('data', onData);
const onExit = () => {
child.stdout.off('data', onData);
reject(new Error('server exited before RPC response'));
};
child.once('exit', onExit);
child.stdin.write(payload);
});
}

function notify(child: Server, msg: {method: string; params?: unknown}): void {
child.stdin.write(JSON.stringify({jsonrpc: '2.0', ...msg}) + '\n');
}

async function initializeAndLaunchBrowser(child: Server): Promise<void> {
await rpc(child, {
method: 'initialize',
params: {
protocolVersion: '2024-11-05',
capabilities: {},
clientInfo: {name: 'shutdown-test', version: '0.0.1'},
},
});
notify(child, {method: 'notifications/initialized'});
// navigate_page forces a real Chrome launch — this is what reproduces
// the hang in #2116. Without an active Chrome subprocess, stdin EOF
// would close the event loop on its own and shutdown would look fine
// even with broken handlers.
await rpc(child, {
method: 'tools/call',
params: {
name: 'navigate_page',
arguments: {url: 'about:blank'},
},
});
}

describe('shutdown', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it('exits within budget on stdin EOF after Chrome launch', async () => {
const child = await spawnServer();
await initializeAndLaunchBrowser(child);
child.stdin.end();
const {elapsedMs} = await waitForExit(child, EXIT_TIMEOUT_MS);
assert.ok(
elapsedMs < SHUTDOWN_BUDGET_MS,
`stdin-EOF shutdown took ${elapsedMs}ms (budget ${SHUTDOWN_BUDGET_MS}ms)`,
);
});

it('exits within budget on SIGTERM after Chrome launch', async () => {
const child = await spawnServer();
await initializeAndLaunchBrowser(child);
child.kill('SIGTERM');
const {elapsedMs} = await waitForExit(child, EXIT_TIMEOUT_MS);
assert.ok(
elapsedMs < SHUTDOWN_BUDGET_MS,
`SIGTERM shutdown took ${elapsedMs}ms (budget ${SHUTDOWN_BUDGET_MS}ms)`,
);
});

it('exits within budget on SIGINT after Chrome launch', async () => {
const child = await spawnServer();
await initializeAndLaunchBrowser(child);
child.kill('SIGINT');
const {elapsedMs} = await waitForExit(child, EXIT_TIMEOUT_MS);
assert.ok(
elapsedMs < SHUTDOWN_BUDGET_MS,
`SIGINT shutdown took ${elapsedMs}ms (budget ${SHUTDOWN_BUDGET_MS}ms)`,
);
});

it('exits within budget on SIGHUP after Chrome launch', async () => {
const child = await spawnServer();
await initializeAndLaunchBrowser(child);
child.kill('SIGHUP');
const {elapsedMs} = await waitForExit(child, EXIT_TIMEOUT_MS);
assert.ok(
elapsedMs < SHUTDOWN_BUDGET_MS,
`SIGHUP shutdown took ${elapsedMs}ms (budget ${SHUTDOWN_BUDGET_MS}ms)`,
);
});
});
Loading