server: Fix stuck test process when using qtap.run() with file not found

This didn't affect the CLI because:
* that calls the runWaitFor() wrapper, which happened to work because
  it rejects after an 'error' event. And, while the main place to
  emit the 'error' event is qtap.run() and we don't reach that here,
  there was code in server.js emitting this directly.
* it then calls process.exit(1), thus causing the HTTP Server instance
  to be force-closed.

In testing, however, while qtap.run() and qtap.runWaitFor() work fine,
the server stays running in the background, and thus the qunit process
hangs forever because server.js emitted the 'error' event, while
qtap.run() is still awaiting browserPromise, and thus we never got
to the "finally", and thus the server still running.

Avoid this kind of out-of-order processing by making sure the only
place to emit the 'error' event is the qtap.run() code, and only after
the 'finally' branch. So instead, pass the error to browser.stop().
That way, browserPromise will settle by throwing, and from there it
will bubble up to qtap.run().

Author: Krinkle

src/qtap.js

@@ -222,8 +222,6 @@ function run (files, browserNames = 'detect', runOptions = {}) {
   })();
   runPromise
     .finally(() => {
-      // Make sure we close our server even if the above throws, so that Node.js
-      // may naturally exit (no open ports remaining)
       for (const server of servers) {
         server.close();
       }

src/server.js

@@ -90,7 +90,7 @@ class ControlServer {
     this.browsers = new Map();
     // Optimization: Prefetch test file in parallel with server creation and browser launching.
     //
-    // To prevent a global error (unhandledRejection), we add a no-op catch() handler here.
+    // To prevent a Node.js error (unhandledRejection), we add a no-op catch() handler here.
     // Once launchBrowser is called, we will await this in handleRequest/getTestFile,
     // which is then propertly caught by server.on('request') below, which emits it
     // as 'error' event.
@@ -130,9 +130,20 @@ class ControlServer {
             await this.handleRequest(req, url, resp);
         }
       } catch (e) {
-        this.logger.warning('respond_uncaught', req.url, String(e));
-        eventbus.emit('error', e);
+        this.logger.warning('server_respond_uncaught', e);
         this.serveError(resp, 500, /** @type {Error} */ (e));
+
+        // When we catch this, qtap.run() is awaiting ControlServer#launchBrowser
+        // (as browerPromise). Make sure we don't get stuck, by stopping the browsers.
+        // That way:
+        // - launchBrowser() will throw/return,
+        // - qtap.run() emit error/finish,
+        // - qtap.runWaitFor() will throw/return.
+        for (const browser of this.browsers.values()) {
+          browser.stop(new util.BrowserStopSignal(String(e), {
+            cause: e
+          }));
+        }
       }
     });
 
@@ -144,8 +155,7 @@ class ControlServer {
         throw new Error('ControlServer.close must only be called once');
       }
       this.closeCalled = true;
-
-      this.logger.debug('http_close');
+      this.logger.debug('server_close');
       server.close();
       server.closeAllConnections();
     };

test/qtap.test.js

@@ -77,15 +77,6 @@ QUnit.module('qtap', function (hooks) {
       options,
       error: /Unknown browser unknown/,
     },
-    'file not found': {
-      files: 'notfound.html',
-      browsers: 'fake',
-      options: {
-        ...options,
-        config: 'test/fixtures/qtap.config.js'
-      },
-      error: new Error('Could not open notfound.html'),
-    },
     'config not found': {
       files: 'notfound.html',
       browsers: 'maybe',
@@ -333,6 +324,20 @@ QUnit.module('qtap', function (hooks) {
       ],
       exitCode: 0
     },
+    notfound: {
+      files: 'notfound.html',
+      browsers: 'fake',
+      options: {
+        ...options,
+        config: 'test/fixtures/qtap.config.js'
+      },
+      expected: [
+        'client: running notfound.html',
+        'online',
+        'bail: Error: Could not open notfound.html',
+      ],
+      exitCode: 1
+    },
     qunitPass: {
       files: 'test/fixtures/qunit-pass.html',
       options,