Make more DDP errors specific

These errors should look like connection errors, not clean closes:

- Heartbeat timeout (DDP-level or SockJS-level)
- Connection timeout (SockJS implementation)

The "disconnected with no error while waiting for something we asked
for" error in ServiceConnection is now a DDP.ConnectionError so that it
prints better.

If a command refuses to run due to a catalog sync error, it should print
the error. (Commands that merely warn that they could not sync don't
print that error, unless METEOR_LOG=debug.)
This commit is contained in:
David Glasser
2014-10-20 14:59:31 -07:00
committed by Emily Stark
parent e14cc7ab97
commit a2218ce786
5 changed files with 25 additions and 22 deletions

View File

@@ -950,9 +950,9 @@ _.extend(Connection.prototype, {
// We detected via DDP-level heartbeats that we've lost the
// connection. Unlike `disconnect` or `close`, a lost connection
// will be automatically retried.
_lostConnection: function () {
_lostConnection: function (error) {
var self = this;
self._stream._lostConnection();
self._stream._lostConnection(error);
},
/**
@@ -1036,15 +1036,8 @@ _.extend(Connection.prototype, {
heartbeatInterval: self._heartbeatInterval,
heartbeatTimeout: self._heartbeatTimeout,
onTimeout: function () {
if (Meteor.isClient && ! self._stream._isStub) {
// only print on the client. this message is useful on the
// browser console to see that we've lost connection. on the
// server (eg when doing server-to-server DDP), it gets
// kinda annoying. also this matches the behavior with
// sockjs timeouts.
Meteor._debug("Connection timeout. No DDP heartbeat received.");
}
self._lostConnection();
self._lostConnection(
new DDP.ConnectionError("DDP heartbeat timed out"));
},
sendPing: function () {
self._send({msg: 'ping'});

View File

@@ -153,6 +153,7 @@ _.extend(LivedataTest.ClientStream.prototype, {
// if we're mid-connection, stop it.
if (self.currentStatus.status === "connecting") {
// Pretend it's a clean close.
self._lostConnection();
}
@@ -193,8 +194,7 @@ _.extend(LivedataTest.ClientStream.prototype, {
self.statusChanged();
},
// maybeError is only guaranteed to be set for the Node implementation, and
// not on a clean close.
// maybeError is set unless it's a clean protocol-level close.
_lostConnection: function (maybeError) {
var self = this;

View File

@@ -80,7 +80,7 @@ _.extend(LivedataTest.ClientStream.prototype, {
},
_cleanup: function () {
_cleanup: function (maybeError) {
var self = this;
self._clearConnectionAndHeartbeatTimers();
@@ -91,7 +91,9 @@ _.extend(LivedataTest.ClientStream.prototype, {
self.socket = null;
}
_.each(self.eventCallbacks.disconnect, function (callback) { callback(); });
_.each(self.eventCallbacks.disconnect, function (callback) {
callback(maybeError);
});
},
_clearConnectionAndHeartbeatTimers: function () {
@@ -109,7 +111,7 @@ _.extend(LivedataTest.ClientStream.prototype, {
_heartbeat_timeout: function () {
var self = this;
Meteor._debug("Connection timeout. No sockjs heartbeat received.");
self._lostConnection();
self._lostConnection(new DDP.ConnectionError("Heartbeat timed out"));
},
_heartbeat_received: function () {
@@ -171,7 +173,6 @@ _.extend(LivedataTest.ClientStream.prototype, {
});
};
self.socket.onclose = function () {
// Meteor._debug("stream disconnect", _.toArray(arguments), (new Date()).toDateString());
self._lostConnection();
};
self.socket.onerror = function () {
@@ -185,8 +186,9 @@ _.extend(LivedataTest.ClientStream.prototype, {
if (self.connectionTimer)
clearTimeout(self.connectionTimer);
self.connectionTimer = setTimeout(
_.bind(self._lostConnection, self),
self.CONNECT_TIMEOUT);
self.connectionTimer = setTimeout(function () {
self._lostConnection(
new DDP.ConnectionError("DDP connection timed out"));
}, self.CONNECT_TIMEOUT);
}
});

View File

@@ -34,6 +34,7 @@ catalog.Refresh.OnceAtStart.prototype.beforeCommand = function () {
if (self.options.ignoreErrors) {
Console.debug("Failed to update package catalog, but will continue.");
} else {
Console.error(catalog.refreshError);
Console.error("This command requires an up-to-date package catalog. Exiting.");
// Avoid circular dependency.
throw new (require('./main.js').ExitWithCode)(1);
@@ -79,14 +80,17 @@ catalog.refreshOrWarn = function (options) {
// XXX is throwing correct for SQLite errors too? probably.
Console.warn("Unable to refresh catalog (are you offline?)\n");
Console.warn("Unable to refresh catalog (are you offline?)");
// XXX: Make this Console.debug(err)
if (Console.isDebugEnabled()) {
Console.printError(err);
}
Console.warn();
catalog.refreshFailed = true;
catalog.refreshError = err;
return false;
}
};

View File

@@ -68,10 +68,14 @@ var ServiceConnection = function (Package, endpointUrl, options) {
// Otherwise, ignore this error. We're going to reconnect!
return;
}
// Are we waiting to connect or for the result of a method apply or a
// subscribeAndWait? If so, disconnecting is a problem.
if (self.currentFuture) {
var fut = self.currentFuture;
self.currentFuture = null;
fut.throw(error || new Error("DDP disconnected"));
fut.throw(error ||
new Package.ddp.DDP.ConnectionError(
"DDP disconnected while connection in progress"));
} else if (error) {
// We got some sort of error with nobody listening for it; handle it.
// XXX probably have a better way to handle it than this