sodiray / radash

Functional utility library - modern, simple, typed, powerful
https://radash-docs.vercel.app
MIT License
4.31k stars 173 forks source link

`tryit` and `parallel` get inconsistent results (expected) #246

Closed ghost closed 1 year ago

ghost commented 1 year ago

Hello,

I ran into an issue using tryit and parallel, giving this code inspired by the doc:

async function test() {
  const userIds = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]

  const [err, users] = await tryit(parallel)(3, userIds, async (userId) => {
    const state = ['ok', 'fail']

    if (draw(state) === 'ok') {
      return `id_${userId}`
    } else {
      throw new Error(`No, I don\'t want to find user ${userId}`)
    }
  })

  console.log(err, users)
}

Results are:

Is this expected? If so, I am using it correctly?

I use the latest radash version 10.7

ghost commented 1 year ago

I've seen the sources, and it is expected. I may have misunderstand the purpose of these methods 😅

Have you any suggestion? or are you planning this approach?

sodiray commented 1 year ago

Hey @BenjaminMINK thanks for bringing this up, this isn't explained very well in the docs (we can fix that). Most retry and concurrency functions handle errros in a set like format. Given 3 input items you'll get back [[1, err?], [2, err?], [3, err?]], you see this with Promise.all. IMO, that is a total pain to process. Also, node itself is headed in a different direction with the semi-newly added AggregateError. So, parallel works the same way, it will either return the results or throw an AggregateError (except it's our own implementation because the support isn't there yet) which has an errors property with all the errors.

There could be an opportunity to improve here by adding a property like partialResults to the AggregateError so you can access all the successful results even if some fail when you catch the aggregate error.

*sounds like you may have figured all this out but still leaving this here for posterity and others

ghost commented 1 year ago

Hello,

Most retry and concurrency functions handle errros in a set like format [...]

Oh I see

There could be an opportunity to improve here by adding a property like partialResults to the AggregateError so you can access all the successful results even if some fail when you catch the aggregate error.

It may be interesting indeed, but are we "allowed" to add a custom property to a standardized AggregateError object?

I was thinking of returning errors and results separately, like so:

[AggregateError?, results?]

*sounds like you may have figured all this out but still leaving this here for posterity and others

Exactly 😁

punowo commented 1 year ago

I'm sorry but I think it should be more evident that parallel and all don't return anything if something errors. I don't think I'm the only one who was expecting errors in AggregateError and whatever succeeded in results.