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

Flush out old logs that have expired (MemoryRateHandler) #3

Open
palmerabollo opened this issue Oct 22, 2013 · 3 comments
Open

Flush out old logs that have expired (MemoryRateHandler) #3

palmerabollo opened this issue Oct 22, 2013 · 3 comments

Comments

@palmerabollo
Copy link

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.

@ivolo
Copy link
Owner

ivolo commented Oct 22, 2013

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!

@palmerabollo
Copy link
Author

@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
Copy link
Author

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.

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

No branches or pull requests

2 participants