diff --git a/js/README.md b/js/README.md index 9f5bf9e6..dcaa57ac 100644 --- a/js/README.md +++ b/js/README.md @@ -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:** diff --git a/js/src/mopidy.js b/js/src/mopidy.js index d1afc289..473d81a5 100644 --- a/js/src/mopidy.js +++ b/js/src/mopidy.js @@ -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); diff --git a/js/test/mopidy-test.js b/js/test/mopidy-test.js index 9f2509fc..97525ba7 100644 --- a/js/test/mopidy-test.js +++ b/js/test/mopidy-test.js @@ -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); + }) + ); } },