jhurliman / node-rate-limiter

A generic rate limiter for node.js. Useful for API clients, web crawling, or other tasks that need to be throttled
MIT License
1.51k stars 135 forks source link

Implement `clearTimeouts` method to clear all queued requests #10

Closed omnidan closed 8 years ago

omnidan commented 10 years ago

I had the problem that throttled requests were still being processed after the client had disconnected. Now I just do client.limiter.clearTimeouts() in a disconnect event listener and everything is fine!

omnidan commented 10 years ago

Just wanted to apologize for the lack of tests, this was a quick fix and I simply forgot to include them in the pull request.

EDIT: Just tried your tests and they seem to fail - seems like you're using an old version of vows that doesn't work on node 0.10 (related? https://github.com/RubenVerborgh/N3.js/issues/9). This project seems to have moved to mocha later.

jhurliman commented 9 years ago

This is a good idea, but as it's implemented right now the this.tokenTimeouts array will keep growing for every call to the rate limiter and does not empty unless clearTimeouts is called. The setTimeout callback should be removing its own timer from the array, which should probably switch to a dictionary (object) so the remove can be done with a lookup instead of searching an array.