Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 966bb80

Browse files
committedJun 21, 2016
On upgrade failure, pass along connected error and reset connection serial
Resetting the message serial is a bit of a hack - fixes the problem for now but may change in the future depending on the conclusion of ably/realtime#587
1 parent 9c03e07 commit 966bb80

File tree

2 files changed

+58
-6
lines changed

2 files changed

+58
-6
lines changed
 

Diff for: ‎common/lib/transport/connectionmanager.js

+17-6
Original file line numberDiff line numberDiff line change
@@ -286,10 +286,10 @@ var ConnectionManager = (function() {
286286
/* if ws and xhrs are connecting in parallel, delay xhrs activation to let ws go ahead */
287287
if(transport.shortName !== optimalTransport && Utils.arrIn(self.getUpgradePossibilities(), optimalTransport)) {
288288
setTimeout(function() {
289-
self.scheduleTransportActivation(transport, connectionKey);
289+
self.scheduleTransportActivation(error, transport, connectionKey, connectionSerial, connectionId, clientId);
290290
}, self.options.timeouts.parallelUpgradeDelay);
291291
} else {
292-
self.scheduleTransportActivation(transport, connectionKey);
292+
self.scheduleTransportActivation(error, transport, connectionKey, connectionSerial, connectionId, clientId);
293293
}
294294
} else {
295295
self.activateTransport(error, transport, connectionKey, connectionSerial, connectionId, clientId);
@@ -324,7 +324,7 @@ var ConnectionManager = (function() {
324324
* @param transport, the transport instance
325325
* @param connectionKey
326326
*/
327-
ConnectionManager.prototype.scheduleTransportActivation = function(transport, connectionKey) {
327+
ConnectionManager.prototype.scheduleTransportActivation = function(error, transport, connectionKey, connectionSerial, connectionId, clientId) {
328328
if(this.state.state !== 'connected') {
329329
/* This is most likely to happen for the delayed xhrs, when xhrs and ws are scheduled in parallel*/
330330
Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.scheduleTransportActivation()', 'Current connection state (' + this.state.state + ') is not valid to upgrade in; abandoning upgrade');
@@ -355,17 +355,28 @@ var ConnectionManager = (function() {
355355
if(self.state === self.states.connected)
356356
self.state = self.states.synchronizing;
357357

358+
/* If the connectionId has changed, the upgrade hasn't worked. But as
359+
* it's still an upgrade, realtime still expects a sync - it just needs to
360+
* be a sync with the new connectionSerial (which will be -1). (And it
361+
* needs to be set in the library, which is done by activateTransport). */
362+
var connectionReset = connectionId !== self.connectionId,
363+
newConnectionSerial = connectionReset ? connectionSerial : self.connectionSerial;
364+
365+
if(connectionReset) {
366+
Logger.logAction(Logger.LOG_ERROR, 'ConnectionManager.scheduleTransportActivation()', 'Upgrade resulted in new connectionId; resetting library connectionSerial from ' + self.connectionSerial + ' to ' + newConnectionSerial + '; upgrade error was ' + error);
367+
}
368+
358369
/* make this the active transport */
359370
Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.scheduleTransportActivation()', 'Activating transport; transport = ' + transport);
360371
/* if activateTransport returns that it has not done anything (eg because the connection is closing), don't bother syncing */
361-
if(self.activateTransport(null, transport, connectionKey, self.connectionSerial, self.connectionId)) {
372+
if(self.activateTransport(error, transport, connectionKey, newConnectionSerial, connectionId, clientId)) {
362373
Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.scheduleTransportActivation()', 'Syncing transport; transport = ' + transport);
363-
self.sync(transport, function(err, connectionSerial, connectionId) {
374+
self.sync(transport, function(err, newConnectionSerial, connectionId) {
364375
if(err) {
365376
Logger.logAction(Logger.LOG_ERROR, 'ConnectionManager.scheduleTransportActivation()', 'Unexpected error attempting to sync transport; transport = ' + transport + '; err = ' + err);
366377
return;
367378
}
368-
Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.scheduleTransportActivation()', 'sync successful upgraded transport; transport = ' + transport + '; connectionSerial = ' + connectionSerial + '; connectionId = ' + connectionId);
379+
Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.scheduleTransportActivation()', 'sync successful on upgraded transport; transport = ' + transport + '; connectionSerial = ' + newConnectionSerial + '; connectionId = ' + connectionId);
369380

370381
Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.scheduleTransportActivation()', 'Sending queued messages on upgraded transport; transport = ' + transport);
371382
/* Restore pre-sync state. If state has changed in the meantime,

Diff for: ‎spec/realtime/upgrade.test.js

+41
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
define(['ably', 'shared_helper'], function(Ably, helper) {
44
var exports = {},
5+
_exports = {},
56
rest,
67
publishAtIntervals = function(numMessages, channel, dataFn, onPublish){
78
for(var i = numMessages; i > 0; i--) {
@@ -492,5 +493,45 @@ define(['ably', 'shared_helper'], function(Ably, helper) {
492493
}
493494
};
494495

496+
exports.unrecoverableUpgrade = function(test) {
497+
test.expect(6);
498+
var realtime,
499+
fakeConnectionKey = '_____!ablyjs_test_fake-key____',
500+
fakeConnectionId = 'ablyjs_tes';
501+
502+
try {
503+
/* on base transport active */
504+
realtime = helper.AblyRealtime();
505+
realtime.connection.connectionManager.once('transport.active', function(transport) {
506+
test.ok(transport.toString().indexOf('/comet/') > -1, 'assert first transport to become active is a comet transport');
507+
test.equal(realtime.connection.errorReason, null, 'Check connection.errorReason is initially null');
508+
/* sabotage the upgrade */
509+
realtime.connection.connectionManager.connectionKey = fakeConnectionKey;
510+
realtime.connection.connectionManager.connectionId = fakeConnectionId;
511+
512+
/* on upgrade failure */
513+
realtime.connection.once('error', function(error) {
514+
test.equal(error.code, 80008, 'Check correct (unrecoverable connection) error');
515+
test.equal(realtime.connection.errorReason.code, 80008, 'Check error set in connection.errorReason');
516+
test.equal(realtime.connection.state, 'connected', 'Check still connected');
517+
518+
/* Check events not still paused */
519+
var channel = realtime.channels.get('unrecoverableUpgrade');
520+
channel.attach(function(err) {
521+
if(err) { test.ok(false, 'Attach error ' + helper.displayError(err)); }
522+
channel.subscribe(function(msg) {
523+
test.ok(true, 'Successfully received message');
524+
closeAndFinish(test, realtime);
525+
});
526+
channel.publish('msg', null);
527+
});
528+
});
529+
});
530+
} catch(e) {
531+
test.ok(false, 'test failed with exception: ' + e.stack);
532+
closeAndFinish(test, realtime);
533+
}
534+
};
535+
495536
return module.exports = (bestTransport === 'web_socket') ? helper.withTimeout(exports) : {};
496537
});

0 commit comments

Comments
 (0)