Prevent caching page with 304 status

[gffuma]

I think that sometimes when a revalidation happens from a request with caching headers this causing the 304 status to be cached.

This PR ensures the 304 from an initial response doesn't affect a background revalidation.

Fixes: #56580

[gffuma]

@ijjk i understand that is not a super elegant solution, an alternative could be strip all client caching headers ex If-None-Match before render the request used to fill the response cache. I don't have enought confidence of the codebase to implement it but could be an idea.
BTW This is a seriois problem that prevent using static rendering in production.

[ijjk]

Hi could we add a failing test case for this showing this resolves the issue properly?

[gffuma]

@ijjk i added a test that match the original issue race condition. It's a little bit complicated but i can't find a more clear way to emulate the race condition. Let me known if it's ok 😄

[ijjk]

Stats from current PR

Default Build

General Overall increase ⚠️

	vercel/next.js canary	gffuma/next.js fix-304-cache	Change
buildDuration	10.7s	10.6s	N/A
buildDurationCached	6.1s	6s	N/A
nodeModulesSize	175 MB	175 MB	⚠️ +1.06 kB
nextStartRea..uration (ms)	424ms	422ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	gffuma/next.js fix-304-cache	Change
199-HASH.js gzip	30 kB	30 kB	N/A
3f784ff6-HASH.js gzip	53.2 kB	53.2 kB	✓
494.HASH.js gzip	182 B	182 B	✓
framework-HASH.js gzip	45.5 kB	45.5 kB	✓
main-app-HASH.js gzip	253 B	252 B	N/A
main-HASH.js gzip	33.1 kB	33.1 kB	N/A
webpack-HASH.js gzip	1.75 kB	1.75 kB	N/A
Overall change	98.9 kB	98.9 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	gffuma/next.js fix-304-cache	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	gffuma/next.js fix-304-cache	Change
_app-HASH.js gzip	205 B	205 B	✓
_error-HASH.js gzip	182 B	181 B	N/A
amp-HASH.js gzip	505 B	507 B	N/A
css-HASH.js gzip	322 B	323 B	N/A
dynamic-HASH.js gzip	2.59 kB	2.59 kB	N/A
edge-ssr-HASH.js gzip	258 B	259 B	N/A
head-HASH.js gzip	348 B	347 B	N/A
hooks-HASH.js gzip	369 B	368 B	N/A
image-HASH.js gzip	4.38 kB	4.38 kB	N/A
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	2.67 kB	2.67 kB	N/A
routerDirect..HASH.js gzip	318 B	318 B	✓
script-HASH.js gzip	384 B	383 B	N/A
withRouter-HASH.js gzip	319 B	320 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	885 B	885 B	✓

Client Build Manifests

	vercel/next.js canary	gffuma/next.js fix-304-cache	Change
_buildManifest.js gzip	484 B	484 B	✓
Overall change	484 B	484 B	✓

Rendered Page Sizes

	vercel/next.js canary	gffuma/next.js fix-304-cache	Change
index.html gzip	529 B	528 B	N/A
link.html gzip	541 B	543 B	N/A
withRouter.html gzip	525 B	524 B	N/A
Overall change	0 B	0 B	✓

Edge SSR bundle Size

	vercel/next.js canary	gffuma/next.js fix-304-cache	Change
edge-ssr.js gzip	96.1 kB	96.1 kB	N/A
page.js gzip	140 kB	140 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	gffuma/next.js fix-304-cache	Change
middleware-b..fest.js gzip	627 B	627 B	✓
middleware-r..fest.js gzip	148 B	151 B	N/A
middleware.js gzip	23 kB	23 kB	N/A
edge-runtime..pack.js gzip	1.92 kB	1.92 kB	✓
Overall change	2.55 kB	2.55 kB	✓

Diff details

Diff for page.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Commit: f96c4ae

[ijjk]

Thanks for adding that, reworked the fix to handle more cases but should be good now as your added test is passing!

[ijjk]

Tests Passed

[mario-mestrovic]

Thanks for adding that, reworked the fix to handle more cases but should be good now as your added test is passing!

@ijjk Just thanks? You had this issue for almost 2 months now and completely ignored it not even acknowledge it exists and even issued a new 'major' along the way.
Sorry but Vercel engineering obviously has ridiculous priorities.

Thanks @gffuma you kinda saved us! You deserve 10 beers at least for this :)

[Emiliano-Bucci]

@gffuma @mario-mestrovic @ijjk Sorry guys, I've just updated next to latest version (14.0.2) which contains this fix but it seems that the issue is still here; could you please verify if you can reproduce?

[smuk3c]

Still broken for me as well