fix(lib): reconnect when the first WebSocket attempt is refused

browser/lib/src/websockets.ts

@@ -475,25 +475,78 @@ export class WSClient {
       ws.addEventListener('message', event => {
         if (!signal.aborted) this.handleMessage(event);
       });
+      let ended = false;
+      let rejectOpen: (error: AtomicError) => void = () => undefined;
+
+      // Tears this socket down and schedules the next attempt. Runs at most
+      // once per socket: normally from `close`, but also from `error` when the
+      // socket never opened, because Node's WebSocket (undici) fires only
+      // `error` on a refused connection, never `close`. Waiting for `close`
+      // there left the client without a retry and `openPromise` pending, so a
+      // client whose first attempt hit a server that was not up yet stayed
+      // offline for good.
+      const end = (error: string | undefined) => {
+        if (ended) return;
+        ended = true;
+        // Explicit close already tears down this client synchronously. Its
+        // later event must not overwrite a replacement socket's live state.
+        if (this._closed || ws !== this.ws) return;
+        this.connection.abort(new RequestCancelledError('WebSocket closed'));
+
+        if (!opened) {
+          // A socket that dies before opening must REJECT `openPromise`, not
+          // leave it pending forever. `authenticate()` awaits it while
+          // holding `isAuthenticating`, and its `finally` (the only thing
+          // that clears that flag) is downstream of this await. A
+          // never-settling promise therefore pins the flag for the lifetime
+          // of the client: the retry loop reconnects, the new socket opens,
+          // and its `authenticate()` takes the `if (this.isAuthenticating)
+          // await this.authPromise` branch onto the dead promise and waits
+          // forever. Auth never completes, so `reportConnected(true)` never
+          // fires and the app sits on "Offline" next to a server answering
+          // in milliseconds. Reproduced on a cold start where the webview is
+          // ready before the embedded server binds.
+          rejectOpen(
+            new AtomicError(
+              `WebSocket to ${wsURL.origin} closed before it opened`,
+              ErrorType.Server,
+            ),
+          );
+        }
+
+        this.reportConnected(false, error);
+        this.rejectAllPending('WebSocket closed before response arrived');
+        this.stopLiveness();
+        this._serverCaps = [];
+        this._challengeNonce = undefined;
+        this._subscribedDrive = undefined;
+
+        this._retryTimer = setTimeout(() => {
+          this.authenticatedWith = undefined;
+          createSocket();
+        }, this._retryDelay);
+        this._retryDelay = Math.min(this._retryDelay * 2, 30000);
+      };
+
       ws.addEventListener('error', () => {
         if (this._closed || ws !== this.ws) return;
-        this.connection.abort(new RequestCancelledError('WebSocket error'));
 
         if (!opened) {
           console.warn('[WS] Connection failed');
+          end(connectionFailedMessage(wsURL));
+
+          return;
         }
 
+        this.connection.abort(new RequestCancelledError('WebSocket error'));
         this.reportConnected(false, connectionFailedMessage(wsURL));
         // Some environments fire error without an immediately-following
         // close. Reject anyway — if close does fire later, the second
         // rejectAllPending sees an empty Map and is a no-op.
         this.rejectAllPending('WebSocket error before response arrived');
       });
       ws.addEventListener('close', (ev: CloseEvent) => {
-        // Explicit close already tears down this client synchronously. Its
-        // later event must not overwrite a replacement socket's live state.
         if (this._closed || ws !== this.ws) return;
-        this.connection.abort(new RequestCancelledError('WebSocket closed'));
 
         // Surface CloseEvent metadata so an unexplained reconnect loop
         // names its own cause: code 1000=normal, 1001=going away,
@@ -503,59 +556,20 @@ export class WSClient {
         // tells us if the closing handshake completed. Without this the
         // user-visible "Connection ... closed" line gives no signal of
         // what actually killed the connection.
-        if (!this._closed) {
+        if (!ended) {
           console.warn(
             `[WS] close code=${ev.code} reason=${JSON.stringify(ev.reason)} wasClean=${ev.wasClean} opened=${opened}`,
           );
         }
 
-        const error = this._closed
-          ? undefined
-          : opened
+        end(
+          opened
             ? `Connection to ${wsURL.origin} closed (code=${ev.code}${ev.reason ? `, reason=${ev.reason}` : ''}).`
-            : connectionFailedMessage(wsURL);
-
-        this.reportConnected(false, error);
-        this.rejectAllPending('WebSocket closed before response arrived');
-        this.stopLiveness();
-        this._serverCaps = [];
-        this._challengeNonce = undefined;
-        this._subscribedDrive = undefined;
-
-        if (!this._closed) {
-          this._retryTimer = setTimeout(() => {
-            this.authenticatedWith = undefined;
-            createSocket();
-          }, this._retryDelay);
-          this._retryDelay = Math.min(this._retryDelay * 2, 30000);
-        }
+            : connectionFailedMessage(wsURL),
+        );
       });
       this.openPromise = new Promise((resolve, reject) => {
-        // A socket that dies before opening must REJECT this, not leave it
-        // pending forever. `authenticate()` awaits it while holding
-        // `isAuthenticating`, and its `finally` — the only thing that clears
-        // that flag — is downstream of this await. A never-settling promise
-        // therefore pins the flag for the lifetime of the client: the retry
-        // loop reconnects, the new socket opens, and its `authenticate()`
-        // takes the `if (this.isAuthenticating) await this.authPromise` branch
-        // onto the dead promise and waits forever. Auth never completes, so
-        // `reportConnected(true)` never fires and the app sits on "Offline"
-        // next to a server answering in milliseconds, with every `ws.fetch`
-        // hung because `REQUEST_TIMEOUT` only starts after auth.
-        //
-        // Reproduced on a cold start where the webview is ready before the
-        // embedded server binds: one `close code=1006 opened=false`, and no
-        // recovery until a reload builds a fresh client.
-        ws.addEventListener('close', () => {
-          if (!opened) {
-            reject(
-              new AtomicError(
-                `WebSocket to ${wsURL.origin} closed before it opened`,
-                ErrorType.Server,
-              ),
-            );
-          }
-        });
+        rejectOpen = reject;
         ws.addEventListener('open', () => {
           opened = true;
           this._retryDelay = 1000;

browser/lib/tests/ws-reconnect.integration.test.ts

@@ -0,0 +1,67 @@
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import net from 'node:net';
+import { setTimeout as delay } from 'node:timers/promises';
+import { startServer, type ServerHandle } from './server-fixture.js';
+import { Agent } from '../src/agent.js';
+import { Store } from '../src/store.js';
+
+/** A port nothing listens on yet: bind to 0, read the port, release it. */
+async function freePort(): Promise<number> {
+  const probe = net.createServer();
+  await new Promise<void>(resolve => probe.listen(0, '127.0.0.1', resolve));
+  const { port } = probe.address() as net.AddressInfo;
+  await new Promise(resolve => probe.close(resolve));
+
+  return port;
+}
+
+// Node's WebSocket fires only `error` on a refused connection, never `close`.
+// The client used to schedule its retry on `close` alone, so a Store whose
+// first attempt came before the server was up never connected at all
+// (develop runs 5208 and 5211: upload-roundtrip logged "[WS] Connection
+// failed" and then waited out its timeout).
+describe('WebSocket reconnect', () => {
+  let server: ServerHandle;
+
+  beforeAll(async () => {
+    server = await startServer();
+  }, 120_000);
+
+  afterAll(async () => {
+    await server?.stop();
+  });
+
+  it('connects once the server comes up after a refused first attempt', async () => {
+    const target = new URL(server.serverUrl);
+    const port = await freePort();
+    const agent = await Agent.fromSecret(server.agentSecret);
+    const store = new Store({ serverUrl: `http://localhost:${port}`, agent });
+
+    // Let the first attempt be refused, then put the server behind that port.
+    await delay(1500);
+    const proxy = net.createServer(socket => {
+      const upstream = net.connect(Number(target.port), '127.0.0.1');
+      socket.pipe(upstream);
+      upstream.pipe(socket);
+      socket.on('error', () => upstream.destroy());
+      upstream.on('error', () => socket.destroy());
+    });
+    await new Promise<void>(resolve =>
+      proxy.listen(port, '127.0.0.1', () => resolve()),
+    );
+
+    try {
+      const start = Date.now();
+
+      // The backoff after one failure is 2 s; allow a few more doublings.
+      while (!store.serverConnected && Date.now() - start < 20_000) {
+        await delay(100);
+      }
+
+      expect(store.serverConnected).toBe(true);
+    } finally {
+      store.disconnect();
+      proxy.close();
+    }
+  }, 60_000);
+});