fix(dashboard): don't evict live registration on a transient probe

packages/playwright-core/src/serverRegistry.ts

@@ -220,25 +220,44 @@ class ServerRegistry extends EventEmitter {
   }
 }
 
+// A single connection probe is not authoritative: under load a live endpoint can
+// transiently refuse or reset a connection (listen backlog exhaustion, the daemon
+// mid-cold-start, etc). Since `list()` unlinks descriptors that fail to connect,
+// one such blip would permanently evict a healthy registration. Retry a few times
+// with a short per-attempt timeout before declaring the endpoint dead — a truly
+// gone endpoint keeps failing fast, while a busy one recovers.
+const kConnectProbeAttempts = 5;
+const kConnectProbeTimeout = 1000;
+const kConnectProbeInterval = 100;
+
 async function canConnectTo(descriptor: BrowserDescriptor): Promise<boolean> {
-  if (!descriptor.endpoint)
+  const endpoint = descriptor.endpoint ?? (descriptor as any).pipeName;
+  if (!endpoint)
     return false;
-  if (descriptor.endpoint.startsWith('ws://') || descriptor.endpoint.startsWith('wss://')) {
-    return await new Promise(resolve => {
-      const url = new URL(descriptor.endpoint!);
-      const socket = net.createConnection(Number(url.port), url.hostname, () => {
-        socket.destroy();
-        resolve(true);
-      });
-      socket.on('error', () => resolve(false));
-    });
+  for (let attempt = 0; attempt < kConnectProbeAttempts; ++attempt) {
+    if (await attemptConnectTo(endpoint))
+      return true;
+    if (attempt < kConnectProbeAttempts - 1)
+      await new Promise(resolve => setTimeout(resolve, kConnectProbeInterval));
   }
-  return await new Promise(resolve => {
-    const socket = net.createConnection(descriptor.endpoint ?? (descriptor as any).pipeName, () => {
+  return false;
+}
+
+function attemptConnectTo(endpoint: string): Promise<boolean> {
+  return new Promise(resolve => {
+    let socket: net.Socket;
+    const done = (result: boolean) => {
       socket.destroy();
-      resolve(true);
-    });
-    socket.on('error', () => resolve(false));
+      resolve(result);
+    };
+    if (endpoint.startsWith('ws://') || endpoint.startsWith('wss://')) {
+      const url = new URL(endpoint);
+      socket = net.createConnection(Number(url.port), url.hostname, () => done(true));
+    } else {
+      socket = net.createConnection(endpoint, () => done(true));
+    }
+    socket.setTimeout(kConnectProbeTimeout, () => done(false));
+    socket.on('error', () => done(false));
   });
 }
 

tests/library/unit/server-registry.spec.ts

@@ -0,0 +1,93 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import fs from 'fs';
+import net from 'net';
+import os from 'os';
+import path from 'path';
+
+import { test as it, expect } from '@playwright/test';
+import { serverRegistry } from '../../../packages/playwright-core/lib/serverRegistry';
+
+async function freePort(): Promise<number> {
+  const server = net.createServer();
+  await new Promise<void>(resolve => server.listen(0, '127.0.0.1', resolve));
+  const port = (server.address() as net.AddressInfo).port;
+  await new Promise<void>(resolve => server.close(() => resolve()));
+  return port;
+}
+
+function writeDescriptor(dir: string, guid: string, endpoint: string) {
+  fs.mkdirSync(dir, { recursive: true });
+  fs.writeFileSync(path.join(dir, guid), JSON.stringify({
+    playwrightVersion: '1.0.0',
+    playwrightLib: '',
+    title: guid,
+    browser: { guid, browserName: 'chromium', launchOptions: {} },
+    endpoint,
+  }));
+}
+
+it.describe('serverRegistry health check', () => {
+  let registryDir: string;
+  let servers: net.Server[];
+
+  it.beforeEach(() => {
+    registryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pw-registry-'));
+    process.env.PWTEST_SERVER_REGISTRY = registryDir;
+    servers = [];
+  });
+
+  it.afterEach(async () => {
+    delete process.env.PWTEST_SERVER_REGISTRY;
+    for (const server of servers)
+      await new Promise<void>(resolve => server.close(() => resolve()));
+    fs.rmSync(registryDir, { recursive: true, force: true });
+  });
+
+  it('should keep a registration whose first connection probe fails but recovers', async () => {
+    const guid = 'guid-recovers';
+    const port = await freePort();
+    // Endpoint is initially unreachable (port refused), so the first probe(s) fail.
+    writeDescriptor(registryDir, guid, `ws://127.0.0.1:${port}`);
+
+    // Bring the endpoint up shortly after, mid-way through the probe retries.
+    const listening = new Promise<void>(resolve => {
+      setTimeout(() => {
+        const server = net.createServer(socket => socket.destroy());
+        servers.push(server);
+        server.listen(port, '127.0.0.1', resolve);
+      }, 300);
+    });
+
+    const [result] = await Promise.all([serverRegistry.list(), listening]);
+    const descriptors = [...result.values()].flat();
+    expect(descriptors.map(d => d.browser.guid)).toContain(guid);
+    // A transient probe failure must not evict a live registration from disk.
+    expect(fs.existsSync(path.join(registryDir, guid))).toBe(true);
+  });
+
+  it('should evict a registration whose endpoint never becomes reachable', async () => {
+    const guid = 'guid-dead';
+    const port = await freePort();
+    writeDescriptor(registryDir, guid, `ws://127.0.0.1:${port}`);
+
+    const result = await serverRegistry.list();
+    const descriptors = [...result.values()].flat();
+    expect(descriptors.map(d => d.browser.guid)).not.toContain(guid);
+    expect(fs.existsSync(path.join(registryDir, guid))).toBe(false);
+  });
+});