Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate Serverless Errors to Platform #12841

Merged
merged 8 commits into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 36 additions & 7 deletions packages/next/build/webpack/loaders/next-serverless-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Expand All @@ -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}
Expand Down Expand Up @@ -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, {
Expand All @@ -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
}
}
`
Expand Down
4 changes: 4 additions & 0 deletions packages/next/next-server/server/api-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,7 @@ export default class Server {
query,
pageModule,
this.renderOpts.previewProps,
false,
this.onErrorMiddleware
)
return true
Expand Down
3 changes: 3 additions & 0 deletions test/integration/prerender/pages/api/bad.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function handler(req, res) {
throw new Error('lol')
}
7 changes: 7 additions & 0 deletions test/integration/prerender/pages/bad-gssp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export function getServerSideProps() {
throw new Error('lol')
}

export default function BadGssp() {
return <div />
}
7 changes: 7 additions & 0 deletions test/integration/prerender/pages/bad-ssr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export function getServerSideProps() {
return { props: {} }
}

export default function BadSsr() {
throw new Error('oops')
}
15 changes: 11 additions & 4 deletions test/integration/prerender/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
}
}
}
})
Expand Down
77 changes: 70 additions & 7 deletions test/integration/prerender/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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')

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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$`
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 !!!')
})
}
}
}

Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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)

Expand All @@ -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)
})
Expand Down