Fix EADDRINUSE race condition in certa. (iTwin#638)

Even though certa was detecting a free port, with enough simultaneous processes it was still possible for the frontend webserver to fail to start.  This should finally fix that by detecting/retrying until the webserver is actually started successfully.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

common/changes/@bentley/certa/fix-certa-ports_2021-01-22-18-20.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@bentley/certa",
+      "comment": "",
+      "type": "none"
+    }
+  ],
+  "packageName": "@bentley/certa",
+  "email": "[email protected]"
+}

full-stack-tests/core/src/backend/backend.ts

@@ -47,7 +47,7 @@ async function init() {
           public: "lib",
           headers: [{ source: "*", headers: [{ key: "Access-Control-Allow-Origin", value: "*" }] }],
         });
-      }).listen(Number(process.env.CERTA_PORT ?? 3011) + 3000, undefined, undefined, resolve);
+      }).listen(Number(process.env.CERTA_PORT ?? 3011) + 4000, undefined, undefined, resolve);
     });
   }
 

full-stack-tests/core/src/frontend/standalone/ExtensionAdmin.test.ts

@@ -13,7 +13,7 @@ describe("ExtensionAdmin tests", () => {
 
   if (!isElectronRenderer) {
     it("loads local extension", async () => {
-      IModelApp.extensionAdmin.addExtensionLoaderFront(new ExternalServerExtensionLoader(`http://localhost:${Number(window.location.port) + 3000}`));
+      IModelApp.extensionAdmin.addExtensionLoaderFront(new ExternalServerExtensionLoader(`http://localhost:${Number(window.location.port) + 4000}`));
 
       await IModelApp.extensionAdmin.loadExtension("loadingTestExtension");
 

tools/certa/src/runners/chrome/ChromeTestRunner.ts

@@ -4,7 +4,7 @@
 *--------------------------------------------------------------------------------------------*/
 import * as path from "path";
 import * as puppeteer from "puppeteer";
-import * as detect from "detect-port";
+import { ChildProcess } from "child_process";
 import { spawnChildProcess } from "../../utils/SpawnUtils";
 import { executeRegisteredCallback } from "../../utils/CallbackUtils";
 import { CertaConfig } from "../../CertaConfig";
@@ -19,6 +19,7 @@ interface ChromeTestResults {
 type ConsoleMethodName = keyof typeof console;
 
 let browser: puppeteer.Browser;
+let webserverProcess: ChildProcess;
 
 export class ChromeTestRunner {
   public static readonly supportsCoverage = true;
@@ -35,26 +36,23 @@ export class ChromeTestRunner {
 
     browser = await puppeteer.launch(options);
 
-    const openPort = await detect(config.ports.frontend);
-    if (openPort !== config.ports.frontend)
-      console.warn(`CERTA: Port ${config.ports.frontend} is already in use, so serving test resources on port ${openPort}`);
-
-    process.env.CERTA_PORT = String(openPort);
-  }
-
-  public static async runTests(config: CertaConfig): Promise<void> {
     const webserverEnv = {
-      CERTA_PORT: process.env.CERTA_PORT, // eslint-disable-line @typescript-eslint/naming-convention
+      CERTA_PORT: `${config.ports.frontend}`, // eslint-disable-line @typescript-eslint/naming-convention
       CERTA_PATH: path.join(__dirname, "../../../public/index.html"), // eslint-disable-line @typescript-eslint/naming-convention
       CERTA_PUBLIC_DIRS: JSON.stringify(config.chromeOptions.publicDirs), // eslint-disable-line @typescript-eslint/naming-convention
     };
-    const webserverProcess = spawnChildProcess("node", [require.resolve("./webserver")], webserverEnv, true);
-
-    // Don't start puppeteer until the webserver is started and listening.
-    const webserverExited = new Promise((_resolve, reject) => webserverProcess.once("exit", () => reject("Webserver exited!")));
-    const webserverStarted = new Promise((resolve) => webserverProcess.once("message", resolve));
-    await Promise.race([webserverExited, webserverStarted]);
+    webserverProcess = spawnChildProcess("node", [require.resolve("./webserver")], webserverEnv, true);
+
+    // Don't continue until the webserver is started and listening.
+    const webserverExited = new Promise<never>((_resolve, reject) => webserverProcess.once("exit", () => reject("Webserver exited!")));
+    const webserverStarted = new Promise<number>((resolve) => webserverProcess.once("message", resolve));
+    const actualPort = await Promise.race([webserverExited, webserverStarted]);
+    if (actualPort !== config.ports.frontend)
+      console.warn(`CERTA: Port ${config.ports.frontend} was already in use, so serving test resources on port ${actualPort}`);
+    process.env.CERTA_PORT = String(actualPort);
+  }
 
+  public static async runTests(config: CertaConfig): Promise<void> {
     // FIXME: Do we really want to always enforce this behavior?
     if (process.env.CI || process.env.TF_BUILD)
       (config.mochaOptions as any).forbidOnly = true;

tools/certa/src/runners/chrome/webserver.ts

@@ -3,7 +3,9 @@
 * See LICENSE.md in the project root for license terms and full copyright notice.
 *--------------------------------------------------------------------------------------------*/
 /* eslint-disable @typescript-eslint/naming-convention */
+import * as detect from "detect-port";
 import * as express from "express";
+import * as http from "http";
 import * as path from "path";
 
 const app = express();
@@ -52,9 +54,34 @@ app.use("*", (req, resp) => {
   resp.sendStatus(404);
 });
 
-// Run the server...
-app.set("port", process.env.CERTA_PORT || 3000);
-app.listen(app.get("port"), () => {
-  console.log(`Certa web frontend now listening on port ${app.get("port")}`);
-  process.send!("Ready");
+// Once the server actually starts, we should log and send the actual port back to the parent process
+let port = parseInt(process.env.CERTA_PORT ?? "3000", 10);
+const server = http.createServer(app);
+server.on("listening", async () => {
+  console.log(`Certa web frontend now listening on port ${port}`);
+  process.send!(port);
 });
+
+// The preferred port (process.env.CERTA_PORT) may be in use.  If that happens detect a free port and try again.
+// Even though we detect a free port, there can still be race conditions when many certa processes run simultaneously.
+// Therefore, we'll retry up to 4 times with a new free port.
+const maxRetries = 4;
+let numRetries = 0;
+server.on("error", async (e: any) => {
+  if (e.code !== "EADDRINUSE" || numRetries >= maxRetries) {
+    console.error(e);
+    process.exit(1);
+  }
+  try {
+    numRetries++;
+    port = await detect(port);
+    server.close();
+    server.listen(port);
+  } catch (error) {
+    console.error(error);
+    process.exit(1);
+  }
+});
+
+// Run the server...
+server.listen(port);