Fix redundant conditional logic in browser_evaluate error test

Co-authored-by: pavelfeldman <883973+pavelfeldman@users.noreply.github.com>

README.md

@@ -99,7 +99,13 @@ Click <code>Save</code>.
 <details>
 <summary>VS Code</summary>
 
-You can also install the Playwright MCP server using the VS Code CLI:
+#### Click the button to install:
+
+[<img src="https://img.shields.io/badge/VS_Code-VS_Code?style=flat-square&label=Install%20Server&color=0098FF" alt="Install in VS Code">](https://insiders.vscode.dev/redirect?url=vscode%3Amcp%2Finstall%3F%257B%2522name%2522%253A%2522playwright%2522%252C%2522command%2522%253A%2522npx%2522%252C%2522args%2522%253A%255B%2522%2540playwright%252Fmcp%2540latest%2522%255D%257D) [<img alt="Install in VS Code Insiders" src="https://img.shields.io/badge/VS_Code_Insiders-VS_Code_Insiders?style=flat-square&label=Install%20Server&color=24bfa5">](https://insiders.vscode.dev/redirect?url=vscode-insiders%3Amcp%2Finstall%3F%257B%2522name%2522%253A%2522playwright%2522%252C%2522command%2522%253A%2522npx%2522%252C%2522args%2522%253A%255B%2522%2540playwright%252Fmcp%2540latest%2522%255D%257D)
+
+#### Or install manually:
+
+Follow the MCP install [guide](https://code.visualstudio.com/docs/copilot/chat/mcp-servers#_add-an-mcp-server), use the standard config above. You can also install the Playwright MCP server using the VS Code CLI:
 
 ```bash
 # For VS Code
@@ -297,26 +303,24 @@ npx @playwright/mcp@latest --config path/to/config.json
 ### Standalone MCP server
 
 When running headed browser on system w/o display or from worker processes of the IDEs,
-run the MCP server from environment with the DISPLAY and pass the `--port` flag to enable HTTP transport.
+run the MCP server from environment with the DISPLAY and pass the `--port` flag to enable SSE transport.
 
 ```bash
 npx @playwright/mcp@latest --port 8931
 ```
 
-And then in MCP client config, set the `url` to the MCP endpoint:
+And then in MCP client config, set the `url` to the SSE endpoint:
 
 ```js
 {
   "mcpServers": {
     "playwright": {
-      "url": "http://localhost:8931/mcp"
+      "url": "http://localhost:8931/sse"
     }
   }
 }
 ```
 
-For legacy SSE transport support, you can use `/sse` instead of `/mcp` in the URL.
-
 <details>
 <summary><b>Docker</b></summary>
 

src/transport.ts

@@ -88,14 +88,7 @@ async function handleStreamable(server: Server, req: http.IncomingMessage, res:
       if (transport.sessionId)
         sessions.delete(transport.sessionId);
     };
-    const connection = await server.createConnection(transport);
-    // Ensure connection is closed when transport closes
-    transport.onclose = () => {
-      if (transport.sessionId)
-        sessions.delete(transport.sessionId);
-      // eslint-disable-next-line no-console
-      void connection.close().catch(e => console.error(e));
-    };
+    await server.createConnection(transport);
     await transport.handleRequest(req, res);
     return;
   }
@@ -122,10 +115,10 @@ export function startHttpTransport(httpServer: http.Server, mcpServer: Server) {
   const streamableSessions = new Map<string, StreamableHTTPServerTransport>();
   httpServer.on('request', async (req, res) => {
     const url = new URL(`http://localhost${req.url}`);
-    if (url.pathname.startsWith('/sse'))
-      await handleSSE(mcpServer, req, res, url, sseSessions);
-    else
+    if (url.pathname.startsWith('/mcp'))
       await handleStreamable(mcpServer, req, res, streamableSessions);
+    else
+      await handleSSE(mcpServer, req, res, url, sseSessions);
   });
   const url = httpAddressToString(httpServer.address());
   const message = [
@@ -134,11 +127,11 @@ export function startHttpTransport(httpServer: http.Server, mcpServer: Server) {
     JSON.stringify({
       'mcpServers': {
         'playwright': {
-          'url': `${url}/mcp`
+          'url': `${url}/sse`
         }
       }
     }, undefined, 2),
-    'For legacy SSE transport support, you can use the /sse endpoint instead.',
+    'If your client supports streamable HTTP, you can use the /mcp endpoint instead.',
   ].join('\n');
     // eslint-disable-next-line no-console
   console.error(message);

tests/evaluate.spec.ts

@@ -49,3 +49,24 @@ test('browser_evaluate (element)', async ({ client, server }) => {
     },
   })).toContainTextContent(`- Result: "red"`);
 });
