fix: status code for 404 props queries to avoid client side navigation with empty props #60968

Lezzio · 2024-01-22T12:20:29Z

What?

Currently, when a middleware is active and the path results in a 404, the server responds with a 200 status code to data requests. We propose a change to ensure that a 404 status code is returned in these situations, as expected, solving production issues for the navigation router.

Why?

The client data requests (identified with _next/data path & x-nextjs-data header) get answered the status code 200. The code below is executed when there is a version skew for the data requests (e.g prefetch in production).

next.js/packages/next/src/server/lib/router-server.ts

Lines 217 to 227 in 4125069

    
           if ( 
        
             req.headers['x-nextjs-data'] && 
        
             fsChecker.getMiddlewareMatchers()?.length && 
        
             removePathPrefix(invokePath, config.basePath) === '/404' 
        
           ) { 
        
             res.setHeader('x-nextjs-matched-path', parsedUrl.pathname || '') 
        
             res.statusCode = 200 
        
             res.setHeader('content-type', 'application/json') 
        
             res.end('{}') 
        
             return null 
        
           }

The version skew consists in the client side version identified with the buildId in __NEXT_DATA__ tag to be obsolete compared to the server version (BUILD_ID file).

In the case of prefetching, this leads to the code above being executed, therefore the prefetch-reducer.ts handles the response as valid and sets it in its cache. Which ultimately triggers a navigation with empty prop, resulting in erroneous behaviours reported in issues and in our production websites:

next.js/packages/next/src/client/components/router-reducer/reducers/prefetch-reducer.ts

Lines 54 to 74 in 4125069

    
           // fetchServerResponse is intentionally not awaited so that it can be unwrapped in the navigate-reducer 
        
           const serverResponse = prefetchQueue.enqueue(() => 
        
             fetchServerResponse( 
        
               url, 
        
               // initialTree is used when history.state.tree is missing because the history state is set in `useEffect` below, it being missing means this is the hydration case. 
        
               state.tree, 
        
               state.nextUrl, 
        
               state.buildId, 
        
               action.kind 
        
             ) 
        
           ) 
        
           // Create new tree based on the flightSegmentPath and router state patch 
        
           state.prefetchCache.set(prefetchCacheKey, { 
        
             // Create new tree based on the flightSegmentPath and router state patch 
        
             treeAtTimeOfPrefetch: state.tree, 
        
             data: serverResponse, 
        
             kind: action.kind, 
        
             prefetchTime: Date.now(), 
        
             lastUsedTime: null, 
        
           })

By switching the response to a 404, we trigger this code in the fetch (fetchServerResponse). This change prompts a hard navigation (mpaNavigation), effectively refreshing the client version and resynching it with the server version.

next.js/packages/next/src/client/components/router-reducer/fetch-server-response.ts

Lines 125 to 134 in 4125069

    
           // If fetch returns something different than flight response handle it like a mpa navigation 
        
           // If the fetch was not 200, we also handle it like a mpa navigation 
        
           if (!isFlightResponse || !res.ok) { 
        
             // in case the original URL came with a hash, preserve it before redirecting to the new URL 
        
             if (url.hash) { 
        
               responseUrl.hash = url.hash 
        
             } 
        
             return doMpaNavigation(responseUrl.toString()) 
        
           }

How?

We simply update the status code to 404 here:

next.js/packages/next/src/server/lib/router-server.ts

Line 223 in 4125069

res.statusCode = 200

Closes: #60785
Closes: #59295
Fixes: #47516

…n with empty props

…rsion-skew

ijjk · 2024-02-05T22:24:11Z

Stats from current PR

Default Build

General

	vercel/next.js canary	Lezzio/next.js fix/routing-with-middleware-empty-props-version-skew	Change
