Merge pull request #4230 from element-hq/matthew/connection-stop-during-connect

Don't report an aborted connect as a connection error

src/state/CallViewModel/remoteMembers/Connection.test.ts

@@ -394,6 +394,34 @@ describe("Start connection states", () => {
     expect(connectedState).toEqual(ConnectionState.LivekitConnected);
   });
 
+  it("stopping while connecting does not report an error", async () => {
+    setupTest();
+    const connection = setupRemoteConnection();
+
+    const capturedStates: (ConnectionState | Error)[] = [];
+    const s = connection.state$.subscribe((value) => {
+      capturedStates.push(value);
+    });
+    onTestFinished(() => s.unsubscribe());
+
+    // livekit-client rejects a pending connect() when disconnect() is called.
+    const pendingConnect = Promise.withResolvers<void>();
+    fakeLivekitRoom.connect.mockReturnValue(pendingConnect.promise);
+    fakeLivekitRoom.disconnect.mockResolvedValue(undefined);
+
+    const started = connection.start();
+    await vi.waitFor(() => expect(fakeLivekitRoom.connect).toHaveBeenCalled());
+    const stopping = connection.stop();
+    pendingConnect.reject(new Error("Client initiated disconnect"));
+    await stopping;
+
+    // start() resolves rather than rejecting (it is not awaited by the
+    // ConnectionManager, so a rejection would be unhandled).
+    await expect(started).resolves.toBeUndefined();
+    expect(capturedStates.at(-1)).toEqual(ConnectionState.Stopped);
+    expect(capturedStates.some((st) => st instanceof Error)).toBe(false);
+  });
+
   it("shutting down the scope should stop the connection", async () => {
     setupTest();
     vi.useFakeTimers();

src/state/CallViewModel/remoteMembers/Connection.ts

@@ -373,6 +373,15 @@ export class Connection {
       // If we were stopped while connecting, don't proceed to update state.
       if (this.stopped) return;
     } catch (error) {
+      if (this.stopped) {
+        // stop() was called while we were connecting, which makes the pending
+        // connect reject. That is the abort we asked for, not a failure, so
+        // don't record an error state on a stopped connection or rethrow it
+        // (start() is not awaited by the ConnectionManager, so a throw here
+        // becomes an unhandled promise rejection).
+        this.logger.debug(`Connect aborted because the connection was stopped`);
+        return;
+      }
       this.logger.debug(`Failed to connect to LiveKit room: ${error}`);
       this._state$.next(
         error instanceof ElementCallError
@@ -411,9 +420,11 @@ export class Connection {
       `stop: disconnecing from lk room ${this.transport.livekit_service_url}`,
     );
     if (this.stopped) return;
+    // Mark as stopped before disconnecting so that a connect() aborted by the
+    // disconnect sees the flag and does not report the abort as an error.
+    this.stopped = true;
     await this.livekitRoom.disconnect();
     this._state$.next(ConnectionState.Stopped);
-    this.stopped = true;
     this.logger.debug(
       `stop: DONE disconnecing from lk room ${this.transport.livekit_service_url}`,
     );