Skip to content

Commit

Permalink
Fix status code for /_not-found route (#64058)
Browse files Browse the repository at this point in the history
### What

Fix the status code in static generation metadata for `/_not-found`
route, aligning it as 404 for both dev and build

### Why
`/_not-found` route should still return 404 code as it's reserved as
default not found route

Closes NEXT-3001
  • Loading branch information
huozhi authored Apr 4, 2024
1 parent 00d9c58 commit 02197bc
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 12 deletions.
11 changes: 8 additions & 3 deletions packages/next/src/export/routes/app-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ export async function exportAppPage(
isDynamicError: boolean,
fileWriter: FileWriter
): Promise<ExportRouteResult> {
let isDefaultNotFound = false
// If the page is `/_not-found`, then we should update the page to be `/404`.
// UNDERSCORE_NOT_FOUND_ROUTE value used here, however we don't want to import it here as it causes constants to be inlined which we don't want here.
if (page === '/_not-found') {
if (page === '/_not-found/page') {
isDefaultNotFound = true
pathname = '/404'
}

Expand Down Expand Up @@ -137,8 +139,11 @@ export async function exportAppPage(
? res.statusCode
: undefined

// If it's parallel route the status from mock response is 404
if (isNonSuccessfulStatusCode && !isParallelRoute) {
if (isDefaultNotFound) {
// Override the default /_not-found page status code to 404
status = 404
} else if (isNonSuccessfulStatusCode && !isParallelRoute) {
// If it's parallel route the status from mock response is 404
status = res.statusCode
}

Expand Down
9 changes: 4 additions & 5 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1747,11 +1747,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {
{ req, res, pathname, renderOpts: opts }: RequestContext,
{ components, query }: FindComponentsResult
): Promise<ResponsePayload | null> {
const is404Page =
// For edge runtime 404 page, /_not-found needs to be treated as 404 page
(process.env.NEXT_RUNTIME === 'edge' &&
pathname === UNDERSCORE_NOT_FOUND_ROUTE) ||
pathname === '/404'
if (pathname === UNDERSCORE_NOT_FOUND_ROUTE) {
pathname = '/404'
}
const is404Page = pathname === '/404'

// Strip the internal headers.
this.stripInternalHeaders(req)
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/app-dir/not-found-default/app/layout.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use client'

import React, { useState } from 'react'
import { useState, Suspense } from 'react'
import { notFound } from 'next/navigation'
import NotFoundTrigger from './not-found-trigger'

Expand All @@ -13,9 +13,9 @@ export default function Root({ children }) {
return (
<html className="root-layout-html">
<body>
<React.Suspense fallback={<div>Loading...</div>}>
<Suspense fallback={<div>Loading...</div>}>
<NotFoundTrigger />
</React.Suspense>
</Suspense>
<button id="trigger-not-found" onClick={() => setClicked(true)}>
Click to not found
</button>
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/app-dir/not-found-default/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ createNextDescribe(
)
})

it('should return 404 status code for default not-found page', async () => {
const res = await next.fetch('/_not-found')
expect(res.status).toBe(404)
})

it('should error on server notFound from root layout on server-side', async () => {
const browser = await next.browser('/?root-not-found=1')

Expand Down
5 changes: 5 additions & 0 deletions test/e2e/app-dir/not-found/basic/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ createNextDescribe(
)
})

it('should return 404 status code for custom not-found page', async () => {
const res = await next.fetch('/_not-found')
expect(res.status).toBe(404)
})

if (isNextStart) {
it('should include not found client reference manifest in the file trace', async () => {
const fileTrace = JSON.parse(
Expand Down
3 changes: 2 additions & 1 deletion test/turbopack-build-tests-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -2052,7 +2052,8 @@
"app dir - not-found - basic with default runtime should use the not-found page for non-matching routes",
"app dir - not-found - basic with runtime = edge should escalate notFound to parent layout if no not-found boundary present in current layer",
"app dir - not-found - basic with runtime = edge should match dynamic route not-found boundary correctly",
"app dir - not-found - basic with runtime = edge should use the not-found page for non-matching routes"
"app dir - not-found - basic with runtime = edge should use the not-found page for non-matching routes",
"app dir - not-found - basic should return 404 status code for custom not-found page"
],
"failed": [
"app dir - not-found - basic should include not found client reference manifest in the file trace"
Expand Down

0 comments on commit 02197bc

Please sign in to comment.