Skip to content

Commit

Permalink
Fix notFound status code with ISR in app (#55542)
Browse files Browse the repository at this point in the history
This ensures we properly set/restore the status code with ISR paths in
app router so that when we set the 404 status code with `notFound` it is
persisted properly.

Fixes: #43831
Closes: #48342
x-ref:
#49387 (comment)
  • Loading branch information
ijjk committed Sep 18, 2023
1 parent 0338dcd commit 9697bcd
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 0 deletions.
5 changes: 5 additions & 0 deletions packages/next/src/server/base-server.ts
Expand Up @@ -2238,6 +2238,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
html: result,
pageData: metadata.pageData,
headers,
status: isAppPath ? res.statusCode : undefined,
},
revalidate: metadata.revalidate,
}
Expand Down Expand Up @@ -2493,6 +2494,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {
)
}

if (cachedData.status) {
res.statusCode = cachedData.status
}

return {
type: isDataReq ? 'rsc' : 'html',
body: isDataReq
Expand Down
46 changes: 46 additions & 0 deletions test/e2e/app-dir/app-static/app-static.test.ts
Expand Up @@ -700,6 +700,10 @@ createNextDescribe(
'variable-revalidate-edge/post-method-request/page_client-reference-manifest.js',
'partial-gen-params-no-additional-lang/[lang]/[slug]/page_client-reference-manifest.js',
'partial-gen-params-no-additional-slug/[lang]/[slug]/page_client-reference-manifest.js',
'articles/[slug]/page.js',
'articles/[slug]/page_client-reference-manifest.js',
'articles/works.html',
'articles/works.rsc',
].sort()
)
})
Expand Down Expand Up @@ -752,6 +756,22 @@ createNextDescribe(
"initialRevalidateSeconds": false,
"srcRoute": "/",
},
"/articles/works": Object {
"dataRoute": "/articles/works.rsc",
"experimentalBypassFor": Array [
Object {
"key": "Next-Action",
"type": "header",
},
Object {
"key": "content-type",
"type": "header",
"value": "multipart/form-data",
},
],
"initialRevalidateSeconds": 1,
"srcRoute": "/articles/[slug]",
},
"/blog/seb": Object {
"dataRoute": "/blog/seb.rsc",
"experimentalBypassFor": Array [
Expand Down Expand Up @@ -1404,6 +1424,23 @@ createNextDescribe(
`)
expect(curManifest.dynamicRoutes).toMatchInlineSnapshot(`
Object {
"/articles/[slug]": Object {
"dataRoute": "/articles/[slug].rsc",
"dataRouteRegex": "^\\\\/articles\\\\/([^\\\\/]+?)\\\\.rsc$",
"experimentalBypassFor": Array [
Object {
"key": "Next-Action",
"type": "header",
},
Object {
"key": "content-type",
"type": "header",
"value": "multipart/form-data",
},
],
"fallback": null,
"routeRegex": "^\\\\/articles\\\\/([^\\\\/]+?)(?:\\\\/)?$",
},
"/blog/[author]": Object {
"dataRoute": "/blog/[author].rsc",
"dataRouteRegex": "^\\\\/blog\\\\/([^\\\\/]+?)\\\\.rsc$",
Expand Down Expand Up @@ -1590,6 +1627,15 @@ createNextDescribe(
})
}

it('should correctly handle statusCode with notFound + ISR', async () => {
for (let i = 0; i < 5; i++) {
const res = await next.fetch('/articles/non-existent')
expect(res.status).toBe(404)
expect(await res.text()).toContain('This page could not be found')
await waitFor(500)
}
})

it('should cache correctly for fetchCache = default-cache', async () => {
const res = await next.fetch('/default-cache')
expect(res.status).toBe(200)
Expand Down
29 changes: 29 additions & 0 deletions test/e2e/app-dir/app-static/app/articles/[slug]/page.tsx
@@ -0,0 +1,29 @@
import { notFound } from 'next/navigation'

export interface Props {
params: {
slug: string
}
}

const Article = ({ params }: Props) => {
const { slug } = params

if (slug !== 'works') {
return notFound()
}
return <div>Articles page with slug</div>
}

export const revalidate = 1
export const dynamicParams = true

export async function generateStaticParams() {
return [
{
slug: 'works', // Anything not this should be a 404 with no ISR
},
]
}

export default Article

0 comments on commit 9697bcd

Please sign in to comment.