Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise.all propagates resolved values into the catch handler. #7

Open
hsjunnesson opened this issue Jul 3, 2020 · 2 comments
Open

Comments

@hsjunnesson
Copy link

For Promise.all(), when some promises are resolved, and some rejected, the catch handler receives all values.
This makes it really hard to tell which are errors and which aren't.

local Promise = require 'promise'

local promise1 = Promise.new()
local promise2 = Promise.new()

Promise.all(promise1, promise2)
:catch(function(error)
  print(error[1])
  print(error[2])
end)

promise1:resolve(123)
promise2:reject("GenericError")

Promise.update()

Outputs this:

123
GenericError

I would like to have:

nil
GenericError
@jojo59516
Copy link

I agree with you. But this is a implement of Promises/A+, and Promise.all is not be mentioned.

However, refer to mozilla's javascript doc about Promise.all():

The Promise.all() method takes an iterable of promises as an input, and returns a single Promise that resolves to an array of the results of the input promises. This returned promise will resolve when all of the input's promises have resolved, or if the input iterable contains no promises. It rejects immediately upon any of the input promises rejecting or non-promises throwing an error, and will reject with this first rejection message / error.

So I think function Promise.all should be modified to something like:

function Promise.all(...)
  local promises = {...}
  local results = {}
  local state = State.FULFILLED
  local remaining = #promises

  local promise = Promise.new()

  for i,p in ipairs(promises) do
    p:next(
      function(value)
        results[i] = value
        remaining = remaining - 1
        
        if remaining == 0 then
          transition(promise, State.FULFILLED, results)
        end
      end,
      function(value)
        transition(promise, State.REJECTED, value)
      end
    )
  end

  if remaining == 0 then
    transition(promise, State.FULFILLED, results)
  end

  return promise
end

@ikegami
Copy link

ikegami commented Aug 27, 2020

I agree with jojo59516.

The idea behind following the Promises/A+ spec is to provide consistency between implementations, and the change they propose would do exactly that.

recih pushed a commit to recih/promise.lua that referenced this issue Dec 24, 2020
recih added a commit to recih/promise.lua that referenced this issue Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants