From 0c264ab143b37b14cbb3d652a55d0e1e9c0557d9 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Wed, 13 May 2020 12:32:22 -0400 Subject: [PATCH 1/7] Propagate Serverless Errors to Platform --- .../build/webpack/loaders/next-serverless-loader.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 545ef0f82f74d..97a5154eb9293 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -199,12 +199,12 @@ const nextServerlessLoader: loader.Loader = function () { 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 err } } } @@ -390,10 +390,11 @@ 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 + throw err } const result = await renderToHTML(req, res, "/_error", parsedUrl.query, Object.assign({}, options, { @@ -414,10 +415,9 @@ 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 err } } ` From e411f5c27ce8704946fe2b434194a166784b57f1 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 1 Jun 2020 10:34:39 -0400 Subject: [PATCH 2/7] Add FAIL FUNCTION response --- .../build/webpack/loaders/next-serverless-loader.ts | 3 +++ test/integration/prerender/server.js | 12 +++++++++--- test/integration/prerender/test/index.test.js | 12 ++++++------ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 97a5154eb9293..6142381c350a0 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -204,6 +204,7 @@ const nextServerlessLoader: loader.Loader = function () { res.statusCode = 400 res.end('Bad Request') } else { + // Throw the error to crash the serverless function throw err } } @@ -394,6 +395,7 @@ const nextServerlessLoader: loader.Loader = function () { res.statusCode = 400 } else { console.error(err) + // Throw the error to crash the serverless function throw err } @@ -417,6 +419,7 @@ const nextServerlessLoader: loader.Loader = function () { } catch(err) { console.error(err) await onError(err) + // Throw the error to crash the serverless function throw err } } diff --git a/test/integration/prerender/server.js b/test/integration/prerender/server.js index 056bdcdd7212c..d70b84604735c 100644 --- a/test/integration/prerender/server.js +++ b/test/integration/prerender/server.js @@ -92,9 +92,15 @@ const server = http.createServer((req, res) => { }) } if (!res.finished) { - return typeof re.render === 'function' - ? re.render(req, res) - : re.default(req, res) + try { + return typeof re.render === 'function' + ? re.render(req, res) + : re.default(req, res) + } catch (e) { + console.log('FAIL_FUNCTION', e) + res.write('FAIL_FUNCTION') + res.end() + } } } }) diff --git a/test/integration/prerender/test/index.test.js b/test/integration/prerender/test/index.test.js index f0fd74e48751a..456b280a23181 100644 --- a/test/integration/prerender/test/index.test.js +++ b/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( @@ -1061,7 +1061,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 +1091,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) From c492afd74fdcf9c1168a9d1a43d1cd8e862f12bf Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 1 Jun 2020 10:54:00 -0400 Subject: [PATCH 3/7] Fix API crashing --- .../next/build/webpack/loaders/next-serverless-loader.ts | 1 + packages/next/next-server/server/api-utils.ts | 4 ++++ packages/next/next-server/server/next-server.ts | 1 + test/integration/prerender/pages/api/bad.js | 3 +++ test/integration/prerender/pages/bad-gssp.js | 7 +++++++ test/integration/prerender/pages/bad-ssr.js | 7 +++++++ test/integration/prerender/server.js | 7 ++++--- test/integration/prerender/test/index.test.js | 8 ++++++++ 8 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 test/integration/prerender/pages/api/bad.js create mode 100644 test/integration/prerender/pages/bad-gssp.js create mode 100644 test/integration/prerender/pages/bad-ssr.js diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 6142381c350a0..be403ccaa7dcc 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -193,6 +193,7 @@ const nextServerlessLoader: loader.Loader = function () { Object.assign({}, parsedUrl.query, params ), resolver, ${encodedPreviewProps}, + true, onError ) } catch (err) { diff --git a/packages/next/next-server/server/api-utils.ts b/packages/next/next-server/server/api-utils.ts index d4a48881427ff..3ed48b73803b6 100644 --- a/packages/next/next-server/server/api-utils.ts +++ b/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 ) { 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') } } diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index b25eeb98febcd..efa818bf55fc9 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/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 diff --git a/test/integration/prerender/pages/api/bad.js b/test/integration/prerender/pages/api/bad.js new file mode 100644 index 0000000000000..5d2b77e5350a3 --- /dev/null +++ b/test/integration/prerender/pages/api/bad.js @@ -0,0 +1,3 @@ +export default function handler(req, res) { + throw new Error('lol') +} diff --git a/test/integration/prerender/pages/bad-gssp.js b/test/integration/prerender/pages/bad-gssp.js new file mode 100644 index 0000000000000..d378a871cdec2 --- /dev/null +++ b/test/integration/prerender/pages/bad-gssp.js @@ -0,0 +1,7 @@ +export function getServerSideProps() { + throw new Error('lol') +} + +export default function BadGssp() { + return
+} diff --git a/test/integration/prerender/pages/bad-ssr.js b/test/integration/prerender/pages/bad-ssr.js new file mode 100644 index 0000000000000..8742aa8fbaeb0 --- /dev/null +++ b/test/integration/prerender/pages/bad-ssr.js @@ -0,0 +1,7 @@ +export function getServerSideProps() { + return { props: {} } +} + +export default function BadSsr() { + throw new Error('oops') +} diff --git a/test/integration/prerender/server.js b/test/integration/prerender/server.js index d70b84604735c..6a98fa3d30806 100644 --- a/test/integration/prerender/server.js +++ b/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 @@ -93,11 +93,12 @@ const server = http.createServer((req, res) => { } if (!res.finished) { try { - return typeof re.render === 'function' + return await (typeof re.render === 'function' ? re.render(req, res) - : re.default(req, res) + : re.default(req, res)) } catch (e) { console.log('FAIL_FUNCTION', e) + res.statusCode = 500 res.write('FAIL_FUNCTION') res.end() } diff --git a/test/integration/prerender/test/index.test.js b/test/integration/prerender/test/index.test.js index 456b280a23181..476cc09f84c5f 100644 --- a/test/integration/prerender/test/index.test.js +++ b/test/integration/prerender/test/index.test.js @@ -1144,6 +1144,14 @@ const runTests = (dev = false, isEmulatedServerless = 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') + }) + } } } From a3870e81bce7b514966e3666bfa98166e40e0333 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 1 Jun 2020 10:54:54 -0400 Subject: [PATCH 4/7] Add page tests --- test/integration/prerender/test/index.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/integration/prerender/test/index.test.js b/test/integration/prerender/test/index.test.js index 476cc09f84c5f..b6533c576172b 100644 --- a/test/integration/prerender/test/index.test.js +++ b/test/integration/prerender/test/index.test.js @@ -1151,6 +1151,18 @@ const runTests = (dev = false, isEmulatedServerless = false) => { 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') + }) } } } From 6fd80e6cdb66cafd7859907dcfe2f66993f9bd35 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 1 Jun 2020 11:44:00 -0400 Subject: [PATCH 5/7] fix test --- test/integration/prerender/test/index.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/integration/prerender/test/index.test.js b/test/integration/prerender/test/index.test.js index b6533c576172b..0757ab98e1bc3 100644 --- a/test/integration/prerender/test/index.test.js +++ b/test/integration/prerender/test/index.test.js @@ -833,6 +833,18 @@ const runTests = (dev = false, isEmulatedServerless = 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$` From 90c9b15ac0151adcc8497aa4f91040da9cfe8cef Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 1 Jun 2020 18:03:46 -0400 Subject: [PATCH 6/7] Ensure we call data function for custom error page --- .../webpack/loaders/next-serverless-loader.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index be403ccaa7dcc..1c12085ceea96 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -216,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} @@ -395,8 +396,32 @@ const nextServerlessLoader: loader.Loader = function () { // TODO: better error? res.statusCode = 400 } else { - console.error(err) + 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 } From c38285a2ddaea0d66d099e2d10d1fca25714dd56 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 1 Jun 2020 18:14:28 -0400 Subject: [PATCH 7/7] Add tests --- test/integration/prerender/test/index.test.js | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/test/integration/prerender/test/index.test.js b/test/integration/prerender/test/index.test.js index 0757ab98e1bc3..7938590878262 100644 --- a/test/integration/prerender/test/index.test.js +++ b/test/integration/prerender/test/index.test.js @@ -1175,6 +1175,15 @@ const runTests = (dev = false, isEmulatedServerless = false) => { 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 !!!') + }) } } } @@ -1385,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') @@ -1393,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') @@ -1402,6 +1417,21 @@ describe('SSG Prerender', () => { `module.exports = { target: 'experimental-serverless-trace' }`, 'utf8' ) + await fs.writeFile( + cstmError, + ` + function Error() { + return
+ } + + Error.getInitialProps = () => { + console.error('CUSTOM_ERROR_GIP_CALLED') + return {} + } + + export default Error + ` + ) await fs.remove(join(appDir, '.next')) await nextBuild(appDir) @@ -1413,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) })