fastify / fastify/fastify-throttle
Multiple setInterval
- 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
Assessment
This issue has not been assessed yet.