+
+test('browser_evaluate (error)', async ({ client, server }) => {
+  expect(await client.callTool({
+    name: 'browser_navigate',
+    arguments: { url: server.HELLO_WORLD },
+  })).toContainTextContent(`- Page Title: Title`);
+
+  // Test with a bogus expression that will cause a JavaScript error
+  const result = await client.callTool({
+    name: 'browser_evaluate',
+    arguments: {
+      function: '() => { undefinedVariable.nonExistentMethod(); }',
+    },
+  });
+
+  // Check that error MCP response is returned
+  expect(result.isError).toBe(true);
+
+  // Check that JavaScript error details are contained in the response
+  expect(result.content?.[0].text).toContain('undefinedVariable is not defined');
+});

tests/sse.spec.ts

@@ -66,16 +66,15 @@ const test = baseTest.extend<{ serverEndpoint: (options?: { args?: string[], noP
   },
 });
 
-test('http transport', async ({ serverEndpoint }) => {
+test('sse transport', async ({ serverEndpoint }) => {
   const { url } = await serverEndpoint();
-  const transport = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport = new SSEClientTransport(url);
   const client = new Client({ name: 'test', version: '1.0.0' });
   await client.connect(transport);
   await client.ping();
-  expect(transport.sessionId, 'has session support').toBeDefined();
 });
 
-test('http transport (config)', async ({ serverEndpoint }) => {
+test('sse transport (config)', async ({ serverEndpoint }) => {
   const config: Config = {
     server: {
       port: 0,
@@ -85,16 +84,16 @@ test('http transport (config)', async ({ serverEndpoint }) => {
   await fs.promises.writeFile(configFile, JSON.stringify(config, null, 2));
 
   const { url } = await serverEndpoint({ noPort: true, args: ['--config=' + configFile] });
-  const transport = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport = new SSEClientTransport(url);
   const client = new Client({ name: 'test', version: '1.0.0' });
   await client.connect(transport);
   await client.ping();
 });
 
-test('http transport browser lifecycle (isolated)', async ({ serverEndpoint, server }) => {
+test('sse transport browser lifecycle (isolated)', async ({ serverEndpoint, server }) => {
   const { url, stderr } = await serverEndpoint({ args: ['--isolated'] });
 
-  const transport1 = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport1 = new SSEClientTransport(url);
   const client1 = new Client({ name: 'test', version: '1.0.0' });
   await client1.connect(transport1);
   await client1.callTool({
@@ -103,7 +102,7 @@ test('http transport browser lifecycle (isolated)', async ({ serverEndpoint, ser
   });
   await client1.close();
 
-  const transport2 = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport2 = new SSEClientTransport(url);
   const client2 = new Client({ name: 'test', version: '1.0.0' });
   await client2.connect(transport2);
   await client2.callTool({
@@ -114,6 +113,9 @@ test('http transport browser lifecycle (isolated)', async ({ serverEndpoint, ser
 
   await expect(async () => {
     const lines = stderr().split('\n');
+    expect(lines.filter(line => line.match(/create SSE session/)).length).toBe(2);
+    expect(lines.filter(line => line.match(/delete SSE session/)).length).toBe(2);
+
     expect(lines.filter(line => line.match(/create context/)).length).toBe(2);
     expect(lines.filter(line => line.match(/close context/)).length).toBe(2);
 
@@ -125,10 +127,10 @@ test('http transport browser lifecycle (isolated)', async ({ serverEndpoint, ser
   }).toPass();
 });
 
-test('http transport browser lifecycle (isolated, multiclient)', async ({ serverEndpoint, server }) => {
+test('sse transport browser lifecycle (isolated, multiclient)', async ({ serverEndpoint, server }) => {
   const { url, stderr } = await serverEndpoint({ args: ['--isolated'] });
 
-  const transport1 = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport1 = new SSEClientTransport(url);
   const client1 = new Client({ name: 'test', version: '1.0.0' });
   await client1.connect(transport1);
   await client1.callTool({
@@ -136,7 +138,7 @@ test('http transport browser lifecycle (isolated, multiclient)', async ({ server
     arguments: { url: server.HELLO_WORLD },
   });
 
-  const transport2 = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport2 = new SSEClientTransport(url);
   const client2 = new Client({ name: 'test', version: '1.0.0' });
   await client2.connect(transport2);
   await client2.callTool({
@@ -145,7 +147,7 @@ test('http transport browser lifecycle (isolated, multiclient)', async ({ server
   });
   await client1.close();
 
-  const transport3 = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport3 = new SSEClientTransport(url);
   const client3 = new Client({ name: 'test', version: '1.0.0' });
   await client3.connect(transport3);
   await client3.callTool({
@@ -158,6 +160,9 @@ test('http transport browser lifecycle (isolated, multiclient)', async ({ server
 
   await expect(async () => {
     const lines = stderr().split('\n');
+    expect(lines.filter(line => line.match(/create SSE session/)).length).toBe(3);
+    expect(lines.filter(line => line.match(/delete SSE session/)).length).toBe(3);
+
     expect(lines.filter(line => line.match(/create context/)).length).toBe(3);
     expect(lines.filter(line => line.match(/close context/)).length).toBe(3);
 
@@ -169,10 +174,10 @@ test('http transport browser lifecycle (isolated, multiclient)', async ({ server
   }).toPass();
 });
 
-test('http transport browser lifecycle (persistent)', async ({ serverEndpoint, server }) => {
+test('sse transport browser lifecycle (persistent)', async ({ serverEndpoint, server }) => {
   const { url, stderr } = await serverEndpoint();
 
-  const transport1 = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport1 = new SSEClientTransport(url);
   const client1 = new Client({ name: 'test', version: '1.0.0' });
   await client1.connect(transport1);
   await client1.callTool({
@@ -181,7 +186,7 @@ test('http transport browser lifecycle (persistent)', async ({ serverEndpoint, s
   });
   await client1.close();
 
-  const transport2 = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport2 = new SSEClientTransport(url);
   const client2 = new Client({ name: 'test', version: '1.0.0' });
   await client2.connect(transport2);
   await client2.callTool({
@@ -192,6 +197,9 @@ test('http transport browser lifecycle (persistent)', async ({ serverEndpoint, s
 
   await expect(async () => {
     const lines = stderr().split('\n');
+    expect(lines.filter(line => line.match(/create SSE session/)).length).toBe(2);
+    expect(lines.filter(line => line.match(/delete SSE session/)).length).toBe(2);
+
     expect(lines.filter(line => line.match(/create context/)).length).toBe(2);
     expect(lines.filter(line => line.match(/close context/)).length).toBe(2);
 
@@ -203,10 +211,10 @@ test('http transport browser lifecycle (persistent)', async ({ serverEndpoint, s
   }).toPass();
 });
 
-test('http transport browser lifecycle (persistent, multiclient)', async ({ serverEndpoint, server }) => {
+test('sse transport browser lifecycle (persistent, multiclient)', async ({ serverEndpoint, server }) => {
   const { url } = await serverEndpoint();
 
-  const transport1 = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport1 = new SSEClientTransport(url);
   const client1 = new Client({ name: 'test', version: '1.0.0' });
   await client1.connect(transport1);
   await client1.callTool({
@@ -214,7 +222,7 @@ test('http transport browser lifecycle (persistent, multiclient)', async ({ serv
     arguments: { url: server.HELLO_WORLD },
   });
 
-  const transport2 = new StreamableHTTPClientTransport(new URL('/mcp', url));
+  const transport2 = new SSEClientTransport(url);
   const client2 = new Client({ name: 'test', version: '1.0.0' });
   await client2.connect(transport2);
   const response = await client2.callTool({
@@ -228,17 +236,9 @@ test('http transport browser lifecycle (persistent, multiclient)', async ({ serv
   await client2.close();
 });
 
-test('sse transport', async ({ serverEndpoint }) => {
+test('streamable http transport', async ({ serverEndpoint }) => {
   const { url } = await serverEndpoint();
-  const transport = new SSEClientTransport(new URL('/sse', url));
-  const client = new Client({ name: 'test', version: '1.0.0' });
-  await client.connect(transport);
-  await client.ping();
-});
-
-test('http transport (default)', async ({ serverEndpoint }) => {
-  const { url } = await serverEndpoint();
-  const transport = new StreamableHTTPClientTransport(url);
+  const transport = new StreamableHTTPClientTransport(new URL('/mcp', url));
   const client = new Client({ name: 'test', version: '1.0.0' });
   await client.connect(transport);
   await client.ping();