rv-kip / express-redis-cache Goto Github PK

Given the following simple example...

var express = require('express');
var cache = require('express-redis-cache')();

var app = express();

app.get('/', cache.route(), function(req, res) {
  res.json(req.query);
});

app.listen(9999);

a request to /?param=1 returns { param: 1 }

a subsequent request to /?param=2 also returns { param: 1 }

Given the nature of query strings changing the returned data, is this the expected behavior or am I using this library wrong?

In route.js in the following code:

name = name || res.expressRedisCacheName || req.originalUrl;

name is getting assigned on the first call and caching is done with that key for the express route from then on.

Hoping you could help me or let me know if I can submit a pull request...

I made a simple POC with "express": "~4.9.0", and add following code
app.get("/", reqcache.route({ expire: 60}), function(req, res, next) {,,,}

First response is fine as it comes from my own code, but second request which is supposed to give cached response gives '[object Object]'

Our app needs the ability to set a dynamic cache prefix. However, whenever I call cache.route({prefix: 'blah'}) and check what prefix is being utilized, it's always 'erc:'. How can I get cache.route to correctly set a prefix?

Thanks!

events.js:72
throw er; // Unhandled 'error' event
^
TypeError: Cannot set property 'name' of null
at Command. (/home/ec2-user/server/api/node_modules/express-redis-cache/lib/ExpressRedisCache/get.js:45:27)
at Command.b (domain.js:177:20)
at Command.b as callback
at normal_reply (/home/ec2-user/server/api/node_modules/redis/index.js:714:21)
at RedisClient.return_reply (/home/ec2-user/server/api/node_modules/redis/index.js:816:9)
at JavascriptRedisParser.Parser.returnReply (/home/ec2-user/server/api/node_modules/redis/index.js:188:18)
at JavascriptRedisParser.execute (/home/ec2-user/server/api/node_modules/redis/node_modules/redis-parser/lib/parser.js:413:12)
at Socket. (/home/ec2-user/server/api/node_modules/redis/index.js:267:27)
at Socket.emit (events.js:95:17)
at Socket. (stream_readable.js:765:14)
at Socket.emit (events.js:92:17)
at emitReadable (_stream_readable.js:427:10)
at emitReadable (_stream_readable.js:423:5)
at readableAddChunk (_stream_readable.js:166:9)
at Socket.Readable.push (_stream_readable.js:128:10)
at TCP.onread (net.js:529:21)

JS

cache.size()

CLI

express-redis-cache size

Thanks for making this library! Really great stuff.

I'm running into an issue though where I have an Express param that is bound to a method that loads some data prior to the route

app.route('/user/:userId')
  .get(cache.route(), controller.getUserById)

app.param('userId', controller.loadUser);

What I'm seeing is that even though the route is cached it still runs the param method which loads the data from the database and slows down the route.

Any ideas how to get around this? Thanks.

I'm having issues with conditional caching. I've tried placing the middleware step to define
res.use_express_redis_cache both before all of my routes, and as a callback before cache.route in a GET request. Neither of these seem to prevent the responses being returned from Redis' cache.

Examples:
As prior middleware:

app.use(function(req, res, next) {
  res.use_express_redis_cache = false;
  next();
});
app.route(...)

In the get request:

app.route('/offers')
  .get(function(req, res, next) {
    res.use_express_redis_cache = false;
    next();
}, cache.route(ms('5ms')), ... )

Upon testing both of these, the page is clearly still caching requests to /offers (checking in redis-cli MONITOR)

Am I missing something here?

It seems that express-redis-cache doesn't cache the data if res is used as a pipe (res.send() is never called).

For example:

app.get('/test/:id', cache.route({ expire: 3600 }), function (req, res, next) {
    request({ method: req.method, uri: BASE_URL + req.url }).pipe(res);
});

When constructing a new express-redis-cache with a default expire, this default expire should be applied to all the routes that did not define an expire. Also, there is no test for that.

It is question not issue,what we are achieving from performance perspective by caching route into redis?
I got this question because I am working on application & I am caching route.
So during development,it becomes tedious to debug as code is getting cache in redis.

would be nice to be able to toggle this using an environment var. I'd be happy to provide a pull request if there's interest.

I'd like to not cache 403/404s...interest? if so, what do you have a preference for how you'd like the options to be specified (var names etc?)

I'm not sure of an easy fix, which is why I've no PR to go along with this report, unfortunately. But the problem itself is hopefully easy enough to explain. Here's my attempt:

When trying to run the test suite via npm test, I noticed the tests never completes, but there are no errors reported.

I traced the faulty path of execution from test.js -> test/route.js -> lib/route.js -> lib/get.js

It turns out that name in lib/route.js is left undefined, and so lib/get.js throws an error:

    if ( typeof name !== 'string' ) {
      throw new Error('Missing name of cache to get');
    }

That error is caught in lib/route.js in the domain.on('error') block:

  domain.on('error', function (error) {
    throw error;
  });

So lib/route.js attempts to re-throw the error from lib/get.js, BUT, that error is never caught or reported.

You can verify this issue by adding a line to that error handler in lib/route.js that spits out some information to the console, as I've done locally:

  domain.on('error', function (error) {
    console.error("Caught error in lib/route.js", error);
    throw error;
  });

Then run the test suite again and you'll see the error logged.

Caught error in lib/route.js { [Error: Missing name of cache to get]
  domain:
   { domain: null,
     _events: { error: [Function] },
     _maxListeners: 10,
     members: [] },
  domainThrown: false,
  domainBound: [Function] }

The reason the process "hangs" is because the then() in test/route.js is never called:

            /** Emulate res **/
            var res = {
              send: function (body) {
                assert('it should be the same message', body === entry.body);

                then(); // <- THIS NEVER GETS CALLED
              }
            };

In summary:

• A test is failing due to the 'name' in lib/route.js being undefined.
• That failure is not being captured/reported by the test suite.
• The test suite hangs because a promise in test/route.js is never resolved.

Other thoughts (nice-to-haves):

• It'd be nice to use a more structured test harness like Mocha that has built-in timeouts for hanging async tests, etc.
• It'd be nice to be able to run individual test scripts one-by-one rather than having to run test/test.js which in turn runs all the tests.

I'm new Express, and just created a basic app and trying to integrate the Redis cache for Express from npm

When I install and add the module in the app, and run the appp it gives the error described on the screenshot.

Express Ver: 4.14.0

Hey, I noticed res.use_express_redis_cache only disables storing to the cache, but not getting from it. Wouldn't it make sense to disable read and write?

I'm looking for a solution to completely disable caching if in debug.

Without the Cache-Control header, browsers will make unnecessary calls to the server to get cached data. If the browser is aware of the cache duration, it can avoid calling the server at all until the cache is expired. This could be done by adding a header like this:

res.setHeader('Cache-Control', 'max-age=' + options.expire + '000');

When I run the npm install express-redis-cache command I get the following error and no directory is created in my node_modules.

npm WARN package.json [email protected] No description
npm WARN package.json [email protected] No repository field.
npm WARN package.json [email protected] No README data
npm WARN install Refusing to install express-redis-cache as a dependency of itself

Hi,

I would like to create a new entry when there is a difference in the header. For example for X-Requested-With or if a cookie is present. Is this something I can do using this module?

version 0.1.8

$ express-redis-cache size
/usr/local/lib/node_modules/express-redis-cache/bin/express-redis-cache.js:14
    throw error;
    ^

TypeError: callback is not a function
    at Domain.<anonymous> (/usr/local/lib/node_modules/express-redis-cache/lib/ExpressRedisCache/size.js:21:5)
    at emitOne (events.js:77:13)
    at Domain.emit (events.js:169:7)
    at emitError (domain.js:65:24)
    at Domain.errorHandler [as _errorHandler] (domain.js:119:16)
    at process._fatalException (node.js:229:33)

If redis crashes, this middleware throws an error and crashes. Ideally, it would simulate a cache miss and let the express layer serve non-cached data. A bunch of error emits would be good too.

Thanks for a great module. The documentation indicates errors should just be emitted and pass through to the route... which seems true if node couldn't connect to Redis at all on startup. But, if connectivity is lost after initial connection is made... the response is never returned. Here is my code (I pass around the exported cache objects as middleware to the routes in server.js):

'use strict';

var redis = require('redis'),
  constants = require('../constants/constants'),
  logger = require('../utils/logger');

var redisClient;
redisClient = redis.createClient(constants.redisPort, constants.redisHost);
redisClient.auth(constants.redisAuth);

redisClient.on('connect', function () {
  logger.info('Connected to redis... ');
});
redisClient.on('error', function (err) {
  logger.error('Error connecting to redis: ' + err);
});

var _redisCache = require('express-redis-cache')({ prefix: constants.env, client: redisClient, expires: 15 });

_redisCache.on('connected', function () {
  logger.info('Cache is functional...');
});
_redisCache.on('error', function (err) {
  logger.error('Cache error: ' + err);
});

exports.tenSecondCache = _redisCache.route(10);
exports.oneMinuteCache = _redisCache.route(60);
exports.fifteenMinuteCache = _redisCache.route(60*15);

It seems that when using a Redis client, once it has problems the error that's emitted to the cache error doesn't allow it to pass through? I can replicate this locally by cutting off network connectivity after the initial connection. Is there a way I can let it fall through in this case?

As explained here https://github.com/rv-kip/express-redis-cache#content-type I tried to use the type config to force content type.

But it doesn't work : in fact I've searched the code to understand where it's used, and it seems to be never use. I think it should be use there : https://github.com/rv-kip/express-redis-cache/blob/master/lib/ExpressRedisCache/route.js#L186

Thanks a lot in advance,

Ghislain

I'll maintain the project if you want to transfer to me.

See here:

https://github.com/co2-git/express-redis-cache/blob/master/bin/express-redis-cache.js#L164

Notice the following text near the end:

Dildo for --prefix <prefix>

😮

I need to cache entries until midnight. This constraint can be accomplished using the redis command expireat, however it doesn't seem to be currently available in express-redis-cache. Is there a way to accomplish that constraint with the current release?

version 0.1.8

$ express-redis-cache ls
/usr/local/lib/node_modules/express-redis-cache/bin/express-redis-cache.js:14
    throw error;
    ^

TypeError: cache.ls is not a function
    at Domain.<anonymous> (/usr/local/lib/node_modules/express-redis-cache/bin/express-redis-cache.js:178:15)
    at Domain.run (domain.js:221:14)
    at /usr/local/lib/node_modules/express-redis-cache/bin/express-redis-cache.js:17:10
    at Object.<anonymous> (/usr/local/lib/node_modules/express-redis-cache/bin/express-redis-cache.js:270:3)
    at Module._compile (module.js:425:26)
    at Object.Module._extensions..js (module.js:432:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Function.Module.runMain (module.js:457:10)
    at startup (node.js:136:18)

TypeError: Cannot set property 'name' of null
at Command. (/home/mdladmin/source/web-chat/JavaScript/node_modules/express-redis-cache/lib/ExpressRedisCache/get.js:45:27)
at intercepted (domain.js:258:14)
at Command.runIntercepted (domain.js:272:12)
at bound (domain.js:287:14)
at Command.runBound (domain.js:300:12)
at RedisClient.return_reply (/home/mdladmin/source/web-chat/JavaScript/node_modules/express-redis-cache/node_modules/redis/index.js:664:25)

It would be useful to support multiple databases on redis instances. The database option would be an integer, 0-based, with a default of 0.

http://www.rediscookbook.org/multiple_databases.html

Hi! The README suggests that redis unavailability won't crash the app: "Should the redis become unavailable, the express-redis-cache object will emit errors but will not crash the app. Express.js requests during this time will be bypass cache and will return fresh data."

Unfortunately, I'm not seeing this -- when I stop my local redis from running, I get

cache { [Error: Redis connection to localhost:6379 failed - connect ECONNREFUSED]
  code: 'ECONNREFUSED',
  errno: 'ECONNREFUSED',
  syscall: 'connect' }

I am calling it like so, from an external (not in app.js) routes file:

//cache
var cache = require('express-redis-cache')({
  host: config.redisHost, port: config.redisPort
  });

cache.on('error', function(error){
  console.error("cache", error);
});
...
routes.get('/fragments/flickr/:word', cache.route({expire: 86400}), fragments.flickr);

Any idea how to handle an unavailable redis? The graceful failure was one of the main selling points of express-redis-cache. :)

When setting up the cache with

var cache = require('express-redis-cache')({ expire: 60 });

The expire value of the entry is always undefined and the item never expired from the cache. The unit test for this always succeeds since the value being checked is always undefined

it ( ' - entry which has a property expire which equals cache.expire', function () {
  should(entry.expire).equal(cache.expire);
});

Expiration does work when specified this way

cache.route("test", 300)

Inside of get.js, line 43, hgetall is used to get a value from redis. It doesn't handle the case when the key doesn't exist. If the key doesn't exist then result is null which results in the below error being thrown. It looks like keys is used to check if the key exists but it could expire or be deleted in between the time that keys is called and hgetall is called.

TypeError: Cannot set property 'name' of null
    at Command.<anonymous> (/Users/dustinmartin/Projects/api/node_modules/express-redis-cache/lib/ExpressRedisCache/get.js:45:27)
    at intercepted (domain.js:251:14)
    at Command.runIntercepted (domain.js:265:12)
    at Command.nrWrappedHandler (/Users/dustinmartin/Projects/api/node_modules/newrelic/lib/transaction/tracer/index.js:398:19)
    at Command.wrapped (/Users/dustinmartin/Projects/api/node_modules/newrelic/lib/transaction/tracer/index.js:182:28)
    at Command.wrappedCallback (/Users/dustinmartin/Projects/api/node_modules/newrelic/lib/transaction/tracer/index.js:450:66)
    at Command.wrapped (/Users/dustinmartin/Projects/api/node_modules/newrelic/lib/transaction/tracer/index.js:182:28)
    at bound (domain.js:280:14)
    at Command.runBound (domain.js:293:12)
    at normal_reply (/Users/dustinmartin/Projects/api/node_modules/redis/index.js:662:21)
    at RedisClient.return_reply (/Users/dustinmartin/Projects/api/node_modules/redis/index.js:722:9)
    at JavascriptReplyParser.RedisClient.reply_parser.Parser.returnReply (/Users/dustinmartin/Projects/api/node_modules/redis/index.js:146:18)
    at JavascriptReplyParser.run (/Users/dustinmartin/Projects/api/node_modules/redis-parser/lib/javascript.js:137:18)
    at JavascriptReplyParser.execute (/Users/dustinmartin/Projects/api/node_modules/redis-parser/lib/javascript.js:112:10)
    at Socket.<anonymous> (/Users/dustinmartin/Projects/api/node_modules/redis/index.js:223:27)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:188:7)
    at readableAddChunk (_stream_readable.js:172:18)
    at Socket.Readable.push (_stream_readable.js:130:10)
    at TCP.onread (net.js:542:20)
