fix: cancel in-flight full reconnects when the engine closes by horvatz · Pull Request #2095 · livekit/client-sdk-js

horvatz · 2026-09-11T14:07:09Z

Calling Room.disconnect() while a full reconnect is in progress can leave the reconnect coroutine running after the engine has closed. When paused cleanup or region selection finishes, it can call join with the saved credentials. A late signal join response can also set _isClosed back to false. By then engine close has removed its listeners, so applications cannot reliably catch the late attempt through room-state events.

This change gives each reconnect attempt an abort controller, aborts it synchronously on engine close, and passes its signal through full reconnect joins and region selection. Cancelled continuations stop without retrying, and late join responses cannot reopen the engine. SignalClient also checks cancellation after waiting for its connection lock/connection parameters and after previous-transport teardown, before creating another transport.

Reproduction and regression coverage

The original sequence is: start a full reconnect, pause peer-connection cleanup, await Room.disconnect(), then release cleanup. Before this change a late join is attempted; after it no join is attempted and the room/engine remain disconnected/closed.

Six new regression cases failed before the corresponding fixes and pass afterward:

close during reconnect cleanup;
close during region selection;
a signal join response settling after close;
an already-aborted signal before join;
cancellation while waiting for the signal connection lock;
cancellation during previous-transport teardown.

A successful full reconnect is covered as well. Tests exercise the actual engine reconnect/join and signal-client paths with network/peer-connection work stubbed; no live server is required. The original reproduction was also rerun through real Room.disconnect().

Validation

Full unit suite: 819 passed, 1 skipped (51 files).
Type check, ESLint, Prettier, throws check, build, browser compatibility check, and package validation commands pass. ESLint reports 13 existing warnings outside the changed files; compatibility and package validation also report warnings.
Includes a patch changeset.

changeset-bot · 2026-09-11T14:07:18Z

🦋 Changeset detected

Latest commit: 4caf3a3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
livekit-client	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

CLAassistant · 2026-09-11T14:07:41Z

All committers have signed the CLA.

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

1egoman

Thanks for the PR - this looks like a really promising new addition. Left a few small comments, other than those this looks good to me.

1egoman · 2026-09-14T17:04:39Z

        }
+        if (alreadyAborted) return;



question: Are there other places further down in this method where something like this could be needed as well? What happens if an abort gets triggered midway through the signaling websocket connection process?

hopefully that would be handled via the abort event listener?

What I was referring to is that this is a big long synchronous function, and there's not many other instances of this alreadyAborted check in there. So the syncronous code could still run after an abort. The event listener would only run once that synchronous function finishes.

I've added 467d566 which I think makes it a little better.

1egoman · 2026-09-14T17:07:31Z

   * having given up on reconnecting.
   */
  async close(reason?: string) {
+    this.reconnectAbortController?.abort();


After aborting this here, I think it might be a good idea to reset this.reconnectAbortController back to its initial undefined value (like you are doing in the finally in attemptReconnect). While in practice I think this isn't critical for the change today, it means that any future code will be able to use the existence of this.reconnectAbortController as a meaningful signal.

Done in 62bd2bb

1egoman · 2026-09-14T17:11:34Z

      this.clearPendingReconnect();
      succeeded = true;
    } catch (e) {
+      if (abortController.signal.aborted) return;


For all of these types of bail outs throughout the change, I wonder if it would be a good idea to add some sort of log for each one so that a user debugging a connection failure would be able to tell a) there was a purposeful bailout midway through, and b) which one bailout was taken. Thoughts on this?

Done in fa91811

1egoman · 2026-09-14T17:16:12Z

Ah @horvatz would you be able to click the button in this comment and sign the CLA? That is a prerequisite before this could be merged.

lukasIO · 2026-10-14T13:31:36Z

@1egoman let's address your outstanding remarks and then merge this?

close() now clears `reconnectAbortController` after aborting it, so the field's presence reliably means a full reconnect is in flight. The attempt's own finally only clears the field when it still holds that attempt's controller, so it can't clobber a newer attempt's. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

The abort listener rejects the join and closes the transport, but the connect continuation kept running once its pending await settled: a failed upgrade still probed the validate endpoint, and a join that raced the abort still fired `onJoined`, publishing the abandoned session's participants to the room. Bail out after each of those awaits, and log each early exit so a cancelled connect is visible when debugging. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Every early exit taken because the engine closed mid-attempt now logs which stage it bailed out at, so a connection failure can be traced to a deliberate cancellation rather than looking like a silent stop. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Since an abandoned connect skips the validate request, the hanging- validate regression test from livekit#2148 no longer consumes its queued fetch mocks, which leaked into the handleConnectionError tests. Assert that no validation happens and reset the mocks afterwards. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

1egoman · 2026-10-14T23:18:51Z

@lukasIO Can you take a look at this again before merging? I had to alter a test you introduced in #2148 as part of rebasing this on top of origin/main and am not 100% sure I got it right. A LLM seems to think it's ok but I'd like to get a confirmation from you as well before merging.

lukasIO

yeah, the test change makes sense.
thanks!

horvatz requested review from 1egoman, lukasIO and xianshijing-lk as code owners September 11, 2026 14:07

devin-ai-integration Bot reviewed Sep 11, 2026

View reviewed changes

1egoman reviewed Sep 14, 2026

View reviewed changes

horvatz and others added 6 commits October 15, 2026 12:10

fix: cancel in-flight full reconnects when the engine closes

9f79880

refactor: inline the reconnect cancellation checks

609885a

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

1egoman force-pushed the fix/cancel-in-flight-reconnect branch from 016e3da to 4caf3a3 Compare October 14, 2026 23:14

lukasIO approved these changes Oct 15, 2026

View reviewed changes

Conversation

horvatz commented Sep 11, 2026

Reproduction and regression coverage

Validation

Uh oh!

changeset-bot Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

CLAassistant commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

1egoman left a comment

Choose a reason for hiding this comment

Uh oh!

1egoman Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

lukasIO Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

1egoman Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1egoman Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

1egoman Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

1egoman Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

1egoman Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

1egoman commented Sep 14, 2026

Uh oh!

lukasIO commented Oct 14, 2026

Uh oh!

1egoman commented Oct 14, 2026

Uh oh!

lukasIO left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

changeset-bot Bot commented Sep 11, 2026 •

edited

Loading

CLAassistant commented Sep 11, 2026 •

edited

Loading

1egoman Oct 14, 2026 •

edited

Loading