From 3779d898ac539dffd3ef2c4b7864d0f43acb8373 Mon Sep 17 00:00:00 2001 From: elijahr Date: Tue, 18 Nov 2025 20:27:22 -0600 Subject: [PATCH 1/2] feat: add switch_browser tool with noLaunch CLI option Adds the ability to dynamically switch between browser instances at runtime, with support for both HTTP and WebSocket connection URLs and configurable connection timeouts. New Features: - switch_browser tool for connecting to different browser instances - --noLaunch CLI flag to start without auto-launching a browser - Timeout parameter with 10-second default for connection attempts - Automatic HTTP to WebSocket URL conversion - Support for ws://, wss://, http://, and https:// protocols Implementation: - Created src/context.ts to manage browser context lifecycle - Created src/config.ts to break dependency cycles - Added getBrowser/setBrowser helpers for better testability - Added disconnectBrowser() to safely close browser connections - Proper timeout cleanup to prevent resource leaks - Comprehensive error handling with proper Error type checks Testing: - 6 new integration tests with actual browser instances - Tests verify browser disconnection, connection switching, and error handling - All tests use headless mode for CI compatibility - Proper test isolation with independent browser instances per test Documentation: - Updated README.md with new tool and CLI option - Auto-generated tool reference documentation - Updated release-please configuration --- README.md | 8 +- docs/tool-reference.md | 13 +- release-please-config.json | 2 +- src/McpContext.ts | 4 +- src/browser.ts | 29 ++- src/cli.ts | 7 + src/config.ts | 15 ++ src/context.ts | 98 ++++++++ src/main.ts | 85 ++++--- src/tools/switch_browser.ts | 97 ++++++++ tests/browser.test.ts | 41 +++- tests/cli.test.ts | 46 ++++ tests/tools/switch_browser.test.ts | 372 +++++++++++++++++++++++++++++ 13 files changed, 775 insertions(+), 42 deletions(-) create mode 100644 src/config.ts create mode 100644 src/context.ts create mode 100644 src/tools/switch_browser.ts create mode 100644 tests/tools/switch_browser.test.ts diff --git a/README.md b/README.md index 3de17ff7..d18f1aa2 100644 --- a/README.md +++ b/README.md @@ -263,12 +263,13 @@ If you run into any issues, checkout our [troubleshooting guide](./docs/troubles - [`hover`](docs/tool-reference.md#hover) - [`press_key`](docs/tool-reference.md#press_key) - [`upload_file`](docs/tool-reference.md#upload_file) -- **Navigation automation** (6 tools) +- **Navigation automation** (7 tools) - [`close_page`](docs/tool-reference.md#close_page) - [`list_pages`](docs/tool-reference.md#list_pages) - [`navigate_page`](docs/tool-reference.md#navigate_page) - [`new_page`](docs/tool-reference.md#new_page) - [`select_page`](docs/tool-reference.md#select_page) + - [`switch_browser`](docs/tool-reference.md#switch_browser) - [`wait_for`](docs/tool-reference.md#wait_for) - **Emulation** (2 tools) - [`emulate`](docs/tool-reference.md#emulate) @@ -361,6 +362,11 @@ The Chrome DevTools MCP server supports the following configuration option: - **Type:** boolean - **Default:** `true` +- **`--noLaunch`** + Do not launch or connect to a browser automatically. Use switch_browser tool to connect manually. + - **Type:** boolean + - **Default:** `false` + Pass them via the `args` property in the JSON configuration. For example: diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 0e126b8c..91b280cd 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -11,12 +11,13 @@ - [`hover`](#hover) - [`press_key`](#press_key) - [`upload_file`](#upload_file) -- **[Navigation automation](#navigation-automation)** (6 tools) +- **[Navigation automation](#navigation-automation)** (7 tools) - [`close_page`](#close_page) - [`list_pages`](#list_pages) - [`navigate_page`](#navigate_page) - [`new_page`](#new_page) - [`select_page`](#select_page) + - [`switch_browser`](#switch_browser) - [`wait_for`](#wait_for) - **[Emulation](#emulation)** (2 tools) - [`emulate`](#emulate) @@ -176,6 +177,16 @@ --- +### `switch_browser` + +**Description:** Connect to a different browser instance. Disconnects from the current browser (if any) and establishes a new connection. Accepts either HTTP URLs (e.g., http://127.0.0.1:9222) or WebSocket endpoints (e.g., ws://127.0.0.1:9222/devtools/browser/<id>). + +**Parameters:** + +- **url** (string) **(required)**: Browser connection URL. Can be an HTTP URL (e.g., http://127.0.0.1:9222) which will be auto-converted to WebSocket, or a direct WebSocket endpoint (e.g., ws://127.0.0.1:52862/devtools/browser/<id>). + +--- + ### `wait_for` **Description:** Wait for the specified text to appear on the selected page. diff --git a/release-please-config.json b/release-please-config.json index 160a9bbc..314cb682 100644 --- a/release-please-config.json +++ b/release-please-config.json @@ -18,7 +18,7 @@ "extra-files": [ { "type": "generic", - "path": "src/main.ts" + "path": "src/config.ts" }, { "type": "json", diff --git a/src/McpContext.ts b/src/McpContext.ts index 1c3c988d..d7d5eb4c 100644 --- a/src/McpContext.ts +++ b/src/McpContext.ts @@ -399,7 +399,9 @@ export class McpContext implements Context { }); if (!this.#selectedPage || this.#pages.indexOf(this.#selectedPage) === -1) { - this.selectPage(this.#pages[0]); + if (this.#pages.length > 0) { + this.selectPage(this.#pages[0]); + } } await this.detectOpenDevToolsWindows(); diff --git a/src/browser.ts b/src/browser.ts index 3abcbe00..778e2ce5 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -19,6 +19,21 @@ import {puppeteer} from './third_party/index.js'; let browser: Browser | undefined; +export async function disconnectBrowser(): Promise { + if (browser?.connected) { + await browser.close(); + browser = undefined; + } +} + +export function getBrowser(): Browser | undefined { + return browser; +} + +export function setBrowser(newBrowser: Browser | undefined): void { + browser = newBrowser; +} + function makeTargetFilter() { const ignoredPrefixes = new Set([ 'chrome://', @@ -67,9 +82,17 @@ export async function ensureBrowserConnected(options: { } logger('Connecting Puppeteer to ', JSON.stringify(connectOptions)); - browser = await puppeteer.connect(connectOptions); - logger('Connected Puppeteer'); - return browser; + try { + browser = await puppeteer.connect(connectOptions); + logger('Connected Puppeteer successfully'); + logger('Browser object type:', typeof browser); + logger('Browser.connected:', browser?.connected); + return browser; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger('Failed to connect to Puppeteer:', message); + throw new Error(`Puppeteer connection failed: ${message}`); + } } interface McpLaunchOptions { diff --git a/src/cli.ts b/src/cli.ts index 5ce8673e..96821c0f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -160,6 +160,12 @@ export const cliOptions = { default: true, describe: 'Set to false to exclude tools related to network.', }, + noLaunch: { + type: 'boolean', + default: false, + describe: + 'Do not launch or connect to a browser automatically. Use switch_browser tool to connect manually.', + }, } satisfies Record; export function parseArguments(version: string, argv = process.argv) { @@ -170,6 +176,7 @@ export function parseArguments(version: string, argv = process.argv) { // We can't set default in the options else // Yargs will complain if ( + !args.noLaunch && !args.channel && !args.browserUrl && !args.wsEndpoint && diff --git a/src/config.ts b/src/config.ts new file mode 100644 index 00000000..96568234 --- /dev/null +++ b/src/config.ts @@ -0,0 +1,15 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {parseArguments} from './cli.js'; + +// If moved update release-please config +// x-release-please-start-version +const VERSION = '0.10.1'; +// x-release-please-end + +export const args = parseArguments(VERSION); +export {VERSION}; diff --git a/src/context.ts b/src/context.ts new file mode 100644 index 00000000..cf9bb8f4 --- /dev/null +++ b/src/context.ts @@ -0,0 +1,98 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {ensureBrowserConnected} from './browser.js'; +import {logger} from './logger.js'; +import {McpContext} from './McpContext.js'; + +let context: McpContext | undefined; + +export interface SetContextOptions { + browserURL?: string; + wsEndpoint?: string; + devtools?: boolean; + experimentalIncludeAllPages?: boolean; + timeout?: number; +} + +export async function setContext( + options: SetContextOptions, +): Promise { + const { + browserURL, + wsEndpoint, + devtools = false, + experimentalIncludeAllPages = false, + timeout, + } = options; + + logger('setContext called with:', { + browserURL, + wsEndpoint, + devtools, + timeout, + }); + + const connectPromise = ensureBrowserConnected({ + browserURL, + wsEndpoint, + devtools, + }); + + let browser; + logger('Starting browser connection, timeout:', timeout); + if (timeout) { + let timeoutId: NodeJS.Timeout | undefined; + const timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout( + () => + reject( + new Error( + `Failed to connect to browser within ${timeout}ms. Please check that the browser is running and accessible at the provided URL.`, + ), + ), + timeout, + ); + }); + + try { + browser = await Promise.race([connectPromise, timeoutPromise]); + } finally { + // Clear timeout to prevent it from firing after connection succeeds + if (timeoutId) { + clearTimeout(timeoutId); + } + } + } else { + browser = await connectPromise; + } + + logger('Browser connection completed, browser type:', typeof browser); + logger('Browser connected status:', browser?.connected); + + if (!browser) { + throw new Error( + 'Failed to connect to browser: browser object is undefined', + ); + } + + logger('Creating McpContext from browser...'); + context = await McpContext.from(browser, logger, { + experimentalDevToolsDebugging: devtools, + experimentalIncludeAllPages, + }); + + logger('McpContext created successfully'); + return context; +} + +export function getContext(): McpContext | undefined { + return context; +} + +export function setContextInstance(newContext: McpContext | undefined): void { + context = newContext; +} diff --git a/src/main.ts b/src/main.ts index 77956876..8a6ef83e 100644 --- a/src/main.ts +++ b/src/main.ts @@ -8,7 +8,11 @@ import './polyfill.js'; import type {Channel} from './browser.js'; import {ensureBrowserConnected, ensureBrowserLaunched} from './browser.js'; -import {parseArguments} from './cli.js'; +import {args, VERSION} from './config.js'; +import { + getContext as getContextInstance, + setContextInstance, +} from './context.js'; import {loadIssueDescriptions} from './issue-descriptions.js'; import {logger, saveLogsToFile} from './logger.js'; import {McpContext} from './McpContext.js'; @@ -30,15 +34,9 @@ import * as performanceTools from './tools/performance.js'; import * as screenshotTools from './tools/screenshot.js'; import * as scriptTools from './tools/script.js'; import * as snapshotTools from './tools/snapshot.js'; +import * as switchBrowserTool from './tools/switch_browser.js'; import type {ToolDefinition} from './tools/ToolDefinition.js'; -// If moved update release-please config -// x-release-please-start-version -const VERSION = '0.10.1'; -// x-release-please-end - -export const args = parseArguments(VERSION); - const logFile = args.logFile ? saveLogsToFile(args.logFile) : undefined; logger(`Starting Chrome DevTools MCP Server v${VERSION}`); @@ -54,39 +52,57 @@ server.server.setRequestHandler(SetLevelRequestSchema, () => { return {}; }); -let context: McpContext; async function getContext(): Promise { + let context = getContextInstance(); + + if (args.noLaunch && !context) { + throw new Error( + 'No browser connected. Use the switch_browser tool to connect to a browser instance.', + ); + } + const extraArgs: string[] = (args.chromeArg ?? []).map(String); if (args.proxyServer) { extraArgs.push(`--proxy-server=${args.proxyServer}`); } const devtools = args.experimentalDevtools ?? false; - const browser = - args.browserUrl || args.wsEndpoint - ? await ensureBrowserConnected({ - browserURL: args.browserUrl, - wsEndpoint: args.wsEndpoint, - wsHeaders: args.wsHeaders, - devtools, - }) - : await ensureBrowserLaunched({ - headless: args.headless, - executablePath: args.executablePath, - channel: args.channel as Channel, - isolated: args.isolated, - logFile, - viewport: args.viewport, - args: extraArgs, - acceptInsecureCerts: args.acceptInsecureCerts, - devtools, - }); - - if (context?.browser !== browser) { - context = await McpContext.from(browser, logger, { - experimentalDevToolsDebugging: devtools, - experimentalIncludeAllPages: args.experimentalIncludeAllPages, - }); + + if (!args.noLaunch) { + const browser = + args.browserUrl || args.wsEndpoint + ? await ensureBrowserConnected({ + browserURL: args.browserUrl, + wsEndpoint: args.wsEndpoint, + wsHeaders: args.wsHeaders, + devtools, + }) + : await ensureBrowserLaunched({ + headless: args.headless, + executablePath: args.executablePath, + channel: args.channel as Channel, + isolated: args.isolated, + logFile, + viewport: args.viewport, + args: extraArgs, + acceptInsecureCerts: args.acceptInsecureCerts, + devtools, + }); + + if (context?.browser !== browser) { + context = await McpContext.from(browser, logger, { + experimentalDevToolsDebugging: devtools, + experimentalIncludeAllPages: args.experimentalIncludeAllPages, + }); + setContextInstance(context); + } } + + if (!context) { + throw new Error( + 'Failed to initialize browser context. This should not happen.', + ); + } + return context; } @@ -180,6 +196,7 @@ const tools = [ ...Object.values(screenshotTools), ...Object.values(scriptTools), ...Object.values(snapshotTools), + ...Object.values(switchBrowserTool), ] as ToolDefinition[]; tools.sort((a, b) => { diff --git a/src/tools/switch_browser.ts b/src/tools/switch_browser.ts new file mode 100644 index 00000000..7650e37d --- /dev/null +++ b/src/tools/switch_browser.ts @@ -0,0 +1,97 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {disconnectBrowser} from '../browser.js'; +import {args} from '../config.js'; +import {setContext} from '../context.js'; +import {zod} from '../third_party/index.js'; + +import {ToolCategory} from './categories.js'; +import {defineTool} from './ToolDefinition.js'; + +async function convertHttpToBrowserUrl( + url: string, +): Promise { + try { + const response = await fetch(`${url}/json/version`); + if (!response.ok) { + throw new Error(`Failed to fetch browser info: ${response.statusText}`); + } + const data = await response.json(); + return data.webSocketDebuggerUrl; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error( + `Could not connect to browser at ${url}: ${message}. Make sure the browser is running and the URL is correct.`, + ); + } +} + +export const switchBrowser = defineTool({ + name: 'switch_browser', + description: `Connect to a different browser instance. Disconnects from the current browser (if any) and establishes a new connection. Accepts either HTTP URLs (e.g., http://127.0.0.1:9222) or WebSocket endpoints (e.g., ws://127.0.0.1:9222/devtools/browser/).`, + annotations: { + category: ToolCategory.NAVIGATION, + readOnlyHint: false, + }, + schema: { + url: zod + .string() + .describe( + 'Browser connection URL. Can be an HTTP URL (e.g., http://127.0.0.1:9222) which will be auto-converted to WebSocket, or a direct WebSocket endpoint (e.g., ws://127.0.0.1:52862/devtools/browser/).', + ), + timeout: zod + .number() + .optional() + .describe( + 'Connection timeout in milliseconds. Defaults to 10000 (10 seconds). If the connection cannot be established within this time, an error will be thrown.', + ), + }, + handler: async (request, response, _context) => { + const {url, timeout = 10000} = request.params; + + // Disconnect from current browser + await disconnectBrowser(); + + // Determine if it's HTTP or WebSocket URL + let wsEndpoint: string | undefined; + let browserURL: string | undefined; + + const urlObj = new URL(url); + + if (urlObj.protocol === 'ws:' || urlObj.protocol === 'wss:') { + // Direct WebSocket endpoint + wsEndpoint = url; + response.appendResponseLine( + `Connecting to browser via WebSocket: ${url}`, + ); + } else if (urlObj.protocol === 'http:' || urlObj.protocol === 'https:') { + // HTTP URL - need to convert to WebSocket + response.appendResponseLine( + `Fetching WebSocket endpoint from browser at ${url}...`, + ); + wsEndpoint = await convertHttpToBrowserUrl(url); + response.appendResponseLine(`Resolved WebSocket endpoint: ${wsEndpoint}`); + browserURL = url; + } else { + throw new Error( + `Unsupported protocol: ${urlObj.protocol}. Expected http://, https://, ws://, or wss://`, + ); + } + + // Connect to the new browser + await setContext({ + browserURL, + wsEndpoint, + devtools: args.experimentalDevtools ?? false, + experimentalIncludeAllPages: args.experimentalIncludeAllPages, + timeout, + }); + + response.appendResponseLine(`✓ Successfully connected to browser`); + response.setIncludePages(true); + }, +}); diff --git a/tests/browser.test.ts b/tests/browser.test.ts index 8066da5f..90bd7886 100644 --- a/tests/browser.test.ts +++ b/tests/browser.test.ts @@ -10,7 +10,12 @@ import {describe, it} from 'node:test'; import {executablePath} from 'puppeteer'; -import {launch} from '../src/browser.js'; +import { + launch, + disconnectBrowser, + getBrowser, + setBrowser, +} from '../src/browser.js'; describe('browser', () => { it('cannot launch multiple times with the same profile', async () => { @@ -72,4 +77,38 @@ describe('browser', () => { await browser.close(); } }); + + it('disconnectBrowser closes connected browser', async () => { + const tmpDir = os.tmpdir(); + const folderPath = path.join(tmpDir, `temp-folder-${crypto.randomUUID()}`); + const browser = await launch({ + headless: true, + isolated: false, + userDataDir: folderPath, + executablePath: executablePath(), + devtools: false, + }); + + // Set the browser as the module-level browser + setBrowser(browser); + + assert.ok(browser.connected); + assert.strictEqual(getBrowser(), browser); + + await disconnectBrowser(); + + assert.ok(!browser.connected); + assert.strictEqual(getBrowser(), undefined); + }); + + it('disconnectBrowser handles no browser gracefully', async () => { + // Ensure no browser is set + setBrowser(undefined); + assert.strictEqual(getBrowser(), undefined); + + // Should not throw + await disconnectBrowser(); + + assert.strictEqual(getBrowser(), undefined); + }); }); diff --git a/tests/cli.test.ts b/tests/cli.test.ts index 19502e28..17ad327a 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -16,6 +16,8 @@ describe('cli args parsing', () => { categoryPerformance: true, 'category-network': true, categoryNetwork: true, + 'no-launch': false, + noLaunch: false, }; it('parses with default args', async () => { @@ -198,4 +200,48 @@ describe('cli args parsing', () => { categoryEmulation: false, }); }); + + it('parses noLaunch flag', async () => { + const args = parseArguments('1.0.0', ['node', 'main.js', '--noLaunch']); + assert.deepStrictEqual(args, { + ...defaultArgs, + _: [], + headless: false, + isolated: false, + $0: 'npx chrome-devtools-mcp@latest', + 'no-launch': true, + noLaunch: true, + }); + }); + + it('noLaunch prevents channel default when no connection options provided', async () => { + const args = parseArguments('1.0.0', ['node', 'main.js', '--noLaunch']); + // When noLaunch is true and no connection options are provided, + // channel should not be defaulted to 'stable' + assert.strictEqual(args.noLaunch, true); + // The channel should not be set when noLaunch is true + assert.strictEqual(args.channel, undefined); + }); + + it('noLaunch with browserUrl', async () => { + const args = parseArguments('1.0.0', [ + 'node', + 'main.js', + '--noLaunch', + '--browserUrl', + 'http://localhost:9222', + ]); + assert.deepStrictEqual(args, { + ...defaultArgs, + _: [], + headless: false, + isolated: false, + $0: 'npx chrome-devtools-mcp@latest', + 'no-launch': true, + noLaunch: true, + 'browser-url': 'http://localhost:9222', + browserUrl: 'http://localhost:9222', + u: 'http://localhost:9222', + }); + }); }); diff --git a/tests/tools/switch_browser.test.ts b/tests/tools/switch_browser.test.ts new file mode 100644 index 00000000..6233db24 --- /dev/null +++ b/tests/tools/switch_browser.test.ts @@ -0,0 +1,372 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ +import assert from 'node:assert'; +import {describe, it} from 'node:test'; + +import logger from 'debug'; +import {executablePath} from 'puppeteer'; +import puppeteer, {Locator} from 'puppeteer'; + +import {getBrowser, setBrowser} from '../../src/browser.js'; +import {getContext, setContextInstance} from '../../src/context.js'; +import {McpContext} from '../../src/McpContext.js'; +import {McpResponse} from '../../src/McpResponse.js'; +import {switchBrowser} from '../../src/tools/switch_browser.js'; + +describe('switch_browser', () => { + it('throws error for unsupported protocol', async () => { + const browser = await puppeteer.launch({ + executablePath: executablePath(), + headless: true, + }); + + try { + await browser.newPage(); + const context = await McpContext.from( + browser, + logger('test'), + { + experimentalDevToolsDebugging: false, + }, + Locator, + ); + const response = new McpResponse(); + + setBrowser(browser); + setContextInstance(context); + + try { + await switchBrowser.handler( + {params: {url: 'ftp://example.com:9222'}}, + response, + context, + ); + assert.fail('Should have thrown an error'); + } catch (error) { + assert.ok(error instanceof Error); + assert.ok(error.message.includes('Unsupported protocol')); + assert.ok(error.message.includes('ftp:')); + } + + // Browser was closed by disconnectBrowser + context.dispose(); + } finally { + if (browser.connected) { + await browser.close(); + } + } + }); + + it('disconnects current browser and connects to new WebSocket endpoint', async () => { + const firstBrowser = await puppeteer.launch({ + executablePath: executablePath(), + headless: true, + }); + const secondBrowser = await puppeteer.launch({ + executablePath: executablePath(), + headless: true, + args: ['--remote-debugging-port=0'], + }); + + try { + await firstBrowser.newPage(); + const initialContext = await McpContext.from( + firstBrowser, + logger('test'), + { + experimentalDevToolsDebugging: false, + }, + Locator, + ); + const response = new McpResponse(); + + setBrowser(firstBrowser); + setContextInstance(initialContext); + + const wsEndpoint = secondBrowser.wsEndpoint(); + assert.ok(wsEndpoint, 'Second browser should have WebSocket endpoint'); + + // Verify initial state + assert.strictEqual(getBrowser(), firstBrowser); + assert.ok(firstBrowser.connected, 'Initial browser should be connected'); + + // Execute switch_browser + await switchBrowser.handler( + {params: {url: wsEndpoint}}, + response, + initialContext, + ); + + // Verify the first browser was disconnected + assert.ok( + !firstBrowser.connected, + 'Initial browser should be disconnected', + ); + + // Verify a new context was created + const newContext = getContext(); + assert.ok(newContext, 'New context should exist'); + assert.notStrictEqual( + newContext, + initialContext, + 'Should have created a new context', + ); + + // Verify the new context is connected to the second browser + const newBrowser = getBrowser(); + assert.ok(newBrowser, 'New browser should exist'); + assert.notStrictEqual( + newBrowser, + firstBrowser, + 'Should be connected to different browser', + ); + assert.ok(newBrowser.connected, 'New browser should be connected'); + + // Verify response messages + assert.ok( + response.responseLines.some(line => + line.includes('Connecting to browser via WebSocket'), + ), + ); + assert.ok( + response.responseLines.some(line => + line.includes('Successfully connected to browser'), + ), + ); + assert.ok(response.includePages); + + // First browser was closed by disconnectBrowser, don't try to close its pages + newContext?.dispose(); + } finally { + if (secondBrowser.connected) { + await secondBrowser.close(); + } + } + }); + + it('converts HTTP URL to WebSocket and connects', async () => { + const firstBrowser = await puppeteer.launch({ + executablePath: executablePath(), + headless: true, + }); + const secondBrowser = await puppeteer.launch({ + executablePath: executablePath(), + headless: true, + args: ['--remote-debugging-port=9224'], + }); + + try { + await firstBrowser.newPage(); + const initialContext = await McpContext.from( + firstBrowser, + logger('test'), + { + experimentalDevToolsDebugging: false, + }, + Locator, + ); + const response = new McpResponse(); + + setBrowser(firstBrowser); + setContextInstance(initialContext); + + const httpUrl = 'http://127.0.0.1:9224'; + + // Execute switch_browser with HTTP URL + await switchBrowser.handler( + {params: {url: httpUrl}}, + response, + initialContext, + ); + + // Verify first browser was disconnected + assert.ok( + !firstBrowser.connected, + 'Initial browser should be disconnected', + ); + + // Verify new connection + const newBrowser = getBrowser(); + assert.ok(newBrowser, 'New browser should exist'); + assert.ok(newBrowser.connected, 'New browser should be connected'); + + // Verify response messages for HTTP conversion + assert.ok( + response.responseLines.some(line => + line.includes('Fetching WebSocket endpoint from browser'), + ), + ); + assert.ok( + response.responseLines.some(line => + line.includes('Resolved WebSocket endpoint'), + ), + ); + assert.ok( + response.responseLines.some(line => + line.includes('Successfully connected to browser'), + ), + ); + + // First browser was closed by disconnectBrowser, don't try to close its pages + const newContext = getContext(); + newContext?.dispose(); + } finally { + if (secondBrowser.connected) { + await secondBrowser.close(); + } + } + }); + + it('respects timeout parameter and fails if connection takes too long', async () => { + const browser = await puppeteer.launch({ + executablePath: executablePath(), + headless: true, + }); + + try { + await browser.newPage(); + const context = await McpContext.from( + browser, + logger('test'), + { + experimentalDevToolsDebugging: false, + }, + Locator, + ); + const response = new McpResponse(); + + setBrowser(browser); + setContextInstance(context); + + // Use a WebSocket endpoint that won't respond + const fakeWsEndpoint = 'ws://127.0.0.1:59999/devtools/browser/fake'; + const shortTimeout = 1000; // 1 second + + try { + await switchBrowser.handler( + {params: {url: fakeWsEndpoint, timeout: shortTimeout}}, + response, + context, + ); + assert.fail('Should have thrown timeout or connection error'); + } catch (error) { + assert.ok( + error instanceof Error, + `Expected Error but got: ${typeof error} - ${String(error)}`, + ); + // Either timeout error or Puppeteer connection error is acceptable + const hasTimeoutMessage = error.message.includes( + 'Failed to connect to browser within', + ); + const hasConnectionError = + error.message.includes('Puppeteer connection failed') || + error.message.includes('Could not connect'); + assert.ok( + hasTimeoutMessage || hasConnectionError, + `Error should mention timeout or connection failure, got: ${error.message}`, + ); + } + + // Browser was closed by disconnectBrowser + context.dispose(); + } finally { + if (browser.connected) { + await browser.close(); + } + } + }); + + it('throws error when HTTP browser info endpoint fails', async () => { + const browser = await puppeteer.launch({ + executablePath: executablePath(), + headless: true, + }); + + try { + await browser.newPage(); + const context = await McpContext.from( + browser, + logger('test'), + { + experimentalDevToolsDebugging: false, + }, + Locator, + ); + const response = new McpResponse(); + + setBrowser(browser); + setContextInstance(context); + + // Use HTTP URL with no browser running + const httpUrl = 'http://127.0.0.1:59998'; + + try { + await switchBrowser.handler( + {params: {url: httpUrl}}, + response, + context, + ); + assert.fail('Should have thrown an error'); + } catch (error) { + assert.ok(error instanceof Error); + assert.ok( + error.message.includes('Could not connect to browser at'), + `Error should mention connection failure, got: ${error.message}`, + ); + } + + // Browser was closed by disconnectBrowser + context.dispose(); + } finally { + if (browser.connected) { + await browser.close(); + } + } + }); + + it('throws error for invalid URL', async () => { + const browser = await puppeteer.launch({ + executablePath: executablePath(), + headless: true, + }); + + try { + await browser.newPage(); + const context = await McpContext.from( + browser, + logger('test'), + { + experimentalDevToolsDebugging: false, + }, + Locator, + ); + const response = new McpResponse(); + + setBrowser(browser); + setContextInstance(context); + + try { + await switchBrowser.handler( + {params: {url: 'not-a-valid-url'}}, + response, + context, + ); + assert.fail('Should have thrown an error'); + } catch (error) { + assert.ok(error instanceof Error); + // URL constructor will throw for invalid URLs + assert.ok(error.message); + } + + // Browser was closed by disconnectBrowser + context.dispose(); + } finally { + if (browser.connected) { + await browser.close(); + } + } + }); +}); From 820b14517dcbe49ffcfa4b3d8560c480c3ca3307 Mon Sep 17 00:00:00 2001 From: elijahr Date: Wed, 19 Nov 2025 23:37:38 -0600 Subject: [PATCH 2/2] fix(switch_browser): add timeout to HTTP fetch to prevent infinite hangs The convertHttpToBrowserUrl() function previously had no timeout on the fetch() call, which could cause switch_browser to hang indefinitely when the target browser was unreachable or unresponsive. Changes: - Added AbortController with configurable timeout (default 10s) - Enhanced error messages to distinguish timeout from connection failures - Added test coverage for HTTP fetch timeout behavior - Updated documentation to reflect new timeout parameter Fixes potential infinite hang when using HTTP URLs with unreachable browsers. --- docs/tool-reference.md | 1 + src/tools/switch_browser.ts | 20 ++++++++- tests/tools/switch_browser.test.ts | 67 ++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 91b280cd..e4f7c6de 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -183,6 +183,7 @@ **Parameters:** +- **timeout** (number) _(optional)_: Connection timeout in milliseconds. Defaults to 10000 (10 seconds). If the connection cannot be established within this time, an error will be thrown. - **url** (string) **(required)**: Browser connection URL. Can be an HTTP URL (e.g., http://127.0.0.1:9222) which will be auto-converted to WebSocket, or a direct WebSocket endpoint (e.g., ws://127.0.0.1:52862/devtools/browser/<id>). --- diff --git a/src/tools/switch_browser.ts b/src/tools/switch_browser.ts index 7650e37d..2d4c7db4 100644 --- a/src/tools/switch_browser.ts +++ b/src/tools/switch_browser.ts @@ -14,9 +14,15 @@ import {defineTool} from './ToolDefinition.js'; async function convertHttpToBrowserUrl( url: string, + timeout: number = 10000, ): Promise { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), timeout); + try { - const response = await fetch(`${url}/json/version`); + const response = await fetch(`${url}/json/version`, { + signal: controller.signal, + }); if (!response.ok) { throw new Error(`Failed to fetch browser info: ${response.statusText}`); } @@ -24,9 +30,19 @@ async function convertHttpToBrowserUrl( return data.webSocketDebuggerUrl; } catch (error) { const message = error instanceof Error ? error.message : String(error); + + // Provide clearer error message for timeout/abort + if (error instanceof Error && error.name === 'AbortError') { + throw new Error( + `Connection timeout: Could not reach browser at ${url} within ${timeout}ms. Make sure the browser is running and accessible.`, + ); + } + throw new Error( `Could not connect to browser at ${url}: ${message}. Make sure the browser is running and the URL is correct.`, ); + } finally { + clearTimeout(timeoutId); } } @@ -73,7 +89,7 @@ export const switchBrowser = defineTool({ response.appendResponseLine( `Fetching WebSocket endpoint from browser at ${url}...`, ); - wsEndpoint = await convertHttpToBrowserUrl(url); + wsEndpoint = await convertHttpToBrowserUrl(url, timeout); response.appendResponseLine(`Resolved WebSocket endpoint: ${wsEndpoint}`); browserURL = url; } else { diff --git a/tests/tools/switch_browser.test.ts b/tests/tools/switch_browser.test.ts index 6233db24..d3bb3887 100644 --- a/tests/tools/switch_browser.test.ts +++ b/tests/tools/switch_browser.test.ts @@ -369,4 +369,71 @@ describe('switch_browser', () => { } } }); + + it('respects timeout parameter for HTTP fetch and fails quickly', async () => { + const browser = await puppeteer.launch({ + executablePath: executablePath(), + headless: true, + }); + + try { + await browser.newPage(); + const context = await McpContext.from( + browser, + logger('test'), + { + experimentalDevToolsDebugging: false, + }, + Locator, + ); + const response = new McpResponse(); + + setBrowser(browser); + setContextInstance(context); + + // Use a non-routable IP address (TEST-NET-1, should timeout not error immediately) + // and a very short timeout to verify fetch timeout works + const httpUrl = 'http://192.0.2.1:9222'; + const shortTimeout = 500; // 500ms + + const startTime = Date.now(); + + try { + await switchBrowser.handler( + {params: {url: httpUrl, timeout: shortTimeout}}, + response, + context, + ); + assert.fail('Should have thrown a timeout error'); + } catch (error) { + const elapsed = Date.now() - startTime; + + assert.ok(error instanceof Error); + + // Should fail within reasonable time (timeout + small buffer) + // Not strict assertion since network behavior varies + assert.ok( + elapsed < shortTimeout + 1000, + `Should timeout quickly, took ${elapsed}ms`, + ); + + // Check for timeout-related error message + const hasTimeoutMessage = + error.message.includes('Connection timeout') || + error.message.includes('Could not connect to browser at'); + + assert.ok( + hasTimeoutMessage, + `Error should mention timeout, got: ${error.message}`, + ); + } + + // Browser was closed by disconnectBrowser + context.dispose(); + } finally { + if (browser.connected) { + await browser.close(); + } + } + }); });