fastify / fastify/fastify-throttle

Multiple setInterval

Open
#4 5 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
23
Forks
6
PR merge metrics
No merged PRs in 30d

Description

### Prerequisites

- [X] I have written a descriptive issue title
- [X] I have searched existing issues to ensure the issue has not already been raised

### Issue

Hi! I was interested in this library after reading Matteo's newsletter. I've looked at the code, and some questions came to mind:

1. If you have thousands of concurrent requests that are responding throttled, it would result in thousands of `setInterval` being set. Have you considered using one "shared" given that the interval amount is always the same (1000)?
2. In `intervalHandler`, I wonder if there's a (very remote) chance of the `setImmediate` callback being called after another `setInterval` callback is executed. Then, setImmediate is called again because the buffer is not cleared. Have you considered calling `_transform()` within the `setInterval` callback and only doing `this._callback()` on `setImmediate`?
```
function intervalHandler (instance) {
try {
instance._allowedBytes = instance.bytesPerSecondFn((Date.now() - instance.startTime) / 1000, instance.bytes)

if (
instance._allowedBytes !== 0 &&
instance._buffer !== null
) {
const cb = this._callback;
this._callback = (err) => setImmediate(() => cb(err));
instance._transform();
}
} catch (err) {
// Handle any errors during interval updates
instance.emit('error', err)
}
}
```
True that if `setImmediate` executes more than a second later, then there are bigger problems with the service, but at least this won't add to the issue.

3. Have you considered making the ThrottleStream standalone so it can be used with other frameworks?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.