buildDuration	11.9s	11.7s	N/A
buildDurationCached	6.1s	4.9s	N/A
nodeModulesSize	200 MB	200 MB	N/A
nextStartRea..uration (ms)	434ms	427ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	Lezzio/next.js fix/routing-with-middleware-empty-props-version-skew	Change
3f784ff6-HASH.js gzip	53.4 kB	53.4 kB	✓
423.HASH.js gzip	185 B	181 B	N/A
68-HASH.js gzip	29.8 kB	29.8 kB	N/A
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	238 B	238 B	✓
main-HASH.js gzip	31.8 kB	31.8 kB	N/A
webpack-HASH.js gzip	1.7 kB	1.7 kB	✓
Overall change	101 kB	101 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	Lezzio/next.js fix/routing-with-middleware-empty-props-version-skew	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	Lezzio/next.js fix/routing-with-middleware-empty-props-version-skew	Change
_app-HASH.js gzip	194 B	195 B	N/A
_error-HASH.js gzip	182 B	181 B	N/A
amp-HASH.js gzip	502 B	501 B	N/A
css-HASH.js gzip	320 B	322 B	N/A
dynamic-HASH.js gzip	2.5 kB	2.5 kB	N/A
edge-ssr-HASH.js gzip	255 B	256 B	N/A
head-HASH.js gzip	350 B	349 B	N/A
hooks-HASH.js gzip	368 B	369 B	N/A
image-HASH.js gzip	4.19 kB	4.18 kB	N/A
index-HASH.js gzip	257 B	256 B	N/A
link-HASH.js gzip	2.61 kB	2.61 kB	N/A
routerDirect..HASH.js gzip	310 B	311 B	N/A
script-HASH.js gzip	384 B	383 B	N/A
withRouter-HASH.js gzip	306 B	308 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	106 B	106 B	✓

Client Build Manifests

	vercel/next.js canary	Lezzio/next.js fix/routing-with-middleware-empty-props-version-skew	Change
_buildManifest.js gzip	483 B	484 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	Lezzio/next.js fix/routing-with-middleware-empty-props-version-skew	Change
index.html gzip	528 B	526 B	N/A
link.html gzip	539 B	537 B	N/A
withRouter.html gzip	523 B	521 B	N/A
Overall change	0 B	0 B	✓

Edge SSR bundle Size

	vercel/next.js canary	Lezzio/next.js fix/routing-with-middleware-empty-props-version-skew	Change
edge-ssr.js gzip	94 kB	94.1 kB	N/A
page.js gzip	150 kB	150 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	Lezzio/next.js fix/routing-with-middleware-empty-props-version-skew	Change
middleware-b..fest.js gzip	625 B	624 B	N/A
middleware-r..fest.js gzip	151 B	149 B	N/A
middleware.js gzip	37.7 kB	37.7 kB	N/A
edge-runtime..pack.js gzip	1.92 kB	1.92 kB	✓
Overall change	1.92 kB	1.92 kB	✓

Next Runtimes

	vercel/next.js canary	Lezzio/next.js fix/routing-with-middleware-empty-props-version-skew	Change
app-page-exp...dev.js gzip	170 kB	170 kB	✓
app-page-exp..prod.js gzip	95.8 kB	95.8 kB	✓
app-page-tur..prod.js gzip	96.4 kB	96.4 kB	✓
app-page-tur..prod.js gzip	91 kB	91 kB	✓
app-page.run...dev.js gzip	142 kB	142 kB	✓
app-page.run..prod.js gzip	90.3 kB	90.3 kB	✓
app-route-ex...dev.js gzip	22.2 kB	22.2 kB	✓
app-route-ex..prod.js gzip	14.9 kB	14.9 kB	✓
app-route-tu..prod.js gzip	14.9 kB	14.9 kB	✓
app-route-tu..prod.js gzip	14.5 kB	14.5 kB	✓
app-route.ru...dev.js gzip	21.7 kB	21.7 kB	✓
app-route.ru..prod.js gzip	14.5 kB	14.5 kB	✓
pages-api-tu..prod.js gzip	9.43 kB	9.43 kB	✓
pages-api.ru...dev.js gzip	9.7 kB	9.7 kB	✓
pages-api.ru..prod.js gzip	9.43 kB	9.43 kB	✓
pages-turbo...prod.js gzip	22 kB	22 kB	✓
pages.runtim...dev.js gzip	22.7 kB	22.7 kB	✓
pages.runtim..prod.js gzip	22 kB	22 kB	✓
server.runti..prod.js gzip	49.7 kB	49.7 kB	✓
Overall change	933 kB	933 kB	✓

Diff details

Diff for main-HASH.js

