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

Caching: stale-if-error rfc5861 #1059

Closed
1 task done
sithmel opened this issue Feb 9, 2020 · 8 comments
Closed
1 task done

Caching: stale-if-error rfc5861 #1059

sithmel opened this issue Feb 9, 2020 · 8 comments

Comments

@sithmel
Copy link

sithmel commented Feb 9, 2020

What problem are you trying to solve?

Fallback on stale cached content when the server is down or returns an error (500)

Describe the feature

https://tools.ietf.org/html/rfc5861
https://docs.fastly.com/en/guides/serving-stale-content

Checklist

  • I have read the documentation and made sure this feature doesn't already exist.
@sithmel
Copy link
Author

sithmel commented Feb 9, 2020

I think this change would be probably enough to have the feature here

@marcoreni
Copy link

Hey @szmarczak @sithmel has this been implemented?

In the docs I see that stale entries are revalidated, but I don't see anything about stale-if-error / stale-while-revalidate.

@szmarczak
Copy link
Collaborator

szmarczak commented Aug 31, 2020

@sithmel
Copy link
Author

sithmel commented Sep 1, 2020

Hi,
sorry for dropping the ball half way.

The change to http-cache-semantic has been released.

I was about to follow with a change to cacheable-request. But I never finished the work.

@marcoreni @szmarczak let me know if you are interested in picking this up. I can give some direction on how I was planning to do it

@szmarczak
Copy link
Collaborator

The change to http-cache-semantic has been released.

So this is finished, no?

I was about to follow with a change to cacheable-request. But I never finished the work.

Hmm... What is left to do here?

I can give some direction on how I was planning to do it

Sure, that would be great.

@sithmel
Copy link
Author

sithmel commented Sep 14, 2020

The code works but I think it should be possible to log or "do something" when the server return 500 or times out, and we are using the stale content.

Here's my commit to http-cache-semantics kornelski/http-cache-semantics@1b35980

In case of errors, if the use-stale-if-error header is used, the revalidatedPolicy returns

        if(this._useStaleIfError() && isErrorResponse(response)) {  // I consider the revalidation request unsuccessful
          return {
            modified: false,
            matches: false,
            policy: this,
          };

https://github.com/kornelski/http-cache-semantics/blob/1b359803db768e0b3f5f8051aa3ea50e27c8d096/index.js#L584

  • modified: false : you can reuse the cached value
  • matches: false: the cached value doesn't match

The code in cacheable-request uses the modified flag and correctly uses the cache https://github.com/lukechilds/cacheable-request/blob/master/src/index.js#L86

The missing part in cacheable-request is that it should also fire an error event, so that you should be able to detect when the server is throwing an error.
This can be tricky as it can potentially break the code used to convert the event-based request to promise. Perhaps it could be necessary use a distinct event.

In got you should be able to control this behaviour, but I didn't go as far as figuring that out yet.

I hope this helps

@szmarczak
Copy link
Collaborator

The code works but I think it should be possible to log or "do something" when the server return 500 or times out, and we are using the stale content.

Well. If it gives us 500 and it is valid to reuse the cached response then no error should be emitted.

@sithmel
Copy link
Author

sithmel commented Sep 14, 2020

I don't think it does @szmarczak https://github.com/lukechilds/cacheable-request/blob/master/src/index.js#L89
It reuses the status code of the stale response. And I think this is correct

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