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

Should clearQueue() reject the pending promises? #25

Open
jedwards1211 opened this issue Apr 20, 2020 · 6 comments
Open

Should clearQueue() reject the pending promises? #25

jedwards1211 opened this issue Apr 20, 2020 · 6 comments

Comments

@jedwards1211
Copy link

Right now it leaves them hanging indefinitely. It seems like undesirable behavior, though I guess it ultimately depends on the use case.

@sindresorhus
Copy link
Owner

There's no way to reject promises from the outside. We could maybe support cancelling them if they are of the type p-cancelable.

@jedwards1211
Copy link
Author

jedwards1211 commented Apr 21, 2020

There's no way to reject promises from the outside

I take it you mean promises returned by the user-supplied function, but I'm not talking about those, I'm talking about the promises p-limit created for queued functions that haven't even been called yet:

const generator = (fn, ...args) => new Promise(resolve => enqueue(fn, resolve, ...args));
                                       ^^^^^^^

We could change this to

const generator = (fn, ...args) => new Promise((resolve, reject) => enqueue(fn, resolve, reject, ...args));

And refactor the queue entries to include reject (instead of just being the bound function), so that clearQueue() can call reject on all the queue entries.

jedwards1211 added a commit to jedwards1211/p-limit that referenced this issue Apr 21, 2020
BREAKING CHANGE: when clearQueue() is called, promises for queued function calls will be rejected.

fix sindresorhus#25
jedwards1211 added a commit to jedwards1211/p-limit that referenced this issue May 26, 2020
BREAKING CHANGE: when clearQueue() is called, promises for queued function calls will be rejected.

fix sindresorhus#25
@RichardBronosky
Copy link

RichardBronosky commented Jul 14, 2020

Could you please add an example of how to clearQueue() on error? I cannot figure it out. No matter when/where I try to call limit.clearQueue() I get TypeError: limit.clearQueue is not a function

@jedwards1211
Copy link
Author

@RichardBronosky maybe you have an old version installed? Looks like clearQueue was only added 3 months ago.

@sindresorhus
Copy link
Owner

I think it would be better to support AbortController and let the user decide what to do about the signal.

@nickluger
Copy link

Reading the docs, I assumed this:

try {
   await Promise.all(things.map(async (thing) => limit(async () => {/*...*/}) ));
} catch (error) {
  limit.clearQueue();
  throw error;
}

would prevent promises not yet queued from running.

For example, if things.length === 1000 and limit is 3 and one of the first 3 promises throws, the queue is emptied and the remaining 997 other things and their Promises are not executed.

Is this correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants