Propagate Serverless Errors to Platform (vercel#12841)

In serverless mode, it's best practice to propagate an unhandled error so that the function is disposed instead of recycled. This helps fix the "bad state" problem.

packages/next/build/webpack/loaders/next-serverless-loader.ts

@@ -193,18 +193,20 @@ const nextServerlessLoader: loader.Loader = function () {
             Object.assign({}, parsedUrl.query, params ),
             resolver,
             ${encodedPreviewProps},
+            true,
             onError
           )
         } catch (err) {
           console.error(err)
           await onError(err)
 
+          // TODO: better error for DECODE_FAILED?
           if (err.code === 'DECODE_FAILED') {
             res.statusCode = 400
             res.end('Bad Request')
           } else {
-            res.statusCode = 500
-            res.end('Internal Server Error')
+            // Throw the error to crash the serverless function
+            throw err
           }
         }
       }
@@ -214,6 +216,7 @@ const nextServerlessLoader: loader.Loader = function () {
     import initServer from 'next-plugin-loader?middleware=on-init-server!'
     import onError from 'next-plugin-loader?middleware=on-error-server!'
     import 'next/dist/next-server/server/node-polyfill-fetch'
+    const {isResSent} = require('next/dist/next-server/lib/utils');
 
     ${envLoading}
     ${runtimeConfigImports}
@@ -390,10 +393,36 @@ const nextServerlessLoader: loader.Loader = function () {
         if (err.code === 'ENOENT') {
           res.statusCode = 404
         } else if (err.code === 'DECODE_FAILED') {
+          // TODO: better error?
           res.statusCode = 400
         } else {
-          console.error(err)
-          res.statusCode = 500
+          console.error('Unhandled error during request:', err)
+
+          // Backwards compat (call getInitialProps in custom error):
+          try {
+            await renderToHTML(req, res, "/_error", parsedUrl.query, Object.assign({}, options, {
+              getStaticProps: undefined,
+              getStaticPaths: undefined,
+              getServerSideProps: undefined,
+              Component: Error,
+              err: err,
+              // Short-circuit rendering:
+              isDataReq: true
+            }))
+          } catch (underErrorErr) {
+            console.error('Failed call /_error subroutine, continuing to crash function:', underErrorErr)
+          }
+
+          // Throw the error to crash the serverless function
+          if (isResSent(res)) {
+            console.error('!!! WARNING !!!')
+            console.error(
+              'Your function crashed, but closed the response before allowing the function to exit.\\n' +
+              'This may cause unexpected behavior for the next request.'
+            )
+            console.error('!!! WARNING !!!')
+          }
+          throw err
         }
 
         const result = await renderToHTML(req, res, "/_error", parsedUrl.query, Object.assign({}, options, {
@@ -414,10 +443,10 @@ const nextServerlessLoader: loader.Loader = function () {
           sendHTML(req, res, html, {generateEtags: ${generateEtags}})
         }
       } catch(err) {
-        await onError(err)
         console.error(err)
-        res.statusCode = 500
-        res.end('Internal Server Error')
+        await onError(err)
+        // Throw the error to crash the serverless function
+        throw err
       }
     }
   `

packages/next/next-server/server/api-utils.ts

@@ -26,6 +26,7 @@ export async function apiResolver(
   params: any,
   resolverModule: any,
   apiContext: __ApiPreviewProps,
+  propagateError: boolean,
   onError?: ({ err }: { err: any }) => Promise<void>
 ) {
   const apiReq = req as NextApiRequest
@@ -89,6 +90,9 @@ export async function apiResolver(
     } else {
       console.error(err)
       if (onError) await onError({ err })
+      if (propagateError) {
+        throw err
+      }
       sendError(apiRes, 500, 'Internal Server Error')
     }
   }

packages/next/next-server/server/next-server.ts

@@ -735,6 +735,7 @@ export default class Server {
       query,
       pageModule,
       this.renderOpts.previewProps,
+      false,
       this.onErrorMiddleware
     )
     return true

test/integration/prerender/pages/api/bad.js

@@ -0,0 +1,3 @@
+export default function handler(req, res) {
+  throw new Error('lol')
+}

test/integration/prerender/pages/bad-gssp.js

@@ -0,0 +1,7 @@
+export function getServerSideProps() {
+  throw new Error('lol')
+}
+
+export default function BadGssp() {
+  return <div />
+}

test/integration/prerender/pages/bad-ssr.js

@@ -0,0 +1,7 @@
+export function getServerSideProps() {
+  return { props: {} }
+}
+
+export default function BadSsr() {
+  throw new Error('oops')
+}

test/integration/prerender/server.js

@@ -2,7 +2,7 @@ const http = require('http')
 const url = require('url')
 const fs = require('fs')
 const path = require('path')
-const server = http.createServer((req, res) => {
+const server = http.createServer(async (req, res) => {
   let { pathname } = url.parse(req.url)
   pathname = pathname.replace(/\/$/, '')
   let isDataReq = false
@@ -92,9 +92,16 @@ const server = http.createServer((req, res) => {
       })
     }
     if (!res.finished) {
-      return typeof re.render === 'function'
-        ? re.render(req, res)
-        : re.default(req, res)
+      try {
+        return await (typeof re.render === 'function'
+          ? re.render(req, res)
+          : re.default(req, res))
+      } catch (e) {
+        console.log('FAIL_FUNCTION', e)
+        res.statusCode = 500
+        res.write('FAIL_FUNCTION')
+        res.end()
+      }
     }
   }
 })

test/integration/prerender/test/index.test.js

@@ -290,7 +290,7 @@ const navigateTest = (dev = false) => {
   })
 }
 
-const runTests = (dev = false, looseMode = false) => {
+const runTests = (dev = false, isEmulatedServerless = false) => {
   navigateTest(dev)
 
   it('should SSR normal page correctly', async () => {
@@ -503,7 +503,7 @@ const runTests = (dev = false, looseMode = false) => {
     expect($('#catchall').text()).toMatch(/Hi.*?second/)
   })
 
-  if (!looseMode) {
+  if (!isEmulatedServerless) {
     it('should handle fallback only page correctly HTML', async () => {
       const browser = await webdriver(appPort, '/fallback-only/first%2Fpost')
 
@@ -539,7 +539,7 @@ const runTests = (dev = false, looseMode = false) => {
     })
   }
 
-  if (!looseMode) {
+  if (!isEmulatedServerless) {
     it('should 404 for a missing catchall explicit route', async () => {
       const res = await fetchViaHTTP(
         appPort,
@@ -788,7 +788,7 @@ const runTests = (dev = false, looseMode = false) => {
       expect(stderr).not.toContain('ERR_HTTP_HEADERS_SENT')
     })
   } else {
-    if (!looseMode) {
+    if (!isEmulatedServerless) {
       it('should should use correct caching headers for a no-revalidate page', async () => {
         const initialRes = await fetchViaHTTP(appPort, '/something')
         expect(initialRes.headers.get('cache-control')).toBe(
@@ -833,6 +833,18 @@ const runTests = (dev = false, looseMode = false) => {
           ),
           page: '/another',
         },
+        {
+          dataRouteRegex: normalizeRegEx(
+            `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/bad-gssp.json$`
+          ),
+          page: '/bad-gssp',
+        },
+        {
+          dataRouteRegex: normalizeRegEx(
+            `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/bad-ssr.json$`
+          ),
+          page: '/bad-ssr',
+        },
         {
           dataRouteRegex: normalizeRegEx(
             `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/blog.json$`
@@ -1061,7 +1073,7 @@ const runTests = (dev = false, looseMode = false) => {
       }
     })
 
-    if (!looseMode) {
+    if (!isEmulatedServerless) {
       it('should handle de-duping correctly', async () => {
         let vals = new Array(10).fill(null)
 
@@ -1091,7 +1103,7 @@ const runTests = (dev = false, looseMode = false) => {
       expect(initialHtml).toBe(newHtml)
     })
 
-    if (!looseMode) {
+    if (!isEmulatedServerless) {
       it('should handle revalidating HTML correctly', async () => {
         const route = '/blog/post-2/comment-2'
         const initialHtml = await renderViaHTTP(appPort, route)
@@ -1144,6 +1156,35 @@ const runTests = (dev = false, looseMode = false) => {
       await waitFor(500)
       expect(stderr).not.toMatch(/Failed to update prerender files for/)
     })
+
+    if (isEmulatedServerless) {
+      it('should fail the api function instead of rendering 500', async () => {
+        const res = await fetchViaHTTP(appPort, '/api/bad')
+        expect(res.status).toBe(500)
+        expect(await res.text()).toBe('FAIL_FUNCTION')
+      })
+
+      it('should fail the page function instead of rendering 500 (getServerSideProps)', async () => {
+        const res = await fetchViaHTTP(appPort, '/bad-gssp')
+        expect(res.status).toBe(500)
+        expect(await res.text()).toBe('FAIL_FUNCTION')
+      })
+
+      it('should fail the page function instead of rendering 500 (render)', async () => {
+        const res = await fetchViaHTTP(appPort, '/bad-ssr')
+        expect(res.status).toBe(500)
+        expect(await res.text()).toBe('FAIL_FUNCTION')
+      })
+
+      it('should call /_error GIP on 500', async () => {
+        stderr = ''
+        const res = await fetchViaHTTP(appPort, '/bad-gssp')
+        expect(res.status).toBe(500)
+        expect(await res.text()).toBe('FAIL_FUNCTION')
+        expect(stderr).toMatch('CUSTOM_ERROR_GIP_CALLED')
+        expect(stderr).not.toMatch('!!! WARNING !!!')
+      })
+    }
   }
 }
 
@@ -1353,6 +1394,8 @@ describe('SSG Prerender', () => {
   })
 
   describe('enumlated serverless mode', () => {
+    const cstmError = join(appDir, 'pages', '_error.js')
+
     beforeAll(async () => {
       const startServerlessEmulator = async (dir, port, buildId) => {
         const scriptPath = join(dir, 'server.js')
@@ -1361,7 +1404,11 @@ describe('SSG Prerender', () => {
           { ...process.env },
           { PORT: port, BUILD_ID: buildId }
         )
-        return initNextServerScript(scriptPath, /ready on/i, env)
+        return initNextServerScript(scriptPath, /ready on/i, env, false, {
+          onStderr: (msg) => {
+            stderr += msg
+          },
+        })
       }
 
       origConfig = await fs.readFile(nextConfig, 'utf8')
@@ -1370,6 +1417,21 @@ describe('SSG Prerender', () => {
         `module.exports = { target: 'experimental-serverless-trace' }`,
         'utf8'
       )
+      await fs.writeFile(
+        cstmError,
+        `
+          function Error() {
+            return <div />
+          }
+
+          Error.getInitialProps = () => {
+            console.error('CUSTOM_ERROR_GIP_CALLED')
+            return {}
+          }
+
+          export default Error
+        `
+      )
       await fs.remove(join(appDir, '.next'))
       await nextBuild(appDir)
 
@@ -1381,6 +1443,7 @@ describe('SSG Prerender', () => {
       app = await startServerlessEmulator(appDir, appPort, buildId)
     })
     afterAll(async () => {
+      await fs.remove(cstmError)
       await fs.writeFile(nextConfig, origConfig)
       await killApp(app)
     })