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

Apply stale-while-revalidate also for responses without a validator #128

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented May 26, 2020

Due to its name, the stale-while-revalidate Cache-Control extension might suggest that it only applies to revalidation requests. However, looking at https://tools.ietf.org/html/rfc5861#section-3...

  1. The stale-while-revalidate Cache-Control Extension
    When present in an HTTP response, the stale-while-revalidate Cache-Control extension indicates that caches MAY serve the response in which it appears after it becomes stale, up to the indicated number of seconds.
    stale-while-revalidate = "stale-while-revalidate" "=" delta-seconds
    If a cached response is served stale due to the presence of this extension, the cache SHOULD attempt to revalidate it while still serving stale responses (i.e., without blocking).

... I see no reason why a cache should not also return a stale response while a complete re-fetch happens in the background (just what would happen if validation fails).

This makes a difference if, for example, a resource has Cache-Control: public, max-age=30, stale-while-revalidate=30 set and no additional Last-Modified or ETag headers.

So my suggestion is to re-arrange the conditions slightly. If the client bothers to specify a maximum stale age, we should respect it.

@mpdude
Copy link
Contributor Author

mpdude commented Jul 3, 2020

ping @Kevinrob

@Kevinrob
Copy link
Owner

Thank @mpdude for this PR!
I will take some times for checking it 👍

Can you please rebase or merge master into your branch? I have adapted the Github Action for running tests on PR too 👍

@mpdude mpdude force-pushed the stale-while-revalidate-background-fetch branch from 7558984 to 9913c15 Compare September 8, 2020 11:31
@mpdude
Copy link
Contributor Author

mpdude commented Sep 8, 2020

@Kevinrob rebased, waiting for your feedback.

@mpdude
Copy link
Contributor Author

mpdude commented Sep 8, 2020

Obviously I'll need to update the tests to reflect changed behaviour; will do once we basically agree.

@Kevinrob
Copy link
Owner

Kevinrob commented Jul 4, 2021

Obviously I'll need to update the tests to reflect changed behaviour; will do once we basically agree.

I think the logic is good like that! Can you adapt the tests?

@Kevinrob
Copy link
Owner

@mpdude Would you like to rebase this PR and adapt tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants