Skip to content

Commit 820b145

Browse files
committed
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.
1 parent d4aaba4 commit 820b145

3 files changed

Lines changed: 86 additions & 2 deletions

File tree

docs/tool-reference.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@
183183

184184
**Parameters:**
185185

186+
- **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.
186187
- **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>).
187188

188189
---

src/tools/switch_browser.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,35 @@ import {defineTool} from './ToolDefinition.js';
1414

1515
async function convertHttpToBrowserUrl(
1616
url: string,
17+
timeout: number = 10000,
1718
): Promise<string | undefined> {
19+
const controller = new AbortController();
20+
const timeoutId = setTimeout(() => controller.abort(), timeout);
21+
1822
try {
19-
const response = await fetch(`${url}/json/version`);
23+
const response = await fetch(`${url}/json/version`, {
24+
signal: controller.signal,
25+
});
2026
if (!response.ok) {
2127
throw new Error(`Failed to fetch browser info: ${response.statusText}`);
2228
}
2329
const data = await response.json();
2430
return data.webSocketDebuggerUrl;
2531
} catch (error) {
2632
const message = error instanceof Error ? error.message : String(error);
33+
34+
// Provide clearer error message for timeout/abort
35+
if (error instanceof Error && error.name === 'AbortError') {
36+
throw new Error(
37+
`Connection timeout: Could not reach browser at ${url} within ${timeout}ms. Make sure the browser is running and accessible.`,
38+
);
39+
}
40+
2741
throw new Error(
2842
`Could not connect to browser at ${url}: ${message}. Make sure the browser is running and the URL is correct.`,
2943
);
44+
} finally {
45+
clearTimeout(timeoutId);
3046
}
3147
}
3248

@@ -73,7 +89,7 @@ export const switchBrowser = defineTool({
7389
response.appendResponseLine(
7490
`Fetching WebSocket endpoint from browser at ${url}...`,
7591
);
76-
wsEndpoint = await convertHttpToBrowserUrl(url);
92+
wsEndpoint = await convertHttpToBrowserUrl(url, timeout);
7793
response.appendResponseLine(`Resolved WebSocket endpoint: ${wsEndpoint}`);
7894
browserURL = url;
7995
} else {

tests/tools/switch_browser.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,4 +369,71 @@ describe('switch_browser', () => {
369369
}
370370
}
371371
});
372+
373+
it('respects timeout parameter for HTTP fetch and fails quickly', async () => {
374+
const browser = await puppeteer.launch({
375+
executablePath: executablePath(),
376+
headless: true,
377+
});
378+
379+
try {
380+
await browser.newPage();
381+
const context = await McpContext.from(
382+
browser,
383+
logger('test'),
384+
{
385+
experimentalDevToolsDebugging: false,
386+
},
387+
Locator,
388+
);
389+
const response = new McpResponse();
390+
391+
setBrowser(browser);
392+
setContextInstance(context);
393+
394+
// Use a non-routable IP address (TEST-NET-1, should timeout not error immediately)
395+
// and a very short timeout to verify fetch timeout works
396+
const httpUrl = 'http://192.0.2.1:9222';
397+
const shortTimeout = 500; // 500ms
398+
399+
const startTime = Date.now();
400+
401+
try {
402+
await switchBrowser.handler(
403+
{params: {url: httpUrl, timeout: shortTimeout}},
404+
response,
405+
context,
406+
);
407+
assert.fail('Should have thrown a timeout error');
408+
} catch (error) {
409+
const elapsed = Date.now() - startTime;
410+
411+
assert.ok(error instanceof Error);
412+
413+
// Should fail within reasonable time (timeout + small buffer)
414+
// Not strict assertion since network behavior varies
415+
assert.ok(
416+
elapsed < shortTimeout + 1000,
417+
`Should timeout quickly, took ${elapsed}ms`,
418+
);
419+
420+
// Check for timeout-related error message
421+
const hasTimeoutMessage =
422+
error.message.includes('Connection timeout') ||
423+
error.message.includes('Could not connect to browser at');
424+
425+
assert.ok(
426+
hasTimeoutMessage,
427+
`Error should mention timeout, got: ${error.message}`,
428+
);
429+
}
430+
431+
// Browser was closed by disconnectBrowser
432+
context.dispose();
433+
} finally {
434+
if (browser.connected) {
435+
await browser.close();
436+
}
437+
}
438+
});
372439
});

0 commit comments

Comments
 (0)