[nodemon] app crashed - waiting for file changes before starting...

events.js:72
throw er; // Unhandled 'error' event
^
TypeError: Cannot set property 'name' of null
at /var/www/mobile2/node_modules/express-redis-cache/lib/ExpressRedisCache/get.js:45:27
at b (domain.js:177:20)
at b (domain.js:183:18)
at try_callback (/var/www/mobile2/node_modules/redis/index.js:592:9)
at RedisClient.return_reply (/var/www/mobile2/node_modules/redis/index.js:685:13)
at ReplyParser. (/var/www/mobile2/node_modules/redis/index.js:321:14)
at ReplyParser.emit (events.js:95:17)
at ReplyParser.send_reply (/var/www/mobile2/node_modules/redis/lib/parser/javascript.js:300:10)
at ReplyParser.execute (/var/www/mobile2/node_modules/redis/lib/parser/javascript.js:211:22)
at RedisClient.on_data (/var/www/mobile2/node_modules/redis/index.js:547:27)
error: Forever detected script exited with code: 7
error: Script restart attempt #2

Now the default cache entry name is req.path, sometimes will make mistakes.

for example:

i define route in this way:

app.use "/day", require "./routes/day"
app.use "/story", require "./routes/story"

in /routes/day the req.path maybe is /123, and in /routes/story the req.path also can be /123.

