alltherooms / cached-request

Node.js module to perform HTTP requests with caching support
MIT License
61 stars 23 forks source link

Set ttl and cache not working #11

Closed FrenchBen closed 8 years ago

FrenchBen commented 8 years ago

using 1.0.0 I can't seem to get my requests to cache. I have the following at the start of my file:

var cachedRequest = require('cached-request')(request);
var cacheDirectory = '/tmp/cache';
cachedRequest.setCacheDirectory(cacheDirectory);
cachedRequest.set('ttl', 3000);

When starting my app, I get cachedRequest.set is not a function although this is in the docs. Looking at the object it doesn't indeed have a set method, but does have a setValue docs need to be updated?

Secondly I declare, as seen above, the cacheDirectory but keep getting the following error:

Error: ENOENT: no such file or directory, open '/tmp/cache/1275300535.json'

Looking at the code: https://github.com/alltherooms/cached-request/blob/master/lib/cached-request.js#L164

It seems that there is no check in place to see if the folder should be created - a simple mkdir:

//Open headers file
  mkdirp(self.cacheDirectory);
  headersReader = fs.createReadStream(this.cacheDirectory + key + '.json');
...
//Open the response file
  mkdirp(self.cacheDirectory);
  responseReader = fs.createReadStream(self.cacheDirectory + key);

would prevent an anti-pattern and make sure that the dir exists.

danypype commented 8 years ago

Hi @FrenchBen. This was a documentation failure. Use cachedRequest.setValue('ttl', 3000). I have updated the docs. Thank you!

FrenchBen commented 8 years ago

@danypype What about the mention of the mkdir not happening for the new cacheDirectory?

danypype commented 8 years ago

Does the user you're launching the app with, have write access to /tmp?

FrenchBen commented 8 years ago

indeed it does - If I add the 'mkdirp' above, it all works without any issue. Are you able to use cache when no directory is created on your end?

danypype commented 8 years ago

You can't use cache if the cache directory is not created. I will look further into this.

danypype commented 8 years ago

@FrenchBen try creating the directory yourself, and then launch the app, just to see if you get the same error opening the headers file.

danypype commented 8 years ago

Looking at the code I remembered that the directory isn't created by cached-request at any time. I will label this as a feature suggestion and implement it. Thank you!

danypype commented 8 years ago

So Error: ENOENT is actually thrown when writing the headers file.

danypype commented 8 years ago

@FrenchBen also feel free to send a pull request. :)

FrenchBen commented 8 years ago

@danypype Yup same error I got. Sure can - didn't know if you wanted to add a dependency for mkdirp