DJWassink / Promise-parallel-throttle

It's kinda like Promise.all(), but throttled!
MIT License
81 stars 3 forks source link

Next callback gobbles up error when rejected #13

Closed i-like-robots closed 7 years ago

i-like-robots commented 7 years ago

I created a next callback function to implement an error limit:

  const next = (status) => {
    if (status.amountRejected >= maxErrors) {
      const lastFailure = status.rejectedIndexes[status.rejectedIndexes.length - 1]
      return Promise.reject(status.taskResults[lastFailure])
    }

    return Promise.resolve()
  }

However the error this rejects with is gobbled up because there is an empty rejected handler:

https://github.com/DJWassink/Promise-parallel-throttle/blob/master/src/throttle.ts#L74

I think it would be useful if this empty handler was removed so that any rejections may be caught and handled my the implementer.

I understand this may cause issues if more tasks have already been spawned but I would be interested to work out a solution.

DJWassink commented 7 years ago

I am thinking about rejecting the complete queue if the nextCheck throws. This would result in a bit of a different API tho.

The nextCheck should always resolve with a boolean, true to start the next task false to abort;

If it however rejects this error will propagate to the user's code and they should do their cleanup however they want.

DJWassink commented 7 years ago

Rejecting in the nextCheck will propagate now in version 2.3.0😄

i-like-robots commented 7 years ago

Nice one, thanks!