so i suggust use req.originalUrl to replace it.

The issues lies in add.js line 48

      /** The new cache entry **/
      var entry = {
        body: body,
        type: options.type || config.type,
        touched: +new Date(),
        expire: options.expire || self.expire
      };

If options.expire is 0, it eval's as a falsey value and uses self.expire instead. It should allow expire to be 0.
Creating a patch right now, will make a PR.

Is a middleware for express, why npm install -g express-redis-cache ?

Hey there,

In our production environment this was pretty slow (https://s.put.re/4ueAZEA.png).
You can read more details about .keys under: http://redis.io/commands/keys

On this screenshot you can see our t2.small redis cache instance cpu usage on amazon, take a look at 2:50. That's where I replaced your caching lib with a custom one that uses .get (https://s.put.re/f36CV4z.png).

Maybe your usage of .keys has a deeper point I don't see here, but let me quote the redis docs:

Warning: consider KEYS as a command that should only be used in production environments with extreme care. It may ruin performance when it is executed against large databases. This command is intended for debugging and special operations, such as changing your keyspace layout. Don't use KEYS in your regular application code.

greetz

If for example "500" is cached, response retrieved from redis is served with "200" response code.

Hi,

Is it possible to switch off caching of error responses? Setting res.use_express_redis_cache=false in error handler has no effect because it is called after cache.route().

/home/faceair/zhihudaily/node_modules/express-redis-cache/node_modules/redis/index.js:575
throw callback_err;
^
Error: OOM command not allowed when used memory > 'maxmemory'.
at ReplyParser. (/home/faceair/zhihudaily/node_modules/express-redis-cache/node_modules/redis/index.js:317:31)
at ReplyParser.emit (events.js:95:17)
at ReplyParser.send_error (/home/faceair/zhihudaily/node_modules/express-redis-cache/node_modules/redis/lib/parser/javascript.js:296:10)
at ReplyParser.execute (/home/faceair/zhihudaily/node_modules/express-redis-cache/node_modules/redis/lib/parser/javascript.js:181:22)
at RedisClient.on_data (/home/faceair/zhihudaily/node_modules/express-redis-cache/node_modules/redis/index.js:547:27)
at Socket. (/home/faceair/zhihudaily/node_modules/express-redis-cache/node_modules/redis/index.js:102:14)
at Socket.emit (events.js:95:17)
at Socket. (stream_readable.js:764:14)
at Socket.emit (events.js:92:17)
at emitReadable (_stream_readable.js:426:10)
at emitReadable (_stream_readable.js:422:5)
at readableAddChunk (_stream_readable.js:165:9)
at Socket.Readable.push (_stream_readable.js:127:10)
at TCP.onread (net.js:528:21)

I install this in my vps, and throw this error, what's wrong ?

Hi,

If redis isn't available the app crashes with:

Error: Redis connection to 127.0.0.1:6379 failed - connect ECONNREFUSED

you require redis in index.js in Line 77, but you don't add redis to package.json

Hi,

An initial request with Accept:text/html header, responding with a Content-Type:text/html, then a subsequent request to the same route with Accept:application/json, responding with Content-Type:application/json doesn't create a new entry.

There is a bug on:
https://github.com/rv-kip/express-redis-cache/blob/master/lib/ExpressRedisCache/route.js#L66
https://github.com/rv-kip/express-redis-cache/blob/master/lib/ExpressRedisCache/route.js#L102

Neither __fileName, nor __line, exist at all. The first one should be __filename and the __line doesn't exist. Check nodejs magic globals: https://nodejs.org/api/globals.html

Use expressRedisCacheName deprecated attribute to check. (Note you are using a domain.)

Thanks for your code.

Hi everybody!

I'd better start giving thanks to co2 for your code. It has saved me a lot of time. Thanks!
I just want to report that the npm version of this library is not the latest and it doesn't give all its features (like using the right content-type of the cached element).

have a nice day!

The README says:

var cache = require('express-redis-cache')({
    host: String, port: Number
    });

But index.js sets client like so:

var client = options.client || require('redis').createClient(options.port);

So it pays attention to port but not host.

I'll attach a pull-request (hopefully) before too long.

Of course the easy workaround is to just build your own redis client and pass that to the constructor. That is certainly reasonable but either the README or the Code should be updated so that they are in sync.

Thanks

When the cache expires,return the cache first,and then restore the cache asynchronously.In this way,the user can aways take a response quickly.Thank you~

If my route returns a binary file, the cache seems to simply ignore it, as far as I can tell.

This module was probably written to target caching of dynamic content, not static content (which most binaries are served as). Yet there are two use cases where you might want to have caching for binary files:

• if the file is generated server side, through some heavy operation.
• if the file is obtained through a http proxy

It should be easy to do with a little buffer.toString("hex") but the module doesn't allow to transform the response before it stores it and after it retrieves it.

You've got a wonderful project here, but I notice results returned from the cache are always sent as 200, even if the original response is a 501 (Not Implemented) or what have you.

At first glance, it wouldn't be too hard to include a field in your entry model to include the original response.

Would you be open to accepting a pull request if I built out this feature?