diff --git a/lib/init.lua b/lib/init.lua index 24bae42..2f090d1 100644 --- a/lib/init.lua +++ b/lib/init.lua @@ -1205,6 +1205,14 @@ end function Promise.prototype:_andThen(traceback, successHandler, failureHandler) self._unhandledRejection = false + -- If we are already cancelled, we return a cancelled Promise + if self._status == Promise.Status.Cancelled then + local promise = Promise.new(function() end) + promise:cancel() + + return promise + end + -- Create a new promise to follow this part of the chain return Promise._new(traceback, function(resolve, reject, onCancel) -- Our default callbacks just pass values onto the next promise. @@ -1239,14 +1247,6 @@ function Promise.prototype:_andThen(traceback, successHandler, failureHandler) elseif self._status == Promise.Status.Rejected then -- This promise died a terrible death! Trigger failure immediately. failureCallback(unpack(self._values, 1, self._valuesLength)) - elseif self._status == Promise.Status.Cancelled then - -- We don't want to call the success handler or the failure handler, - -- we just reject this promise outright. - reject(Error.new({ - error = "Promise is cancelled", - kind = Error.Kind.AlreadyCancelled, - context = "Promise created at\n\n" .. traceback, - })) end end, self) end @@ -1430,28 +1430,37 @@ end --[[ Used to set a handler for when the promise resolves, rejects, or is - cancelled. Returns a new promise chained from this promise. + cancelled. ]] -function Promise.prototype:_finally(traceback, finallyHandler, onlyOk) - if not onlyOk then - self._unhandledRejection = false - end +function Promise.prototype:_finally(traceback, finallyHandler) + self._unhandledRejection = false + + local promise = Promise._new(traceback, function(resolve, reject, onCancel) + onCancel(function() + -- The finally Promise is not a proper consumer of self. We don't care about the resolved value. + -- All we care about is running at the end. Therefore, if self has no other consumers, it's safe to + -- cancel. We don't need to hold out cancelling just because there's a finally handler. + self:_consumerCancelled(self) + end) - -- Return a promise chained off of this promise - return Promise._new(traceback, function(resolve, reject) local finallyCallback = resolve if finallyHandler then - finallyCallback = createAdvancer(traceback, finallyHandler, resolve, reject) - end - - if onlyOk then - local callback = finallyCallback finallyCallback = function(...) - if self._status == Promise.Status.Rejected then - return resolve(self) - end + local callbackReturn = finallyHandler(...) - return callback(...) + if Promise.is(callbackReturn) then + callbackReturn + :finally(function(status) + if status ~= Promise.Status.Rejected then + resolve(self) + end + end) + :catch(function(...) + reject(...) + end) + else + resolve(self) + end end end @@ -1462,7 +1471,9 @@ function Promise.prototype:_finally(traceback, finallyHandler, onlyOk) -- The promise already settled or was cancelled, run the callback now. finallyCallback(self._status) end - end, self) + end) + + return promise end --[=[ @@ -1543,69 +1554,6 @@ function Promise.prototype:finallyReturn(...) end) end ---[=[ - Set a handler that will be called only if the Promise resolves or is cancelled. This method is similar to `finally`, except it doesn't catch rejections. - - :::caution - `done` should be reserved specifically when you want to perform some operation after the Promise is finished (like `finally`), but you don't want to consume rejections (like in this example). You should use `andThen` instead if you only care about the Resolved case. - ::: - - :::warning - Like `finally`, if the Promise is cancelled, any Promises chained off of it with `andThen` won't run. Only Promises chained with `done` and `finally` will run in the case of cancellation. - ::: - - Returns a new promise chained from this promise. - - @param doneHandler (status: Status) -> ...any - @return Promise<...any> -]=] -function Promise.prototype:done(doneHandler) - assert(doneHandler == nil or isCallable(doneHandler), string.format(ERROR_NON_FUNCTION, "Promise:done")) - return self:_finally(debug.traceback(nil, 2), doneHandler, true) -end - ---[=[ - Same as `andThenCall`, except for `done`. - - Attaches a `done` handler to this Promise that calls the given callback with the predefined arguments. - - @param callback (...: any) -> any - @param ...? any -- Additional arguments which will be passed to `callback` - @return Promise -]=] -function Promise.prototype:doneCall(callback, ...) - assert(isCallable(callback), string.format(ERROR_NON_FUNCTION, "Promise:doneCall")) - local length, values = pack(...) - return self:_finally(debug.traceback(nil, 2), function() - return callback(unpack(values, 1, length)) - end, true) -end - ---[=[ - Attaches a `done` handler to this Promise that discards the resolved value and returns the given value from it. - - ```lua - promise:doneReturn("some", "values") - ``` - - This is sugar for - - ```lua - promise:done(function() - return "some", "values" - end) - ``` - - @param ... any -- Values to return from the function - @return Promise -]=] -function Promise.prototype:doneReturn(...) - local length, values = pack(...) - return self:_finally(debug.traceback(nil, 2), function() - return unpack(values, 1, length) - end, true) -end - --[=[ Yields the current thread until the given Promise completes. Returns the Promise's status, followed by the values that the promise resolved or rejected with. diff --git a/lib/init.spec.lua b/lib/init.spec.lua index 0edcea9..51e8c57 100644 --- a/lib/init.spec.lua +++ b/lib/init.spec.lua @@ -556,9 +556,11 @@ return function() count += 1 end) - root:andThen(function() - count += 1 - end):cancel() + root + :andThen(function() + count += 1 + end) + :cancel() resolve("foo") @@ -636,7 +638,7 @@ return function() it("should track consumers", function() local pending = Promise.new(function() end) local p0 = Promise.resolve() - local p1 = p0:finally(function() + local p1 = p0:andThen(function() return pending end) local p2 = Promise.new(function(resolve) @@ -703,18 +705,10 @@ return function() expect(callCount).to.equal(5) end) - it("should be a child of the parent Promise", function() - local p1 = Promise.new(function() end) - local p2 = p1:finally(function() end) - - expect(p2._parent).to.equal(p1) - expect(p1._consumers[p2]).to.equal(true) - end) - - it("should forward return values", function() + it("should not forward return values", function() local value - Promise.resolve() + Promise.resolve(2) :finally(function() return 1 end) @@ -722,7 +716,124 @@ return function() value = v end) - expect(value).to.equal(1) + expect(value).to.equal(2) + end) + + it("should not consume rejections", function() + local catchRan = false + local thenRan = false + Promise.reject(5) + :finally(function() + return 42 + end) + :andThen(function() + thenRan = true + end) + :catch(function(value) + catchRan = true + expect(value).to.equal(5) + end) + + expect(catchRan).to.equal(true) + expect(thenRan).to.equal(false) + end) + + it("should wait for returned promises", function() + local resolve + local promise = Promise.reject("foo"):finally(function() + return Promise.new(function(r) + resolve = r + end) + end) + + expect(promise:getStatus()).to.equal(Promise.Status.Started) + + resolve() + + expect(promise:getStatus()).to.equal(Promise.Status.Rejected) + local _, value = promise:_unwrap() + expect(value).to.equal("foo") + end) + + it("should reject with a returned rejected promise's value", function() + local reject + local promise = Promise.reject("foo"):finally(function() + return Promise.new(function(_, r) + reject = r + end) + end) + + expect(promise:getStatus()).to.equal(Promise.Status.Started) + + reject("bar") + + expect(promise:getStatus()).to.equal(Promise.Status.Rejected) + local _, value = promise:_unwrap() + expect(value).to.equal("bar") + end) + + it("should reject when handler errors", function() + local errorValue = {} + local promise = Promise.reject("bar"):finally(function() + error(errorValue) + end) + + local ok, value = promise:_unwrap() + + expect(ok).to.equal(false) + expect(value).to.equal(errorValue) + end) + + it("should not prevent cancellation", function() + local promise = Promise.new(function() end) + + local finallyRan = false + promise:finally(function() + finallyRan = true + end) + + local consumer = promise:andThen(function() end) + + consumer:cancel() + + expect(promise:getStatus()).to.equal(Promise.Status.Cancelled) + expect(finallyRan).to.equal(true) + end) + + it("should propagate cancellation downwards", function() + local finallyRan = false + local andThenRan = false + local root = Promise.new(function() end) + + local consumer = root:finally(function() + finallyRan = true + end) + + root:cancel() + + expect(root:getStatus()).to.equal(Promise.Status.Cancelled) + expect(consumer:getStatus()).to.equal(Promise.Status.Cancelled) + + expect(finallyRan).to.equal(true) + expect(andThenRan).to.equal(false) + end) + + it("should propagate cancellation upwards", function() + local finallyRan = false + local andThenRan = false + local root = Promise.new(function() end) + + local consumer = root:finally(function() + finallyRan = true + end) + + consumer:cancel() + + expect(root:getStatus()).to.equal(Promise.Status.Cancelled) + expect(consumer:getStatus()).to.equal(Promise.Status.Cancelled) + + expect(finallyRan).to.equal(true) + expect(andThenRan).to.equal(false) end) end) @@ -1168,20 +1279,6 @@ return function() end) end) - describe("Promise:doneReturn", function() - it("should return the given values", function() - local value1, value2 - - Promise.resolve():doneReturn(1, 2):andThen(function(one, two) - value1 = one - value2 = two - end) - - expect(value1).to.equal(1) - expect(value2).to.equal(2) - end) - end) - describe("Promise:andThenCall", function() it("should call the given function with arguments", function() local value1, value2 @@ -1195,47 +1292,6 @@ return function() end) end) - describe("Promise:doneCall", function() - it("should call the given function with arguments", function() - local value1, value2 - Promise.resolve():doneCall(function(a, b) - value1 = a - value2 = b - end, 3, 4) - - expect(value1).to.equal(3) - expect(value2).to.equal(4) - end) - end) - - describe("Promise:done", function() - it("should trigger on resolve or cancel", function() - local promise = Promise.new(function() end) - local value - - local p = promise:done(function() - value = true - end) - - expect(value).to.never.be.ok() - promise:cancel() - expect(p:getStatus()).to.equal(Promise.Status.Cancelled) - expect(value).to.equal(true) - - local never, always - Promise.reject() - :done(function() - never = true - end) - :finally(function() - always = true - end) - - expect(never).to.never.be.ok() - expect(always).to.be.ok() - end) - end) - describe("Promise.some", function() it("should resolve once the goal is reached", function() local p = Promise.some({