express-rate icon indicating copy to clipboard operation
express-rate copied to clipboard

Flush out old logs that have expired (MemoryRateHandler)

Open palmerabollo opened this issue 11 years ago • 3 comments

Express-rate should flush out very old remote logs that have expired (redis does that for you).

One alternative is to use node-cache, an in-memory cache with a put = function(key, value, time) function that handles the expiration times for you.

I could implement it if you are interested on it.

palmerabollo avatar Oct 22 '13 20:10 palmerabollo

Yep - I'm happy to take some PR on this!

And in case you're interested :) -- this library needs to be updated with a better code style (using prototypes. Actual method comments, proper error handling, and clearer docs. Here's a recent middleware example I wrote with the style and docs I'm talking about.

I haven't had time to maintain this lib, as we don't actively use it for Segment.io. But if you have some time, I'd love to accept some PRs in this direction.

Thanks!

ivolo avatar Oct 22 '13 23:10 ivolo

@ivolo, do tests pass for you? After cloning the project, installing "expresso", and adding "express" and "should" to the dev dependencies, I run "make test" and I get many errors like this:

memory.test.js Routes can be rate limited, reallowed, and have proper headers: Error: Response not completed: Routes can be rate limited, reallowed, and have proper headers. 
    at Function.assert.response (/Users/guido/.nvm/v0.10.22/lib/node_modules/expresso/bin/expresso:400:17)
    at makeRequest (/private/tmp/express-rate/test/common.rate.js:122:25)
    at /private/tmp/express-rate/test/common.rate.js:177:13
    at MemoryRateHandler._reset_key (/private/tmp/express-rate/lib/memory.js:54:19)
    at BaseRateHandler.reset (/private/tmp/express-rate/lib/base.js:19:10)
    at Object.module.exports.Routes can be rate limited, reallowed, and have proper headers (/private/tmp/express-rate/test/common.rate.js:92:17)
    at Test.module.exports.Routes can be rate limited, reallowed, and have proper headers [as fn] (/private/tmp/express-rate/test/memory.test.js:27:81)
    at Test.runParallel (/Users/guido/.nvm/v0.10.22/lib/node_modules/expresso/bin/expresso:959:10)
    at Test.run (/Users/guido/.nvm/v0.10.22/lib/node_modules/expresso/bin/expresso:924:18)
    at next (/Users/guido/.nvm/v0.10.22/lib/node_modules/expresso/bin/expresso:867:22)

Did you face this before? I don't know why I'm getting "Error: Response not completed" with expresso.

palmerabollo avatar Dec 10 '13 21:12 palmerabollo

BTW you can see the changes I propose here https://github.com/palmerabollo/express-rate/compare/feature;memory_ttl?expand=1 , but I'd like to test it a bit more and make your tests pass.

palmerabollo avatar Dec 10 '13 21:12 palmerabollo