harden browser contract-test adapter and entity against client leaks/hangs

kinyoklion · kinyoklion · commit ebc7c5d8b6eb · 2026-05-29T09:12:01.000-07:00
When the test service (adapter + browser entity) is reused across many
harness runs, two issues could wedge it permanently:

- The adapter's send() waited forever for the browser to answer a
  command. A single unanswered command hung that REST request -- and
  therefore the harness -- indefinitely (status queries returned 000),
  with no recovery. Add a timeout (ADAPTER_REQUEST_TIMEOUT_MS, default
  30s) so a stalled command fails that one request with HTTP 500 instead
  of wedging everything; reject in-flight commands on WS close; and log
  REST listen errors so a silent bind failure is visible.

- The final test's client is never deleted by the harness before it
  exits, so it lingered and kept reconnecting (404 -&gt; retry) across runs
  when the browser is reused. On getCapabilities (sent at the start of
  each run) the entity now destroys any leftover clients, so nothing
  accumulates across runs.

Verified: interrupting a run leaves a retrying client that the next
run's getCapabilities destroys (retries stop); 5 consecutive suites
against one reused browser/adapter stay green with zero leaked clients.
diff --git a/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts b/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts
@@ -33,6 +33,13 @@ export default class TestHarnessWebSocket {
       const resData: any = { reqId: data.reqId };
       switch (data.command) {
         case 'getCapabilities':
+          // A capabilities query marks the start of a test-harness run. Destroy
+          // any entities left over from a previous run -- e.g. a run that was
+          // interrupted before its final deleteClient, or the final test's
+          // client which the harness never deletes -- so SDK clients do not
+          // accumulate and keep reconnecting across runs when the browser is
+          // reused.
+          this._destroyAllEntities();
           resData.capabilities = [
             'client-side',
             'service-endpoints',
@@ -90,6 +97,22 @@ export default class TestHarnessWebSocket {
     this._ws?.close();
   }
 
+  private _destroyAllEntities() {
+    const ids = Object.keys(this._entities);
+    if (ids.length === 0) {
+      return;
+    }
+    this._logger.info(`Destroying ${ids.length} leftover client(s) before new run.`);
+    ids.forEach((id) => {
+      try {
+        this._entities[id].close();
+      } catch (err) {
+        this._logger.warn(`Error closing leftover client ${id}: ${err}`);
+      }
+      delete this._entities[id];
+    });
+  }
+
   send(data: unknown) {
     this._ws?.send(JSON.stringify(data));
   }
diff --git a/packages/tooling/contract-test-utils/src/adapter/startAdapter.ts b/packages/tooling/contract-test-utils/src/adapter/startAdapter.ts
@@ -10,42 +10,92 @@ import { WebSocketServer } from 'ws';
 export interface AdapterOptions {
   restPort?: number;
   wsPort?: number;
+  /**
+   * How long (ms) the adapter waits for the browser entity to answer a command
+   * before failing the REST request. Prevents a single unanswered command from
+   * wedging the adapter (and therefore the test harness) indefinitely.
+   * Override with ADAPTER_REQUEST_TIMEOUT_MS. Defaults to 30s.
+   */
+  requestTimeoutMs?: number;
+}
+
+/**
+ * Tracks an outstanding command sent to the browser entity, awaiting its
+ * response over the WebSocket.
+ */
+interface Waiter {
+  resolve: (data: any) => void;
+  reject: (err: Error) => void;
+  timer: ReturnType<typeof setTimeout>;
 }
 
 let server: http.Server | undefined;
 
 export function startAdapter(options?: AdapterOptions) {
   const restPort = options?.restPort ?? 8000;
   const wsPort = options?.wsPort ?? 8001;
+  const requestTimeoutMs =
+    options?.requestTimeoutMs ?? Number(process.env.ADAPTER_REQUEST_TIMEOUT_MS ?? 30000);
 
   const wss = new WebSocketServer({ port: wsPort });
-  const waiters: Record<string, (data: unknown) => void> = {};
+  const waiters: Record<string, Waiter> = {};
 
   console.log('Running contract test harness adapter.');
   wss.on('connection', async (ws) => {
     ws.on('error', console.error);
 
     ws.on('message', (stringData: string) => {
       const data = JSON.parse(stringData);
-      if (Object.prototype.hasOwnProperty.call(waiters, data.reqId)) {
-        waiters[data.reqId](data);
+      const waiter = waiters[data.reqId];
+      if (waiter) {
+        clearTimeout(waiter.timer);
         delete waiters[data.reqId];
+        waiter.resolve(data);
       } else {
         console.error('Did not find outstanding request', data.reqId);
       }
     });
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    const send = (data: { [key: string]: unknown; reqId: string }): Promise<any> => {
-      let resolver: (data: unknown) => void;
-      const waiter = new Promise((resolve) => {
-        resolver = resolve;
+    // When the entity's socket drops, fail every outstanding command instead
+    // of leaving REST requests (and the harness) hanging forever.
+    ws.on('close', () => {
+      Object.keys(waiters).forEach((reqId) => {
+        const waiter = waiters[reqId];
+        clearTimeout(waiter.timer);
+        delete waiters[reqId];
+        waiter.reject(new Error('WebSocket connection to entity closed before a response was received.'));
       });
-      // @ts-expect-error The body of the above assignment runs sequentially.
-      waiters[data.reqId] = resolver;
-      ws.send(JSON.stringify(data));
-      return waiter;
-    };
+    });
+
+    const send = (data: { [key: string]: unknown; reqId: string }): Promise<any> =>
+      new Promise((resolve, reject) => {
+        const timer = setTimeout(() => {
+          delete waiters[data.reqId];
+          reject(
+            new Error(
+              `Adapter timed out after ${requestTimeoutMs}ms awaiting entity response to '${data.command}' (reqId ${data.reqId}).`,
+            ),
+          );
+        }, requestTimeoutMs);
+        waiters[data.reqId] = { resolve, reject, timer };
+        ws.send(JSON.stringify(data));
+      });
+
+    // Wrap async route handlers so an unanswered/timed-out command produces a
+    // 500 for that single request rather than an unhandled rejection that
+    // leaves the request — and the harness — hanging.
+    const guard =
+      (
+        handler: (req: express.Request, res: express.Response) => Promise<void>,
+      ): express.RequestHandler =>
+      (req, res) => {
+        handler(req, res).catch((err) => {
+          console.error('Adapter request failed:', err);
+          if (!res.headersSent) {
+            res.status(500).send(String(err));
+          }
+        });
+      };
 
     if (server) {
       await util.promisify(server.close).call(server);
@@ -63,54 +113,71 @@ export function startAdapter(options?: AdapterOptions) {
     );
     app.use(bodyParser.json());
 
-    app.get('/', async (_req, res) => {
-      const commandResult = await send({ command: 'getCapabilities', reqId: randomUUID() });
-      res.header('Content-Type', 'application/json');
-      res.json(commandResult);
-    });
+    app.get(
+      '/',
+      guard(async (_req, res) => {
+        const commandResult = await send({ command: 'getCapabilities', reqId: randomUUID() });
+        res.header('Content-Type', 'application/json');
+        res.json(commandResult);
+      }),
+    );
 
     app.delete('/', () => {
       process.exit();
     });
 
-    app.post('/', async (req, res) => {
-      const commandResult = await send({
-        command: 'createClient',
-        body: req.body,
-        reqId: randomUUID(),
-      });
-      if (commandResult.resourceUrl) {
-        res.set('Location', commandResult.resourceUrl);
-      }
-      if (commandResult.status) {
-        res.status(commandResult.status);
-      }
-      res.send();
-    });
+    app.post(
+      '/',
+      guard(async (req, res) => {
+        const commandResult = await send({
+          command: 'createClient',
+          body: req.body,
+          reqId: randomUUID(),
+        });
+        if (commandResult.resourceUrl) {
+          res.set('Location', commandResult.resourceUrl);
+        }
+        if (commandResult.status) {
+          res.status(commandResult.status);
+        }
+        res.send();
+      }),
+    );
 
-    app.post('/clients/:id', async (req, res) => {
-      const commandResult = await send({
-        command: 'runCommand',
-        id: req.params.id,
-        body: req.body,
-        reqId: randomUUID(),
-      });
-      if (commandResult.status) {
-        res.status(commandResult.status);
-      }
-      if (commandResult.body) {
-        res.write(JSON.stringify(commandResult.body));
-      }
-      res.send();
-    });
+    app.post(
+      '/clients/:id',
+      guard(async (req, res) => {
+        const commandResult = await send({
+          command: 'runCommand',
+          id: req.params.id,
+          body: req.body,
+          reqId: randomUUID(),
+        });
+        if (commandResult.status) {
+          res.status(commandResult.status);
+        }
+        if (commandResult.body) {
+          res.write(JSON.stringify(commandResult.body));
+        }
+        res.send();
+      }),
+    );
 
-    app.delete('/clients/:id', async (req, res) => {
-      await send({ command: 'deleteClient', id: req.params.id, reqId: randomUUID() });
-      res.send();
-    });
+    app.delete(
+      '/clients/:id',
+      guard(async (req, res) => {
+        await send({ command: 'deleteClient', id: req.params.id, reqId: randomUUID() });
+        res.send();
+      }),
+    );
 
     server = app.listen(restPort, () => {
       console.log('Listening on port %d', restPort);
     });
+    // Surface bind failures (e.g. EADDRINUSE) instead of silently never
+    // logging "Listening on port".
+    server.on('error', (err) => {
+      console.error(`REST server error on port ${restPort}:`, err);
+    });
   });
 }
