From 62e83ec2bc6036ef88b48e9ccfec6467ece33369 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 29 Jan 2013 13:57:57 -0800 Subject: [PATCH] When a connection fails permanently due to version number skew, show it in the reactive status object and don't try to reconnect. Note that this only happens if the server rejects the connection due to an unfixable version mismatch: if the server is unreachable or not speaking versioned DDP, the client will continue to retry. --- docs/client/api.html | 9 ++- packages/livedata/livedata_connection.js | 8 +-- .../livedata/livedata_connection_tests.js | 9 ++- packages/stream/stream_client.js | 63 +++++++++++-------- 4 files changed, 54 insertions(+), 35 deletions(-) diff --git a/docs/client/api.html b/docs/client/api.html index a28563bf19..90cd244316 100644 --- a/docs/client/api.html +++ b/docs/client/api.html @@ -359,8 +359,9 @@ the server. The return value is an object with the following fields: Describes the current reconnection status. The possible values are `connected` (the connection is up and running), `connecting` (disconnected and trying to open a - new connection), and `waiting` (failed to connect and - waiting to try to reconnect). + new connection), `failed` (permanently failed to connect; e.g., the client + and server support different versions of DDP) and `waiting` (failed + to connect and waiting to try to reconnect). {{/dtdd}} {{#dtdd name="retryCount" type="Number"}} @@ -374,6 +375,10 @@ the server. The return value is an object with the following fields: `retryTime - (new Date()).getTime()`. This key will be set only when `status` is `waiting`. {{/dtdd}} + +{{#dtdd name="reason" type="String or undefined"}} + If `status` is `failed`, a description of why the connection failed. +{{/dtdd}} Instead of using callbacks to notify you on changes, this is diff --git a/packages/livedata/livedata_connection.js b/packages/livedata/livedata_connection.js index f0203542aa..6dc9191f4c 100644 --- a/packages/livedata/livedata_connection.js +++ b/packages/livedata/livedata_connection.js @@ -197,10 +197,10 @@ Meteor._LivedataConnection = function (url, options) { self._versionSuggestion = msg.version; self._stream.reconnect({_force: true}); } else { - // XXX We should probably plumb this through to Meteor.connect and add - // an error callback there. - options.onConnectionFailure("Version negotiation failed; server requested version " + msg.version); - self._stream.forceDisconnect(true); + var error = + "Version negotiation failed; server requested version " + msg.version; + self._stream.forceDisconnect(error); + options.onConnectionFailure(error); } } else if (_.include(['added', 'changed', 'removed', 'complete', 'updated'], msg.msg)) diff --git a/packages/livedata/livedata_connection_tests.js b/packages/livedata/livedata_connection_tests.js index 1a2e2f38aa..23ce32d80f 100644 --- a/packages/livedata/livedata_connection_tests.js +++ b/packages/livedata/livedata_connection_tests.js @@ -1214,7 +1214,7 @@ Tinytest.addAsync("livedata connection - version negotiation requires renegotiat onConnectionFailure: function () { test.fail(); onComplete(); }, onConnected: function () { test.equal(connection._version, Meteor._SUPPORTED_DDP_VERSIONS[0]); - connection._stream.forceDisconnect(true); + connection._stream.forceDisconnect(); onComplete(); } }); @@ -1225,7 +1225,12 @@ Tinytest.addAsync("livedata connection - version negotiation fails", var connection = new Meteor._LivedataConnection("/", { reloadWithOutstanding: true, supportedDDPVersions: ["garbled", "more garbled"], - onConnectionFailure: function () { onComplete(); }, + onConnectionFailure: function () { + test.equal(connection.status().status, "failed"); + test.matches(connection.status().reason, /Version negotiation failed/); + test.isFalse(connection.status().connected); + onComplete(); + }, onConnected: function () { test.fail(); onComplete(); diff --git a/packages/stream/stream_client.js b/packages/stream/stream_client.js index 5edcbaaea0..c7b5c6d5c7 100644 --- a/packages/stream/stream_client.js +++ b/packages/stream/stream_client.js @@ -9,7 +9,7 @@ Meteor._Stream = function (url) { self.event_callbacks = {}; // name -> [callback] self.server_id = null; self.sent_update_available = false; - self.force_fail = false; // for debugging. + self._forcedToDisconnect = false; //// Constants @@ -151,14 +151,14 @@ _.extend(Meteor._Stream.prototype, { if (self.current_status.connected) { if (options && options._force) { // force reconnect. - self._disconnected(); + self._lostConnection(); } // else, noop. return; } // if we're mid-connection, stop it. if (self.current_status.status === "connecting") { - self._fake_connect_failed(); + self._lostConnection(); } if (self.retry_timer) @@ -170,14 +170,6 @@ _.extend(Meteor._Stream.prototype, { self._retry_now(); }, - // Permanently disconnect a stream. - forceDisconnect: function (flag) { - var self = this; - self.force_fail = flag; - if (flag && self.socket) - self.socket.close(); - }, - _connected: function (welcome_message) { var self = this; @@ -227,23 +219,20 @@ _.extend(Meteor._Stream.prototype, { }, - _cleanup_socket: function () { + _cleanupSocket: function () { var self = this; + self._clearConnectionAndHeartbeatTimers(); if (self.socket) { self.socket.onmessage = self.socket.onclose = self.socket.onerror = function () {}; self.socket.close(); - - var old_socket = self.socket; self.socket = null; - } }, - _disconnected: function () { + _clearConnectionAndHeartbeatTimers: function () { var self = this; - if (self.connection_timer) { clearTimeout(self.connection_timer); self.connection_timer = null; @@ -252,20 +241,40 @@ _.extend(Meteor._Stream.prototype, { clearTimeout(self.heartbeat_timer); self.heartbeat_timer = null; } - self._cleanup_socket(); - self._retry_later(); // sets status. no need to do it here. }, - _fake_connect_failed: function () { + // Permanently disconnect a stream. + forceDisconnect: function (optionalErrorMessage) { var self = this; - self._cleanup_socket(); - self._disconnected(); + self._forcedToDisconnect = true; + self._cleanupSocket(); + if (self.retry_timer) { + clearTimeout(self.retry_timer); + self.retry_timer = null; + } + self.current_status = { + status: "failed", + connected: false, + retryCount: 0, + // XXX Backwards compatibility only. Remove this before 1.0. + retry_count: 0 + }; + if (optionalErrorMessage) + self.current_status.reason = optionalErrorMessage; + self.status_changed(); + }, + + _lostConnection: function () { + var self = this; + + self._cleanupSocket(); + self._retry_later(); // sets status. no need to do it here. }, _heartbeat_timeout: function () { var self = this; Meteor._debug("Connection timeout. No heartbeat received."); - self._fake_connect_failed(); + self._lostConnection(); }, _heartbeat_received: function () { @@ -312,7 +321,7 @@ _.extend(Meteor._Stream.prototype, { _retry_now: function () { var self = this; - if (self.force_fail) + if (self._forcedToDisconnect) return; self.current_status.retryCount += 1; @@ -330,7 +339,7 @@ _.extend(Meteor._Stream.prototype, { _launch_connection: function () { var self = this; - self._cleanup_socket(); // cleanup the old socket, if there was one. + self._cleanupSocket(); // cleanup the old socket, if there was one. // Convert raw URL to SockJS URL each time we open a connection, so that we // can connect to random hostnames and get around browser per-host @@ -358,7 +367,7 @@ _.extend(Meteor._Stream.prototype, { }; self.socket.onclose = function () { // Meteor._debug("stream disconnect", _.toArray(arguments), (new Date()).toDateString()); - self._disconnected(); + self._lostConnection(); }; self.socket.onerror = function () { // XXX is this ever called? @@ -372,7 +381,7 @@ _.extend(Meteor._Stream.prototype, { if (self.connection_timer) clearTimeout(self.connection_timer); self.connection_timer = setTimeout( - _.bind(self._fake_connect_failed, self), + _.bind(self._lostConnection, self), self.CONNECT_TIMEOUT); } });