andpp/mopidy
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

js: Reject promises using Error objects

Browse Source 
This is recommended to get proper stack traces, according to
https://github.com/cujojs/when/blob/master/docs/api.md#a-note-on-javascript-errors.

The new Error objects has the exact same properties as the objects we used to
reject promises previously, thus this should be fully backwards compatible, but
improve debuggability.
This commit is contained in:
Stein Magnus Jodal 2014-06-15 12:17:47 +02:00
parent 4a609ce95d
commit 026fdb38a3
3 changed files with 115 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
js/README.md
View File

@ -88,6 +88,10 @@ Changelog
and the
[when.js debugging guide](https://github.com/cujojs/when/blob/master/docs/api.md#debugging-promises).
- All promise rejection values are now of the Error type. This ensures that all
JavaScript VMs will show a useful stack trace if a rejected promise's value
is used to throw an exception.
### 0.2.0 (2014-01-04)
- **Backwards incompatible change for Node.js users:**

38
js/src/mopidy.js
View File

@ -102,12 +102,9 @@ Mopidy.prototype._cleanup = function (closeEvent) {
Object.keys(this._pendingRequests).forEach(function (requestId) {
var resolver = this._pendingRequests[requestId];
delete this._pendingRequests[requestId];
// TODO Mopidy.js 1.0 should reject with an Error object to produce
// usable stack traces
resolver.reject({
message: "WebSocket closed",
closeEvent: closeEvent
});
var error = new Error("WebSocket closed");
error.closeEvent = closeEvent;
resolver.reject(error);
}.bind(this));
this.emit("state:offline");
@ -145,17 +142,11 @@ Mopidy.prototype._handleWebSocketError = function (error) {
Mopidy.prototype._send = function (message) {
switch (this._webSocket.readyState) {
case Mopidy.WebSocket.CONNECTING:
// TODO Mopidy.js 1.0 should reject with an Error object to produce
// usable stack traces
return when.reject({message: "WebSocket is still connecting"});
return when.reject(new Error("WebSocket is still connecting"));
case Mopidy.WebSocket.CLOSING:
// TODO Mopidy.js 1.0 should reject with an Error object to produce
// usable stack traces
return when.reject({message: "WebSocket is closing"});
return when.reject(new Error("WebSocket is closing"));
case Mopidy.WebSocket.CLOSED:
// TODO Mopidy.js 1.0 should reject with an Error object to produce
// usable stack traces
return when.reject({message: "WebSocket is closed"});
return when.reject(new Error("WebSocket is closed"));
default:
var deferred = when.defer();
message.jsonrpc = "2.0";
@ -205,23 +196,22 @@ Mopidy.prototype._handleResponse = function (responseMessage) {
return;
}
var error;
var resolver = this._pendingRequests[responseMessage.id];
delete this._pendingRequests[responseMessage.id];
if (responseMessage.hasOwnProperty("result")) {
resolver.resolve(responseMessage.result);
} else if (responseMessage.hasOwnProperty("error")) {
// TODO Mopidy.js 1.0 should reject with an Error object to produce
// usable stack traces
resolver.reject(responseMessage.error);
error = new Error(responseMessage.error.message);
error.code = responseMessage.error.code;
error.data = responseMessage.error.data;
resolver.reject(error);
this._console.warn("Server returned error:", responseMessage.error);
} else {
// TODO Mopidy.js 1.0 should reject with an Error object to produce
// usable stack traces
resolver.reject({
message: "Response without 'result' or 'error' received",
data: {response: responseMessage}
});
error = new Error("Response without 'result' or 'error' received");
error.data = {response: responseMessage};
resolver.reject(error);
this._console.warn(
"Response without 'result' or 'error' received. Message was:",
responseMessage);

135
js/test/mopidy-test.js
View File

@ -169,12 +169,16 @@ buster.testCase("Mopidy", {
this.mopidy._cleanup(closeEvent);
assert.equals(Object.keys(this.mopidy._pendingRequests).length, 0);
when.join(promise1, promise2).then(done(function () {
assert(false, "Promises should be rejected");
}), done(function (error) {
assert.equals(error.message, "WebSocket closed");
assert.same(error.closeEvent, closeEvent);
}));
when.join(promise1, promise2).done(
done(function () {
assert(false, "Promises should be rejected");
}),
done(function (error) {
assert(error instanceof Error);
assert.equals(error.message, "WebSocket closed");
assert.same(error.closeEvent, closeEvent);
})
);
},
"emits 'state:offline' event when done": function () {
@ -388,12 +392,16 @@ buster.testCase("Mopidy", {
var promise = this.mopidy._send({method: "foo"});
refute.called(this.mopidy._webSocket.send);
promise.then(done(function () {
assert(false);
}), done(function (error) {
assert.equals(
error.message, "WebSocket is still connecting");
}));
promise.done(
done(function () {
assert(false);
}),
done(function (error) {
assert(error instanceof Error);
assert.equals(
error.message, "WebSocket is still connecting");
})
);
},
"immediately rejects request if CLOSING": function (done) {
@ -402,12 +410,15 @@ buster.testCase("Mopidy", {
var promise = this.mopidy._send({method: "foo"});
refute.called(this.mopidy._webSocket.send);
promise.then(done(function () {
assert(false);
}), done(function (error) {
assert.equals(
error.message, "WebSocket is closing");
}));
promise.done(
done(function () {
assert(false);
}),
done(function (error) {
assert(error instanceof Error);
assert.equals(error.message, "WebSocket is closing");
})
);
},
"immediately rejects request if CLOSED": function (done) {
@ -416,12 +427,15 @@ buster.testCase("Mopidy", {
var promise = this.mopidy._send({method: "foo"});
refute.called(this.mopidy._webSocket.send);
promise.then(done(function () {
assert(false);
}), done(function (error) {
assert.equals(
error.message, "WebSocket is closed");
}));
promise.done(
done(function () {
assert(false);
}),
done(function (error) {
assert(error instanceof Error);
assert.equals(error.message, "WebSocket is closed");
})
);
}
},
@ -544,7 +558,11 @@ buster.testCase("Mopidy", {
"rejects and logs requests which get errors back": function (done) {
var stub = this.stub(this.mopidy._console, "warn");
var promise = this.mopidy._send({method: "bar"});
var responseError = {message: "Error", data: {}};
var responseError = {
code: -32601,
message: "Method not found",
data: {}
};
var responseMessage = {
jsonrpc: "2.0",
id: Object.keys(this.mopidy._pendingRequests)[0],
@ -555,11 +573,48 @@ buster.testCase("Mopidy", {
assert.calledOnceWith(stub,
"Server returned error:", responseError);
promise.then(done(function () {
assert(false);
}), done(function (error) {
assert.equals(error, responseError);
}));
promise.done(
done(function () {
assert(false);
}),
done(function (error) {
assert(error instanceof Error);
assert.equals(error.code, responseError.code);
assert.equals(error.message, responseError.message);
assert.equals(error.data, responseError.data);
})
);
},
"rejects and logs requests which get errors without data": function (done) {
var stub = this.stub(this.mopidy._console, "warn");
var promise = this.mopidy._send({method: "bar"});
var responseError = {
code: -32601,
message: "Method not found"
// 'data' key intentionally missing
};
var responseMessage = {
jsonrpc: "2.0",
id: Object.keys(this.mopidy._pendingRequests)[0],
error: responseError
};
this.mopidy._handleResponse(responseMessage);
assert.calledOnceWith(stub,
"Server returned error:", responseError);
promise.done(
done(function () {
assert(false);
}),
done(function (error) {
assert(error instanceof Error);
assert.equals(error.code, responseError.code);
assert.equals(error.message, responseError.message);
refute.defined(error.data);
})
);
},
"rejects and logs responses without result or error": function (done) {
@ -575,14 +630,18 @@ buster.testCase("Mopidy", {
assert.calledOnceWith(stub,
"Response without 'result' or 'error' received. Message was:",
responseMessage);
promise.then(done(function () {
assert(false);
}), done(function (error) {
assert.equals(
error.message,
"Response without 'result' or 'error' received");
assert.equals(error.data.response, responseMessage);
}));
promise.done(
done(function () {
assert(false);
}),
done(function (error) {
assert(error instanceof Error);
assert.equals(
error.message,
"Response without 'result' or 'error' received");
assert.equals(error.data.response, responseMessage);
})
);
}
},