From 03469a043cd2dae7fb892e6560586533267e081a Mon Sep 17 00:00:00 2001 From: Simon Woolf Date: Fri, 5 Aug 2022 17:28:49 +0100 Subject: [PATCH] ConnectionManager: Fix uprade bug that could lead to an indefinitely 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) --- src/common/lib/transport/connectionmanager.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/common/lib/transport/connectionmanager.ts b/src/common/lib/transport/connectionmanager.ts index 549d49a3d2..59cbc82862 100644 --- a/src/common/lib/transport/connectionmanager.ts +++ b/src/common/lib/transport/connectionmanager.ts @@ -582,11 +582,12 @@ class ConnectionManager extends EventEmitter { connectionDetails: Record, 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); @@ -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( @@ -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(); + } } /**