From a964880a4d0ba6d3a9e66fcbd07fdb9fa3e6db8f Mon Sep 17 00:00:00 2001 From: Ivan Etchart Date: Thu, 18 May 2017 17:08:18 -0300 Subject: [PATCH 1/4] Avoid .any not rejecting when promises reject with no value --- q.js | 7 +++++-- spec/q-spec.js | 21 ++++++++++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/q.js b/q.js index 14dc24a6..8eb68bf5 100644 --- a/q.js +++ b/q.js @@ -1645,8 +1645,11 @@ function any(promises) { function onRejected(err) { pendingCount--; if (pendingCount === 0) { - err.message = ("Q can't get fulfillment value from any promise, all " + - "promises were rejected. Last error message: " + err.message); + if (err) { + err.message = ("Q can't get fulfillment value from any promise, all " + + "promises were rejected. Last error message: " + err.message); + } + deferred.reject(err); } } diff --git a/spec/q-spec.js b/spec/q-spec.js index 9acb19b1..1cf282d4 100644 --- a/spec/q-spec.js +++ b/spec/q-spec.js @@ -1204,10 +1204,22 @@ describe("any", function() { return testReject(promises, deferreds); }); - function testReject(promises, deferreds) { + + it("rejects after all promises are rejected with no values", function() { + var deferreds = [Q.defer(), Q.defer()]; + var promises = [deferreds[0].promise, deferreds[1].promise]; + + return testReject(promises, deferreds, true); + }); + + function testReject(promises, deferreds, expectNull) { var promise = Q.any(promises); var expectedError = new Error('Rejected'); + if (expectNull) { + var expectedError = null; + } + for (var index = 0; index < deferreds.length; index++) { var deferred = deferreds[index]; (function() { @@ -1219,8 +1231,11 @@ describe("any", function() { .then(function() { expect(promise.isRejected()).toBe(true); expect(promise.inspect().reason).toBe(expectedError); - expect(promise.inspect().reason.message) - .toBe("Q can't get fulfillment value from any promise, all promises were rejected. Last error message: Rejected"); + + if (!expectNull) { + expect(promise.inspect().reason.message) + .toBe("Q can't get fulfillment value from any promise, all promises were rejected. Last error message: Rejected"); + } }) .timeout(1000); } From 1bcb612fe565ad02e8eaa5e11712480dfd77a59b Mon Sep 17 00:00:00 2001 From: Ivan Etchart Date: Wed, 31 May 2017 17:04:46 -0300 Subject: [PATCH 2/4] Improve any behavior --- q.js | 10 +++++----- spec/q-spec.js | 9 ++++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/q.js b/q.js index 8eb68bf5..27ba23bf 100644 --- a/q.js +++ b/q.js @@ -1645,12 +1645,12 @@ function any(promises) { function onRejected(err) { pendingCount--; if (pendingCount === 0) { - if (err) { - err.message = ("Q can't get fulfillment value from any promise, all " + - "promises were rejected. Last error message: " + err.message); - } + var rejection = err || new Error("Rejection was null/undefined"); + + rejection.message = ("Q can't get fulfillment value from any promise, all " + + "promises were rejected. Last error message: " + err.message); - deferred.reject(err); + deferred.reject(rejection); } } function onProgress(progress) { diff --git a/spec/q-spec.js b/spec/q-spec.js index 1cf282d4..3eab603d 100644 --- a/spec/q-spec.js +++ b/spec/q-spec.js @@ -1217,7 +1217,7 @@ describe("any", function() { var expectedError = new Error('Rejected'); if (expectNull) { - var expectedError = null; + var expectedError = new Error("Rejection was null/undefined"); } for (var index = 0; index < deferreds.length; index++) { @@ -1232,9 +1232,12 @@ describe("any", function() { expect(promise.isRejected()).toBe(true); expect(promise.inspect().reason).toBe(expectedError); - if (!expectNull) { + if (expectNull) { expect(promise.inspect().reason.message) - .toBe("Q can't get fulfillment value from any promise, all promises were rejected. Last error message: Rejected"); + .toBe("Q can't get fulfillment value from any promise, all promises were rejected. Last error message: Rejection was null/undefined"); + } else { + expect(promise.inspect().reason.message) + .toBe("Q can't get fulfillment value from any promise, all promises were rejected. Last error message: Rejected"); } }) .timeout(1000); From a78a46ef9883a25dd322b29bb1b424c97132d2c1 Mon Sep 17 00:00:00 2001 From: Ivan Etchart Date: Wed, 31 May 2017 18:34:08 -0300 Subject: [PATCH 3/4] Improve tests --- q.js | 4 ++-- spec/q-spec.js | 46 +++++++++++++++++++++++++++------------------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/q.js b/q.js index 27ba23bf..0bf41a6d 100644 --- a/q.js +++ b/q.js @@ -1645,10 +1645,10 @@ function any(promises) { function onRejected(err) { pendingCount--; if (pendingCount === 0) { - var rejection = err || new Error("Rejection was null/undefined"); + var rejection = err || new Error("Rejection value was: " + err); rejection.message = ("Q can't get fulfillment value from any promise, all " + - "promises were rejected. Last error message: " + err.message); + "promises were rejected. Last error message: " + rejection.message); deferred.reject(rejection); } diff --git a/spec/q-spec.js b/spec/q-spec.js index 3eab603d..f1468fd5 100644 --- a/spec/q-spec.js +++ b/spec/q-spec.js @@ -1192,7 +1192,7 @@ describe("any", function() { var deferreds = [Q.defer(), Q.defer()]; var promises = [deferreds[0].promise, deferreds[1].promise]; - return testReject(promises, deferreds); + return testReject(promises, deferreds, new Error('Rejected')); }); it("rejects after all promises in a sparse array are rejected", function() { @@ -1201,44 +1201,52 @@ describe("any", function() { promises[0] = deferreds[0].promise; promises[3] = deferreds[1].promise; - return testReject(promises, deferreds); + return testReject(promises, deferreds, new Error("Rejected")); }); - it("rejects after all promises are rejected with no values", function() { + it("rejects after all promises are rejected with null", function() { var deferreds = [Q.defer(), Q.defer()]; var promises = [deferreds[0].promise, deferreds[1].promise]; - return testReject(promises, deferreds, true); + return testReject(promises, deferreds, null); }); - function testReject(promises, deferreds, expectNull) { - var promise = Q.any(promises); - var expectedError = new Error('Rejected'); + it("rejects after all promises are rejected with undefined", function() { + var deferreds = [Q.defer(), Q.defer()]; + var promises = [deferreds[0].promise, deferreds[1].promise]; - if (expectNull) { - var expectedError = new Error("Rejection was null/undefined"); + return testReject(promises, deferreds, undefined); + }); + + function testReject(promises, deferreds, rejectionValue) { + var promise = Q.any(promises); + var expectedError; + + switch (rejectionValue) { + case null: + expectedError = new Error("Rejection value was: null"); + break; + case undefined: + expectedError = new Error("Rejection value was: undefined"); + break; + default: + expectedError = new Error(rejectionValue.message); } for (var index = 0; index < deferreds.length; index++) { var deferred = deferreds[index]; (function() { - deferred.reject(expectedError); + deferred.reject(rejectionValue); })(); } return Q.delay(250) .then(function() { expect(promise.isRejected()).toBe(true); - expect(promise.inspect().reason).toBe(expectedError); - - if (expectNull) { - expect(promise.inspect().reason.message) - .toBe("Q can't get fulfillment value from any promise, all promises were rejected. Last error message: Rejection was null/undefined"); - } else { - expect(promise.inspect().reason.message) - .toBe("Q can't get fulfillment value from any promise, all promises were rejected. Last error message: Rejected"); - } + expect(promise.inspect().reason).toEqual(expectedError); + expect(promise.inspect().reason.message) + .toBe("Q can't get fulfillment value from any promise, all promises were rejected. Last error message: " + expectedError.message); }) .timeout(1000); } From 9175f60da197b2edf23dfc55fb7343502216ebd9 Mon Sep 17 00:00:00 2001 From: Ivan Etchart Date: Wed, 31 May 2017 18:55:28 -0300 Subject: [PATCH 4/4] Apply feedback --- q.js | 2 +- spec/q-spec.js | 15 +++++---------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/q.js b/q.js index 0bf41a6d..6e467958 100644 --- a/q.js +++ b/q.js @@ -1645,7 +1645,7 @@ function any(promises) { function onRejected(err) { pendingCount--; if (pendingCount === 0) { - var rejection = err || new Error("Rejection value was: " + err); + var rejection = err || new Error("" + err); rejection.message = ("Q can't get fulfillment value from any promise, all " + "promises were rejected. Last error message: " + rejection.message); diff --git a/spec/q-spec.js b/spec/q-spec.js index f1468fd5..6d4068b5 100644 --- a/spec/q-spec.js +++ b/spec/q-spec.js @@ -1192,7 +1192,7 @@ describe("any", function() { var deferreds = [Q.defer(), Q.defer()]; var promises = [deferreds[0].promise, deferreds[1].promise]; - return testReject(promises, deferreds, new Error('Rejected')); + return testReject(promises, deferreds, new Error("Rejected")); }); it("rejects after all promises in a sparse array are rejected", function() { @@ -1223,15 +1223,10 @@ describe("any", function() { var promise = Q.any(promises); var expectedError; - switch (rejectionValue) { - case null: - expectedError = new Error("Rejection value was: null"); - break; - case undefined: - expectedError = new Error("Rejection value was: undefined"); - break; - default: - expectedError = new Error(rejectionValue.message); + if (rejectionValue) { + expectedError = new Error(rejectionValue.message); + } else { + expectedError = new Error("" + rejectionValue); } for (var index = 0; index < deferreds.length; index++) {