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

Support 'Cache-Control: max-age, s-maxage' and Expires headers #26

Open
eschwartz opened this issue Sep 17, 2015 · 4 comments
Open

Support 'Cache-Control: max-age, s-maxage' and Expires headers #26

eschwartz opened this issue Sep 17, 2015 · 4 comments

Comments

@eschwartz
Copy link

I'm thinking it would be nice if Cacher supported Cache-Control: max-age, Cache-Control: s-maxage, and Expires headers out of the box.

I've added support for this in my own application by overriding the genCacheTtl function, but I could submit this feature as a pull request, if you'd like. Here's what my implementation looks like:

var parseCacheControl = require('parse-cache-control');
var cacher = new Cacher();

cacher.genCacheTtl = function(res, origTtl) {
    if (res.get('Cache-Control')) {
        var cacheControl = parseCacheControl(res.get('Cache-Control'));
        if (cacheControl['s-maxage']) {
            return parseInt(cacheControl['s-maxage']);
        }
        if (cacheControl['max-age']) {
            return parseInt(cacheControl['max-age']);
        }
    }

    if (res.get('Expires')) {
        return (Date.parse(res.get('Expires')) - Date.now()) / 1000;
    }

    return origTtl;
};

I've read just a little bit about cache control headers, and from what I understand the order of precedence goes:

  1. Cache-Control: s-maxage
  2. Cache-Control: max-age
  3. Cache-Control: Expires
@addisonj
Copy link
Owner

addisonj commented Nov 4, 2015

Sorry for the delay on this! I would love to see this as a PR, I will have to take a deeper look to see if this would be considered a 'breaking' change, but perhaps we could just have an opt in option? something like 'respectCacheControl' that would use this function?

@abulaatikko
Copy link

Could you please elaborate because I'm sure I haven't understood something. Why doesn't any of response headers tell when the cache will expire? Shouldn't client get this information to know when it's reasonable to request again to possibly get new data?

I find this information very relevant but yet I don't see any module to support it...

And should the status code be 200 when it didn't hit the cache and 304 when hit?

@eschwartz
Copy link
Author

Shouldn't client get this information to know when it's reasonable to request again to possibly get new data?

The client will receive cache headers, and browser-clients will use info to cache the request on the client side. However, I want caching on my server as well, so I can cache responses between different clients. The example I gave above shows how you can use your existing cache-control response headers to determine server-side caching. Effectively, your server-side and client-side caches will use the same expires time.

And should the status code be 200 when it didn't hit the cache and 304 when hit?

I don't know -- I don't think I've ever used 304 before. It looks to me like 304s are for a special use case (If-Modified-Since conditional requests), which I have never supported.

@abulaatikko
Copy link

However, I want caching on my server as well, so I can cache responses between different clients.

This is exactly my goal as well!

I do get max-age (with or without your code) but not Expires which I think is more relevant (because it tells the exact moment when the data is refreshed - on the other hand this can cause all the clients requesting at the same time which is not good...) Your code presumes that the Expires header is present but I don't have it. Should node-cacher (or Express?) generate the header?

Here is my code if you have time to investigate: https://github.com/lassiheikkinen/liikuntoilu/blob/dbb0b9de27da2730847e7473d921d199dfed5acd/app/backend/server.js

But in the end I can live with the max-age so this is more like a curiosity!

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

3 participants