Incorrect cache-control for stale pages on ISR

[conico974]

Verify canary release

• [X] I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #42~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue Apr 18 17:40:00 UTC 2
    Binaries:
      Node: 16.13.1
      npm: 8.1.2
      Yarn: 1.22.15
      pnpm: 7.20.0
    Relevant packages:
      next: 13.3.5-canary.2
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0

Which area(s) of Next.js are affected? (leave empty if unsure)

Data fetching (gS(S)P, getInitialProps), Standalone mode (output: "standalone")

Link to the code that reproduces this issue

https://github.com/conico974/isr-stale

To Reproduce

Just run next start

Describe the Bug

cache-control header is set to s-maxage=x, stale-while-revalidate when the pages is stale. This cause issue when used with a cdn because the cdn will cache stale data for the duration of revalidate

Expected Behavior

We should set the cache-control header to s-maxage=0, must-revalidate to force the cdn to revalidate stale data.

To solve this we could replace this line https://github.com/vercel/next.js/blob/e76c881d4d4def2e511354b3b69f20762229f63b/packages/next/src/server/base-server.ts#L1952 with

private: isPreviewMode || (is404Page && cachedData) || cachedData?.isStale

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

next start or any standalone output

[NobodysNightmare]

I have read this issue a few days ago and I am thinking about proper caching behaviour myself.

I am still unsure how a proper cache-control header would look like.

Current implementation

The problem with the current implementation (when using a CDN in front of next.js) is that revaldation takes at minimum one revalidation duration longer than expected. Assuming revalidate = 60:

1.CDN makes first request next.js caches 60 seconds, CDN caches 60 seconds

2. after 60 + X seconds CDN makes revalidation request to next.js, receiving a stale response and caching that for another 60 seconds
3. Only the third CDN request (after at least 120 + X seconds) will receive the latest hit from next.js (which might have become stale at that point again, causing another revalidation cycle inside next.js)

However, what will still work nicely right now is that the CDN will protect the next.js application from a storm of requests, because regardless of the number of client requests, the CDN will only request the same page once per revalidation duration.

Proposal from this issue

This issue proposes to respond with Cache-Control: s-maxage=0, must-revalidate while the next.js cache is stale.

I agree that this fixes the issue of taking twice as long to revalidate responses, however, assuming a page with frequent client visits, it will also result in a request surge every revalidate duration.

While the CDN will shield the next.js application for the revalidation duration, the next hit from the CDN to next.js is guaranteed to be a stale response (next.js did not receive any intermediate requests from the CDN), so the CDN will stop caching and for each client request, it will make a request to nextjs, until nextjs responds with a s-maxage=x again.

This sounds very undesirable as well.

[NobodysNightmare]

Honestly, one possible way out could be to document this behaviour more clearly and if you are concerned that 2 * x is a too high number, then you can still reduce the value for x.

[conico974]

You're right that my solution is definitely not ideal. One of the other issue here is https://github.com/vercel/next.js/issues/51823.

There is something else that we could do, setting cache-control to s-maxage=x, stale-while-revalidate=2592000 where x is the remaining revalidation time, maybe a with a minimum of 1. This way the CDN is still shielding your app, while not keeping stale data for too long.

This would also solve another issue with CDN caching, potential difference between client side routing and server side routing. For example for page dir you could have stale data for json but already revalidated data for the html page (It's even worse for app dir since for rsc component, next-router-state-tree headers are also used for CDN caching)

I might open a PR to do this, just need to figure out how to test this properly.

[alivtar]

We were facing the same issue in our home page and fixed the problem by creating a rule in our CDN provider to bypass the cache of the home page. and it works. Simply create a rule in CDN provider to do NOT cache the html of that page.