Skip to content

Commit

Permalink
ConnectionManager: Fix uprade bug that could lead to an indefinitely …
Browse files Browse the repository at this point in the history
…sync-pending transport

If we start an upgrade, but then the previous active transport
disconnects by the time the upgrade transport connects, we just want to
activate it immediately. But the previous logic was sending you down the
activateTransport path in that case, and the problem with that is that
the library only sends a sync() message to the server in the
scheduleTransportActivation path. So the server is left waiting for a
sync that will never come, and stays in upgrade-pending limbo.

Fixed by just using the scheduleTransportActivation path, which can cope
fine with the lack of an active protocol (though made a small tweak to
that logic too)
  • Loading branch information
SimonWoolf committed Aug 8, 2022
1 parent 3ab05e2 commit 9cec75d
Showing 1 changed file with 13 additions and 4 deletions.
17 changes: 13 additions & 4 deletions src/common/lib/transport/connectionmanager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,11 +582,12 @@ class ConnectionManager extends EventEmitter {
connectionDetails: Record<string, any>,
connectionPosition: ConnectionManager
) => {
if (mode == 'upgrade' && this.activeProtocol) {
if (mode == 'upgrade') {
/* if ws and xhrs are connecting in parallel, delay xhrs activation to let ws go ahead */
if (
transport.shortName !== optimalTransport &&
Utils.arrIn(this.getUpgradePossibilities(), optimalTransport)
Utils.arrIn(this.getUpgradePossibilities(), optimalTransport) &&
this.activeProtocol
) {
setTimeout(() => {
this.scheduleTransportActivation(error, transport, connectionId, connectionDetails, connectionPosition);
Expand Down Expand Up @@ -678,7 +679,7 @@ class ConnectionManager extends EventEmitter {
'Scheduling transport upgrade; transport = ' + transport
);

this.realtime.channels.onceNopending((err: ErrorInfo) => {
const onReadyToUpgrade = (err?: ErrorInfo) => {
let oldProtocol: Protocol | null;
if (err) {
Logger.logAction(
Expand Down Expand Up @@ -822,7 +823,15 @@ class ConnectionManager extends EventEmitter {
}
}
);
});
};

// No point waiting for pending attaches if there's no active transport, just sync and
// activate the new one immediately, attaches will be retried on the new one
if (currentTransport) {
this.realtime.channels.onceNopending(onReadyToUpgrade);
} else {
onReadyToUpgrade();
}
}

/**
Expand Down

0 comments on commit 9cec75d

Please sign in to comment.