promise-pool: Allow the pool to "failLoud" on error

Currently, the promise pool run the given callback on each item. Even though an error happens during the processing. The pool won’t fail on errors and instead collect all errors.

This PR should add an option to let the pool fail loud instead of being silent on errors.

Expected API

try {
  const pool = new PromisePool()
    .for(users)
    .failLoud()  // <-- new method

  await pool.process(async user => {
    // process user
  })
} catch (error) {
  // handle error 
}

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 31 (22 by maintainers)

Commits related to this issue

Most upvoted comments

@SimonJang Hey Simon, I agree: at some point most of the promise pool users want some kind of dedicated error handling for their app.

I’m currently thinking through the error handling cases. I’ll share my an overview of my thoughts tomorrow. We can then iterate over all ideas

@marcuspoehls Ok, I’ll have something in PR by the end of the week. Enjoy your vacation!

@txau Hey Jaume, I’m planning a new major release of the promise pool library changing the default behavior of collecting all errors to throwing immediately on error. As a user of the package, you’re able to intercept the error handling.

Here’s how the new functionality may look like (depending on your feedback 😃 )

const PromisePool = require('@supercharge/promise-pool')

const results = await PromisePool
  .for(items)
  .withConcurrency(3)
  .onError((error, item) => {
    if (error instanceof SystemError) {
      return
    }
    
    throw error
  })
  .process(item => {
    // ... item processing
  })

What do you think about the onError callback to customize the error handling? The onError callback will be called for each item throwing an error in the process callback.

At this point, there’s no support for an async error handler. Would an async error handler useful? I mean .onError(async (error, item) => {})

@Fly-Style Great, thank you!

@marcuspoehls Nice!

My idea was to add a .failLoud() method that configures the promise-pool to throw an error as soon as it occurs. At this point, the library collects all errors and processes every item.

In the last days, I was thinking through this approach and decided to change direction. I’d like to introduce a breaking change and throw errors instead of collecting them.

If you’re still interested in implementing the changes, here’s what the promise-pool library should do in the future:

  • throw on error: no try/catch to collect errors
  • return the results directly, not wrapped in an object

Does this make sense? Are you still on board implementing the changes and submitting a PR for it?

@Fly-Style Hey Alex, thank you for the update 👍