fix(connect): respect timeout in all scenarios (#6762)

Drive-by: fix console error in test arising from bad usage
of test websocket server in `connect()` calls.

src/client/browserType.ts

@@ -143,7 +143,12 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel, chann
           }
         });
       });
-      return await new Promise<Browser>(async (fulfill, reject) => {
+
+      let timeoutCallback = (e: Error) => {};
+      const timeoutPromise = new Promise<Browser>((f, r) => timeoutCallback = r);
+      const timer = params.timeout ? setTimeout(() => timeoutCallback(new Error(`Timeout ${params.timeout}ms exceeded.`)), params.timeout) : undefined;
+
+      const successPromise = new Promise<Browser>(async (fulfill, reject) => {
         if ((params as any).__testHookBeforeCreateBrowser) {
           try {
             await (params as any).__testHookBeforeCreateBrowser();
@@ -202,6 +207,13 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel, chann
           reject(new Error(event.message + '. Most likely ws endpoint is incorrect'));
         });
       });
+
+      try {
+        return await Promise.race([successPromise, timeoutPromise]);
+      } finally {
+        if (timer)
+          clearTimeout(timer);
+      }
     }, logger);
   }
 

tests/browsertype-connect.spec.ts

@@ -62,6 +62,14 @@ test('should be able to connect two browsers at the same time', async ({browserT
   await browser2.close();
 });
 
+test('should timeout while connecting', async ({browserType, startRemoteServer, server}) => {
+  const e = await browserType.connect({
+    wsEndpoint: `ws://localhost:${server.PORT}/ws`,
+    timeout: 100,
+  }).catch(e => e);
+  expect(e.message).toContain('browserType.connect: Timeout 100ms exceeded.');
+});
+
 test('should send extra headers with connect request', async ({browserType, startRemoteServer, server}) => {
   const [request] = await Promise.all([
     server.waitForWebSocketConnectionRequest(),
@@ -70,7 +78,8 @@ test('should send extra headers with connect request', async ({browserType, star
       headers: {
         'User-Agent': 'Playwright',
         'foo': 'bar',
-      }
+      },
+      timeout: 100,
     }).catch(() => {})
   ]);
   expect(request.headers['user-agent']).toBe('Playwright');

tests/capabilities.spec.ts

@@ -26,6 +26,7 @@ it('Web Assembly should work', async function({page, server, browserName, platfo
 });
 
 it('WebSocket should work', async ({page, server}) => {
+  server.sendOnWebSocketConnection('incoming');
   const value = await page.evaluate(port => {
     let cb;
     const result = new Promise(f => cb = f);

tests/chromium/chromium.spec.ts

@@ -217,7 +217,8 @@ playwrightTest('should send extra headers with connect request', async ({browser
         headers: {
           'User-Agent': 'Playwright',
           'foo': 'bar',
-        }
+        },
+        timeout: 100,
       }).catch(() => {})
     ]);
     expect(request.headers['user-agent']).toBe('Playwright');
@@ -231,7 +232,8 @@ playwrightTest('should send extra headers with connect request', async ({browser
         headers: {
           'User-Agent': 'Playwright',
           'foo': 'bar',
-        }
+        },
+        timeout: 100,
       }).catch(() => {})
     ]);
     expect(request.headers['user-agent']).toBe('Playwright');

tests/ignorehttpserrors.spec.ts

@@ -63,6 +63,7 @@ it('should work with mixed content', async ({browser, server, httpsServer}) => {
 });
 
 it('should work with WebSocket', async ({browser, httpsServer}) => {
+  httpsServer.sendOnWebSocketConnection('incoming');
   const context = await browser.newContext({ ignoreHTTPSErrors: true });
   const page = await context.newPage();
   const value = await page.evaluate(endpoint => {

tests/web-socket.spec.ts

@@ -19,6 +19,7 @@ import { contextTest as it, expect } from './config/browserTest';
 import { Server as WebSocketServer } from 'ws';
 
 it('should work', async ({ page, server }) => {
+  server.sendOnWebSocketConnection('incoming');
   const value = await page.evaluate(port => {
     let cb;
     const result = new Promise(f => cb = f);
@@ -49,6 +50,7 @@ it('should emit close events', async ({ page, server }) => {
 });
 
 it('should emit frame events', async ({ page, server }) => {
+  server.sendOnWebSocketConnection('incoming');
   let socketClosed;
   const socketClosePromise = new Promise(f => socketClosed = f);
   const log = [];
@@ -126,6 +128,7 @@ it('should emit error', async ({page, server, browserName}) => {
 });
 
 it('should not have stray error events', async ({page, server}) => {
+  server.sendOnWebSocketConnection('incoming');
   let error;
   page.on('websocket', ws => ws.on('socketerror', e => error = e));
   await Promise.all([
@@ -142,6 +145,7 @@ it('should not have stray error events', async ({page, server}) => {
 });
 
 it('should reject waitForEvent on socket close', async ({page, server}) => {
+  server.sendOnWebSocketConnection('incoming');
   const [ws] = await Promise.all([
     page.waitForEvent('websocket').then(async ws => {
       await ws.waitForEvent('framereceived');
@@ -157,6 +161,7 @@ it('should reject waitForEvent on socket close', async ({page, server}) => {
 });
 
 it('should reject waitForEvent on page close', async ({page, server}) => {
+  server.sendOnWebSocketConnection('incoming');
   const [ws] = await Promise.all([
     page.waitForEvent('websocket').then(async ws => {
       await ws.waitForEvent('framereceived');

utils/testserver/index.d.ts

@@ -29,6 +29,7 @@ export class TestServer {
   setRedirect(from: string, to: string);
   waitForRequest(path: string): Promise<IncomingMessage & {postBody: Buffer}>;
   waitForWebSocketConnectionRequest(): Promise<IncomingMessage>;
+  sendOnWebSocketConnection(data: string);
   reset();
   serveFile(request: IncomingMessage, response: ServerResponse);
   serveFile(request: IncomingMessage, response: ServerResponse, filePath: string);

utils/testserver/index.js

@@ -71,7 +71,10 @@ class TestServer {
       this._server = http.createServer(this._onRequest.bind(this));
     this._server.on('connection', socket => this._onSocket(socket));
     this._wsServer = new WebSocketServer({server: this._server, path: '/ws'});
-    this._wsServer.on('connection', this._onWebSocketConnection.bind(this));
+    this._wsServer.on('connection', ws => {
+      if (this._onWebSocketConnectionData !== undefined)
+        ws.send(this._onWebSocketConnectionData);
+    });
     this._server.listen(port);
     this._dirPath = dirPath;
     this.debugServer = require('debug')('pw:server');
@@ -91,6 +94,8 @@ class TestServer {
     this._gzipRoutes = new Set();
     /** @type {!Map<string, !Promise>} */
     this._requestSubscribers = new Map();
+    /** @type {string|undefined} */
+    this._onWebSocketConnectionData = undefined;
 
     const cross_origin = loopback || '127.0.0.1';
     const same_origin = loopback || 'localhost';
@@ -299,8 +304,8 @@ class TestServer {
     });
   }
 
-  _onWebSocketConnection(ws) {
+  sendOnWebSocketConnection(data) {
-    ws.send('incoming');
+    this._onWebSocketConnectionData = data;
   }
 }