MCP/playwright-mcp
1
0
Fork 0
mirror of https://github.com/microsoft/playwright-mcp.git synced 2025-07-27 00:52:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: sticky launch errors (#324)

Browse Source 
This fixes an issue that there were sticky launch errors. When the
[following code
path](a15f0f301b/src/context.ts (L307-L339))
was throwing, the Error was stored in the Promise and not cleared
afterwards, this meant:

- If a browser was not there and the user tried to install it via
`browser_install` it was never working since the error was sticky.
- If other errors like CDP is not available yet etc. error appear a
re-connect would not work - the MCP server would require a restart.

Test plan: Since we don't have any `browser_install` tests I added a CDP
test for now to cover this bug.
This commit is contained in:
Max Schmitt 2025-05-02 15:32:37 +02:00 committed by GitHub
parent a713300c5b
commit 062cdd0704
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 54 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/context.ts
View File

@ -299,8 +299,12 @@ ${code.join('\n')}
} }
private async _createBrowserContext(): Promise<{ browser?: playwright.Browser, browserContext: playwright.BrowserContext }> { private async _createBrowserContext(): Promise<{ browser?: playwright.Browser, browserContext: playwright.BrowserContext }> {
if (!this._createBrowserContextPromise) if (!this._createBrowserContextPromise) {
this._createBrowserContextPromise = this._innerCreateBrowserContext(); this._createBrowserContextPromise = this._innerCreateBrowserContext();
void this._createBrowserContextPromise.catch(() => {
this._createBrowserContextPromise = undefined;
});
}
return this._createBrowserContextPromise; return this._createBrowserContextPromise;
} }

22
tests/cdp.spec.ts
View File

@ -17,7 +17,7 @@
import { test, expect } from './fixtures.js'; import { test, expect } from './fixtures.js';
test('cdp server', async ({ cdpEndpoint, startClient }) => { test('cdp server', async ({ cdpEndpoint, startClient }) => {
const client = await startClient({ args: [`--cdp-endpoint=${cdpEndpoint}`] }); const client = await startClient({ args: [`--cdp-endpoint=${await cdpEndpoint()}`] });
expect(await client.callTool({ expect(await client.callTool({
name: 'browser_navigate', name: 'browser_navigate',
arguments: { arguments: {
@ -27,7 +27,7 @@ test('cdp server', async ({ cdpEndpoint, startClient }) => {
}); });
test('cdp server reuse tab', async ({ cdpEndpoint, startClient }) => { test('cdp server reuse tab', async ({ cdpEndpoint, startClient }) => {
const client = await startClient({ args: [`--cdp-endpoint=${cdpEndpoint}`] }); const client = await startClient({ args: [`--cdp-endpoint=${await cdpEndpoint()}`] });
expect(await client.callTool({ expect(await client.callTool({
name: 'browser_click', name: 'browser_click',
@ -54,3 +54,21 @@ test('cdp server reuse tab', async ({ cdpEndpoint, startClient }) => {
\`\`\` \`\`\`
`); `);
}); });
test('should throw connection error and allow re-connecting', async ({ cdpEndpoint, startClient }) => {
const port = 3200 + test.info().parallelIndex;
const client = await startClient({ args: [`--cdp-endpoint=http://localhost:${port}`] });
expect(await client.callTool({
name: 'browser_navigate',
arguments: {
url: 'data:text/html,<html><title>Title</title><body>Hello, world!</body></html>',
},
})).toContainTextContent(`Error: browserType.connectOverCDP: connect ECONNREFUSED`);
await cdpEndpoint(port);
expect(await client.callTool({
name: 'browser_navigate',
arguments: {
url: 'data:text/html,<html><title>Title</title><body>Hello, world!</body></html>',
},
})).toContainTextContent(`- text: Hello, world!`);
});

22
tests/fixtures.ts
View File

@ -22,7 +22,7 @@ import { chromium } from 'playwright';
import { test as baseTest, expect as baseExpect } from '@playwright/test'; import { test as baseTest, expect as baseExpect } from '@playwright/test';
import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js';
import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { spawn } from 'child_process'; import { ChildProcessWithoutNullStreams, spawn } from 'child_process';
import { TestServer } from './testserver/index.ts'; import { TestServer } from './testserver/index.ts';
import type { Config } from '../config'; import type { Config } from '../config';
@ -36,7 +36,7 @@ type TestFixtures = {
visionClient: Client; visionClient: Client;
startClient: (options?: { args?: string[], config?: Config }) => Promise<Client>; startClient: (options?: { args?: string[], config?: Config }) => Promise<Client>;
wsEndpoint: string; wsEndpoint: string;
cdpEndpoint: string; cdpEndpoint: (port?: number) => Promise<string>;
server: TestServer; server: TestServer;
httpsServer: TestServer; httpsServer: TestServer;
mcpHeadless: boolean; mcpHeadless: boolean;
@ -95,9 +95,14 @@ export const test = baseTest.extend<TestFixtures & TestOptions, WorkerFixtures>(
}, },
cdpEndpoint: async ({ }, use, testInfo) => { cdpEndpoint: async ({ }, use, testInfo) => {
const port = 3200 + (+process.env.TEST_PARALLEL_INDEX!); let browserProcess: ChildProcessWithoutNullStreams | undefined;
const executablePath = chromium.executablePath();
const browserProcess = spawn(executablePath, [ await use(async port => {
if (!port)
port = 3200 + test.info().parallelIndex;
if (browserProcess)
return `http://localhost:${port}`;
browserProcess = spawn(chromium.executablePath(), [
`--user-data-dir=${testInfo.outputPath('user-data-dir')}`, `--user-data-dir=${testInfo.outputPath('user-data-dir')}`,
`--remote-debugging-port=${port}`, `--remote-debugging-port=${port}`,
`--no-first-run`, `--no-first-run`,
@ -109,13 +114,14 @@ export const test = baseTest.extend<TestFixtures & TestOptions, WorkerFixtures>(
stdio: 'pipe', stdio: 'pipe',
}); });
await new Promise<void>(resolve => { await new Promise<void>(resolve => {
browserProcess.stderr.on('data', data => { browserProcess!.stderr.on('data', data => {
if (data.toString().includes('DevTools listening on ')) if (data.toString().includes('DevTools listening on '))
resolve(); resolve();
}); });
}); });
await use(`http://localhost:${port}`); return `http://localhost:${port}`;
browserProcess.kill(); });
browserProcess?.kill();
}, },
mcpHeadless: async ({ headless }, use) => { mcpHeadless: async ({ headless }, use) => {

4
tests/tabs.spec.ts
View File

@ -142,11 +142,11 @@ test('close tab', async ({ client }) => {
}); });
test('reuse first tab when navigating', async ({ startClient, cdpEndpoint }) => { test('reuse first tab when navigating', async ({ startClient, cdpEndpoint }) => {
const browser = await chromium.connectOverCDP(cdpEndpoint); const browser = await chromium.connectOverCDP(await cdpEndpoint());
const [context] = browser.contexts(); const [context] = browser.contexts();
const pages = context.pages(); const pages = context.pages();
const client = await startClient({ args: [`--cdp-endpoint=${cdpEndpoint}`] }); const client = await startClient({ args: [`--cdp-endpoint=${await cdpEndpoint()}`] });
await client.callTool({ await client.callTool({
name: 'browser_navigate', name: 'browser_navigate',
arguments: { arguments: {