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

Support getting current Promise queue length #39

Merged
merged 4 commits into from
May 12, 2023

Conversation

jsau-
Copy link
Contributor

@jsau- jsau- commented May 10, 2023

Useful in cases e.g. a consuming program wants to check for promise-queue congestion, for reporting on growing backlogs (e.g. In my case exposing the current promise-queue length as a custom Prometheus metric to alert if the process is seeing latency in churning through what should be a quick-to-process set of queued promises.)

Useful in cases e.g. a consuming program wants to check for promise-queue congestion, for reporting on task backlogs.
index.js Outdated
@@ -92,6 +92,8 @@ export default function pThrottle({limit, interval, strict}) {

throttled.isEnabled = true;

throttled.queueLength = () => queue.size;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be a getter instead of a function.

queueSize would be a more corrct name.

Copy link
Contributor Author

@jsau- jsau- May 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - I might be misunderstanding the comment - I don't believe we can use the get keyword here 'cos we're not in a class / object literal, we're in a function.

Would you mind providing a snippet of what you'd like this changing to and I can get it sorted? Cheers!

(Renamed to queueSize though!)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object.defineProperty

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be sorted in latest commit!

index.d.ts Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
@sindresorhus sindresorhus merged commit 3e7a732 into sindresorhus:main May 12, 2023
1 check passed
@sindresorhus
Copy link
Owner

Thanks :)

@whyando
Copy link

whyando commented May 29, 2023

Would it make more sense if queueSize was a property of the throttle not the throttled function?

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

Successfully merging this pull request may close these issues.

None yet

3 participants