Diff too large to display

Commit: 889e464

ijjk

Thanks for the PR! Went ahead and updated to handle some cases but seems good

The issue should be fixed according to this release: https://github.com/vercel/next.js/releases/tag/v14.1.1-canary.37 and to this PR: vercel/next.js#60968 But testing the same workflow with v14.1.1-canary.37 doesn't fix it.

fix: status code for 404 props queries to avoid client side navigatio…

9fa7a37

…n with empty props

Lezzio requested review from timneutkens, ijjk, shuding, huozhi, feedthejim, ztanner and wyattjoh as code owners January 22, 2024 12:20

ijjk added the type: next label Jan 22, 2024

Lezzio changed the title ~~fix: status code for 404 props queries to avoid client side navigation with empty props~~ fix: status code 404 for props queries to avoid client side navigation with empty props Jan 22, 2024

Lezzio changed the title ~~fix: status code 404 for props queries to avoid client side navigation with empty props~~ fix: status code for 404 props queries to avoid client side navigation with empty props Jan 23, 2024

Merge branch 'canary' into fix/routing-with-middleware-empty-props-ve…

946c217

…rsion-skew

fix cases

889e464

ijjk added the area: tests label Feb 5, 2024

ijjk approved these changes Feb 5, 2024

View reviewed changes

ijjk merged commit 4ba1f06 into vercel:canary Feb 5, 2024
64 of 69 checks passed

ijjk mentioned this pull request Feb 6, 2024

SSR-prefetches with middleware break application between deployments #59612

Closed

1 task

inside mentioned this pull request Feb 7, 2024

Adding a middleware causes additional requests to /_next/data/**/*.json and their reponses are empty {}. #59295

Closed

1 task

github-actions bot added the locked label Feb 20, 2024

github-actions bot locked as resolved and limited conversation to collaborators Feb 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: status code for 404 props queries to avoid client side navigation with empty props #60968

fix: status code for 404 props queries to avoid client side navigation with empty props #60968

Lezzio commented Jan 22, 2024 •

edited by ijjk

ijjk commented Feb 5, 2024 •

edited

ijjk left a comment

	if (
	req.headers['x-nextjs-data'] &&
	fsChecker.getMiddlewareMatchers()?.length &&
	removePathPrefix(invokePath, config.basePath) === '/404'
	) {
	res.setHeader('x-nextjs-matched-path', parsedUrl.pathname \|\| '')
	res.statusCode = 200
	res.setHeader('content-type', 'application/json')
	res.end('{}')
	return null
	}

	// fetchServerResponse is intentionally not awaited so that it can be unwrapped in the navigate-reducer
	const serverResponse = prefetchQueue.enqueue(() =>
	fetchServerResponse(
	url,
	// initialTree is used when history.state.tree is missing because the history state is set in `useEffect` below, it being missing means this is the hydration case.
	state.tree,
	state.nextUrl,
	state.buildId,
	action.kind
	)
	)

	// Create new tree based on the flightSegmentPath and router state patch
	state.prefetchCache.set(prefetchCacheKey, {
	// Create new tree based on the flightSegmentPath and router state patch
	treeAtTimeOfPrefetch: state.tree,
	data: serverResponse,
	kind: action.kind,
	prefetchTime: Date.now(),
	lastUsedTime: null,
	})

	// If fetch returns something different than flight response handle it like a mpa navigation
	// If the fetch was not 200, we also handle it like a mpa navigation
	if (!isFlightResponse \|\| !res.ok) {
	// in case the original URL came with a hash, preserve it before redirecting to the new URL
	if (url.hash) {
	responseUrl.hash = url.hash
	}

	return doMpaNavigation(responseUrl.toString())
	}

fix: status code for 404 props queries to avoid client side navigation with empty props #60968

fix: status code for 404 props queries to avoid client side navigation with empty props #60968

Conversation

Lezzio commented Jan 22, 2024 • edited by ijjk

What?

Why?

How?

ijjk commented Feb 5, 2024 • edited

Stats from current PR

ijjk left a comment

Choose a reason for hiding this comment

Lezzio commented Jan 22, 2024 •

edited by ijjk

ijjk commented Feb 5, 2024 •

edited