Fix(app renderer): set status code to 500 if unexpected error occurs before streaming

dpnolte · dpnolte · commit bee4a54e17dd · 2023-09-30T14:05:25.000+02:00
diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx
@@ -1616,6 +1616,10 @@ export const renderToHTMLOrFlight: AppPageRender = (
 
           const is404 = res.statusCode === 404
 
+          if (!is404 && !hasRedirectError) {
+            res.statusCode = 500
+          }
+
           // Preserve the existing RSC inline chunks from the page rendering.
           // To avoid the same stream being operated twice, clone the origin stream for error rendering.
           const serverErrorComponentsRenderOpts: typeof serverComponentsRenderOpts =
diff --git a/test/production/app-dir/unexpected-error/app/isr-unexpected-error/page.tsx b/test/production/app-dir/unexpected-error/app/isr-unexpected-error/page.tsx
@@ -0,0 +1,13 @@
+export const revalidate = 0
+
+export default function UnexpectedErrorPage(props) {
+  // use query param to only throw error during runtime, not build time
+  if (props.searchParams.error) {
+    throw new Error('Oh no')
+  }
+  return (
+    <>
+      <p id="page">/unexpected-error</p>
+    </>
+  )
+}
diff --git a/test/production/app-dir/unexpected-error/app/layout.tsx b/test/production/app-dir/unexpected-error/app/layout.tsx
@@ -0,0 +1,7 @@
+export default function Root({ children }: { children: React.ReactNode }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/production/app-dir/unexpected-error/app/ssr-unexpected-error-after-streaming/loading.tsx b/test/production/app-dir/unexpected-error/app/ssr-unexpected-error-after-streaming/loading.tsx
@@ -0,0 +1,3 @@
+export default function Loading() {
+  return <p>loading</p>
+}
diff --git a/test/production/app-dir/unexpected-error/app/ssr-unexpected-error-after-streaming/page.tsx b/test/production/app-dir/unexpected-error/app/ssr-unexpected-error-after-streaming/page.tsx
@@ -0,0 +1,11 @@
+export default function UnexpectedErrorPage(props) {
+  // use query param to only throw error during runtime, not build time
+  if (props.searchParams.error) {
+    throw new Error('Oh no')
+  }
+  return (
+    <>
+      <p id="page">/unexpected-error</p>
+    </>
+  )
+}
diff --git a/test/production/app-dir/unexpected-error/app/ssr-unexpected-error/page.tsx b/test/production/app-dir/unexpected-error/app/ssr-unexpected-error/page.tsx
@@ -0,0 +1,11 @@
+export default function UnexpectedErrorPage(props) {
+  // use query param to only throw error during runtime, not build time
+  if (props.searchParams.error) {
+    throw new Error('Oh no')
+  }
+  return (
+    <>
+      <p id="page">/unexpected-error</p>
+    </>
+  )
+}
diff --git a/test/production/app-dir/unexpected-error/next.config.js b/test/production/app-dir/unexpected-error/next.config.js
@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig
diff --git a/test/production/app-dir/unexpected-error/unexpected-error.test.ts b/test/production/app-dir/unexpected-error/unexpected-error.test.ts
@@ -0,0 +1,26 @@
+import { createNextDescribe } from 'e2e-utils'
+
+createNextDescribe(
+  'unexpected-error',
+  {
+    files: __dirname,
+  },
+  ({ next }) => {
+    it('should set response status to 500 for unexpected errors in ssr app route', async () => {
+      const res = await next.fetch('/ssr-unexpected-error?error=true')
+      expect(res.status).toBe(500)
+    })
+
+    it('cannot change response status when streaming has started', async () => {
+      const res = await next.fetch(
+        '/ssr-unexpected-error-after-streaming?error=true'
+      )
+      expect(res.status).toBe(200)
+    })
+
+    it('should set response status to 500 for unexpected errors in isr app route', async () => {
+      const res = await next.fetch('/isr-unexpected-error?error=true')
+      expect(res.status).toBe(500)
+    })
+  }
+)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+export default function Loading() {`
	`2`	`+ return <p>loading</p>`
	`3`	`+}`