fix(launchServer): wait for the server to start before taking its address (#4513)

This is easily triggered by launching from a cluster's worker.

src/browserServerImpl.ts

@@ -48,7 +48,7 @@ export class BrowserServerLauncherImpl implements BrowserServerLauncher {
       ignoreAllDefaultArgs: !!options.ignoreDefaultArgs && !Array.isArray(options.ignoreDefaultArgs),
       env: options.env ? envObjectToArray(options.env) : undefined,
     }, toProtocolLogger(options.logger));
-    return new BrowserServerImpl(browser, options.port);
+    return BrowserServerImpl.start(browser, options.port);
   }
 }
 
@@ -57,17 +57,30 @@ export class BrowserServerImpl extends EventEmitter implements BrowserServer {
   private _browser: Browser;
   private _wsEndpoint: string;
   private _process: ChildProcess;
+  private _ready: Promise<void>;
 
-  constructor(browser: Browser, port: number = 0) {
+  static async start(browser: Browser, port: number = 0): Promise<BrowserServerImpl> {
+    const server = new BrowserServerImpl(browser, port);
+    await server._ready;
+    return server;
+  }
+
+  constructor(browser: Browser, port: number) {
     super();
 
     this._browser = browser;
+    this._wsEndpoint = '';
+    this._process = browser._options.browserProcess.process!;
+
+    let readyCallback = () => {};
+    this._ready = new Promise<void>(f => readyCallback = f);
 
     const token = createGuid();
-    this._server = new ws.Server({ port });
+    this._server = new ws.Server({ port }, () => {
       const address = this._server.address();
       this._wsEndpoint = typeof address === 'string' ? `${address}/${token}` : `ws://127.0.0.1:${address.port}/${token}`;
-    this._process = browser._options.browserProcess.process!;
+      readyCallback();
+    });
 
     this._server.on('connection', (socket: ws, req) => {
       if (req.url !== '/' + token) {

test/browsertype-launch-server.spec.ts

@@ -15,7 +15,8 @@
  * limitations under the License.
  */
 
-import { it, expect, describe } from './fixtures';
+import { folio } from './remoteServer.fixture';
+const { it, expect, describe } = folio;
 
 describe('lauch server', (suite, { mode }) => {
   suite.skip(mode !== 'default');
@@ -80,4 +81,10 @@ describe('lauch server', (suite, { mode }) => {
     expect(logs.some(log => log.startsWith('protocol:verbose:SEND ►'))).toBe(true);
     expect(logs.some(log => log.startsWith('protocol:verbose:◀ RECV'))).toBe(true);
   });
+
+  it('should work with cluster', async ({browserType, clusterRemoteServer}) => {
+    const browser = await browserType.connect({ wsEndpoint: clusterRemoteServer.wsEndpoint() });
+    const page = await browser.newPage();
+    expect(await page.evaluate('1 + 2')).toBe(3);
+  });
 });

test/fixtures/closeme.js

@@ -1,4 +1,6 @@
-(async() => {
+const cluster = require('cluster');
+
+async function start() {
   const { playwrightPath, browserTypeName, launchOptions, stallOnClose } = JSON.parse(process.argv[2]);
   if (stallOnClose) {
     launchOptions.__testHookGracefullyClose = () => {
@@ -16,4 +18,13 @@
   });
   console.log(`(pid=>${browserServer.process().pid})`);
   console.log(`(wsEndpoint=>${browserServer.wsEndpoint()})`);
-})();
+}
+
+if (cluster.isWorker || !JSON.parse(process.argv[2]).inCluster) {
+  start();
+} else {
+  cluster.fork();
+  cluster.on('exit', (worker, code, signal) => {
+    process.exit(0);
+  });
+}

test/remoteServer.fixture.ts

@@ -23,6 +23,7 @@ import type { BrowserType, Browser, LaunchOptions } from '..';
 type ServerFixtures = {
   remoteServer: RemoteServer;
   stallingRemoteServer: RemoteServer;
+  clusterRemoteServer: RemoteServer;
 };
 const fixtures = base.extend<ServerFixtures>();
 
@@ -40,6 +41,13 @@ fixtures.stallingRemoteServer.init(async ({ browserType, browserOptions }, run)
   await remoteServer.close();
 });
 
+fixtures.clusterRemoteServer.init(async ({ browserType, browserOptions }, run) => {
+  const remoteServer = new RemoteServer();
+  await remoteServer._start(browserType, browserOptions, { inCluster: true });
+  await run(remoteServer);
+  await remoteServer.close();
+});
+
 export const folio = fixtures.build();
 
 const playwrightPath = path.join(__dirname, '..');
@@ -55,7 +63,7 @@ export class RemoteServer {
   _didExit: boolean;
   _wsEndpoint: string;
 
-  async _start(browserType: BrowserType<Browser>, browserOptions: LaunchOptions, extraOptions?: { stallOnClose: boolean; }) {
+  async _start(browserType: BrowserType<Browser>, browserOptions: LaunchOptions, extraOptions?: { stallOnClose?: boolean; inCluster?: boolean }) {
     this._output = new Map();
     this._outputCallback = new Map();
     this._didExit = false;