diff --git a/packages/next/client/dev/on-demand-entries-client.js b/packages/next/client/dev/on-demand-entries-client.js index 4176b9ec921aa89..83fe32345e3d9d7 100644 --- a/packages/next/client/dev/on-demand-entries-client.js +++ b/packages/next/client/dev/on-demand-entries-client.js @@ -9,7 +9,11 @@ export default async ({ assetPrefix }) => { ) }) - setupPing(assetPrefix, () => Router.pathname, currentPage) + setupPing( + assetPrefix, + () => Router.query.__NEXT_PAGE || Router.pathname, + currentPage + ) // prevent HMR connection from being closed when running tests if (!process.env.__NEXT_TEST_MODE) { diff --git a/packages/next/client/dev/on-demand-entries-utils.js b/packages/next/client/dev/on-demand-entries-utils.js index 5a0aca21ff90fac..864a51f11efb927 100644 --- a/packages/next/client/dev/on-demand-entries-utils.js +++ b/packages/next/client/dev/on-demand-entries-utils.js @@ -29,7 +29,10 @@ export function setupPing(assetPrefix, pathnameFn, retry) { if (event.data.indexOf('{') === -1) return try { const payload = JSON.parse(event.data) - if (payload.invalid) { + // don't attempt fetching the page if we're already showing + // the dev overlay as this can cause the error to be triggered + // repeatedly + if (payload.invalid && !self.__NEXT_DATA__.err) { // Payload can be invalid even if the page does not exist. // So, we need to make sure it exists before reloading. fetch(location.href, { diff --git a/packages/next/client/next-dev.js b/packages/next/client/next-dev.js index 0fe54678b370fbe..4c4636898acc6a9 100644 --- a/packages/next/client/next-dev.js +++ b/packages/next/client/next-dev.js @@ -59,6 +59,12 @@ initNext({ webpackHMR }) } else if (event.data.indexOf('serverOnlyChanges') !== -1) { const { pages } = JSON.parse(event.data) + // Make sure to reload when the dev-overlay is showing for an + // API route + if (pages.includes(router.query.__NEXT_PAGE)) { + return window.location.reload() + } + if (!router.clc && pages.includes(router.pathname)) { console.log('Refreshing page data due to server-side change') diff --git a/packages/next/server/api-utils.ts b/packages/next/server/api-utils.ts index f6ef0654012d854..510d3019662cbfe 100644 --- a/packages/next/server/api-utils.ts +++ b/packages/next/server/api-utils.ts @@ -25,7 +25,9 @@ export async function apiResolver( query: any, resolverModule: any, apiContext: __ApiPreviewProps, - propagateError: boolean + propagateError: boolean, + dev?: boolean, + page?: string ): Promise { const apiReq = req as NextApiRequest const apiRes = res as NextApiResponse @@ -117,6 +119,13 @@ export async function apiResolver( if (err instanceof ApiError) { sendError(apiRes, err.statusCode, err.message) } else { + if (dev) { + if (err) { + err.page = page + } + throw err + } + console.error(err) if (propagateError) { throw err diff --git a/packages/next/server/dev/hot-reloader.ts b/packages/next/server/dev/hot-reloader.ts index 277e6b958d9e5bc..caee569744ce1e8 100644 --- a/packages/next/server/dev/hot-reloader.ts +++ b/packages/next/server/dev/hot-reloader.ts @@ -134,7 +134,7 @@ export default class HotReloader { private webpackHotMiddleware: (NextHandleFunction & any) | null private config: NextConfigComplete private stats: webpack.Stats | null - private serverStats: webpack.Stats | null + public serverStats: webpack.Stats | null private clientError: Error | null = null private serverError: Error | null = null private serverPrevDocumentHash: string | null diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts index 60ab771ec783a7f..056944720f8948d 100644 --- a/packages/next/server/dev/next-dev-server.ts +++ b/packages/next/server/dev/next-dev-server.ts @@ -1,5 +1,6 @@ import crypto from 'crypto' import fs from 'fs' +import chalk from 'chalk' import { IncomingMessage, ServerResponse } from 'http' import { Worker } from 'jest-worker' import AmpHtmlValidator from 'next/dist/compiled/amphtml-validator' @@ -47,6 +48,12 @@ import { loadDefaultErrorComponents, } from '../load-components' import { DecodeError } from '../../shared/lib/utils' +import { parseStack } from '@next/react-dev-overlay/lib/internal/helpers/parseStack' +import { + createOriginalStackFrame, + getSourceById, +} from '@next/react-dev-overlay/lib/middleware' +import * as Log from '../../build/output/log' // Load ReactDevOverlay only when needed let ReactDevOverlayImpl: React.FunctionComponent @@ -325,6 +332,15 @@ export default class DevServer extends Server { ) // This is required by the tracing subsystem. setGlobal('telemetry', telemetry) + + process.on('unhandledRejection', (reason) => { + this.logErrorWithOriginalStack(reason, 'unhandledRejection').catch( + () => {} + ) + }) + process.on('uncaughtException', (err) => { + this.logErrorWithOriginalStack(err, 'uncaughtException').catch(() => {}) + }) } protected async close(): Promise { @@ -431,8 +447,85 @@ export default class DevServer extends Server { // if they should match against the basePath or not parsedUrl.pathname = originalPathname } + try { + return await super.run(req, res, parsedUrl) + } catch (err) { + res.statusCode = 500 + try { + this.logErrorWithOriginalStack(err).catch(() => {}) + return await this.renderError(err, req, res, pathname!, { + __NEXT_PAGE: err?.page || pathname, + }) + } catch (internalErr) { + console.error(internalErr) + res.end('Internal Server Error') + } + } + } - return super.run(req, res, parsedUrl) + private async logErrorWithOriginalStack( + possibleError?: any, + type?: 'unhandledRejection' | 'uncaughtException' + ) { + let usedOriginalStack = false + + if (possibleError?.name && possibleError?.stack && possibleError?.message) { + const err: Error & { stack: string } = possibleError + try { + const frames = parseStack(err.stack) + const frame = frames[0] + + if (frame.lineNumber && frame?.file) { + const compilation = this.hotReloader?.serverStats?.compilation + const moduleId = frame.file!.replace( + /^(webpack-internal:\/\/\/|file:\/\/)/, + '' + ) + + const source = await getSourceById( + !!frame.file?.startsWith(sep) || !!frame.file?.startsWith('file:'), + moduleId, + compilation, + this.hotReloader!.isWebpack5 + ) + + const originalFrame = await createOriginalStackFrame({ + line: frame.lineNumber!, + column: frame.column, + source, + frame, + modulePath: moduleId, + rootDirectory: this.dir, + }) + + if (originalFrame) { + const { originalCodeFrame, originalStackFrame } = originalFrame + const { file, lineNumber, column, methodName } = originalStackFrame + + console.error( + chalk.red('error') + + ' - ' + + `${file} (${lineNumber}:${column}) @ ${methodName}` + ) + console.error(`${chalk.red(err.name)}: ${err.message}`) + console.error(originalCodeFrame) + usedOriginalStack = true + } + } + } catch (_) { + // failed to load original stack using source maps + // this un-actionable by users so we don't show the + // internal error and only show the provided stack + } + } + + if (!usedOriginalStack) { + if (type) { + Log.error(`${type}:`, possibleError) + } else { + Log.error(possibleError) + } + } } // override production loading of routes-manifest diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts index fc32aadf7bbb8f1..189b4b508d445fc 100644 --- a/packages/next/server/next-server.ts +++ b/packages/next/server/next-server.ts @@ -487,7 +487,7 @@ export default class Server { try { return await this.run(req, res, parsedUrl) } catch (err) { - if (this.minimalMode) { + if (this.minimalMode || this.renderOpts.dev) { throw err } this.logError(err) @@ -1125,7 +1125,9 @@ export default class Server { query, pageModule, this.renderOpts.previewProps, - this.minimalMode + this.minimalMode, + this.renderOpts.dev, + page ) return true } @@ -1857,6 +1859,7 @@ export default class Server { ctx: RequestContext ): Promise { const { res, query, pathname } = ctx + let page = pathname const bubbleNoFallback = !!query._nextBubbleNoFallback delete query._nextBubbleNoFallback @@ -1888,6 +1891,7 @@ export default class Server { ) if (dynamicRouteResult) { try { + page = dynamicRoute.page return await this.renderToResponseWithComponents( { ...ctx, @@ -1929,7 +1933,10 @@ export default class Server { ) if (!isWrappedError) { - if (this.minimalMode) { + if (this.minimalMode || this.renderOpts.dev) { + if (err) { + err.page = page + } throw err } this.logError(err) diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index 2ae9a764e457409..c49671aa9b22b3c 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -822,10 +822,7 @@ export async function renderToHTML( ;(renderOpts as any).pageData = props } } catch (dataFetchError) { - if (isDataReq || !dev || !dataFetchError) throw dataFetchError - ctx.err = dataFetchError - renderOpts.err = dataFetchError - console.error(dataFetchError) + throw dataFetchError } if ( diff --git a/packages/react-dev-overlay/src/middleware.ts b/packages/react-dev-overlay/src/middleware.ts index 22305e97c43b9f2..ef1cba9827f351f 100644 --- a/packages/react-dev-overlay/src/middleware.ts +++ b/packages/react-dev-overlay/src/middleware.ts @@ -177,52 +177,50 @@ export async function createOriginalStackFrame({ } } -function getOverlayMiddleware(options: OverlayMiddlewareOptions) { - async function getSourceById( - isServerSide: boolean, - isFile: boolean, - id: string - ): Promise { - if (isFile) { - const fileContent: string | null = await fs - .readFile(id, 'utf-8') - .catch(() => null) - - if (fileContent == null) { - return null - } - - const map = getRawSourceMap(fileContent) - if (map == null) { - return null - } +export async function getSourceById( + isFile: boolean, + id: string, + compilation: any, + isWebpack5: boolean +): Promise { + if (isFile) { + const fileContent: string | null = await fs + .readFile(id, 'utf-8') + .catch(() => null) + + if (fileContent == null) { + return null + } - return { - map() { - return map - }, - } + const map = getRawSourceMap(fileContent) + if (map == null) { + return null } - try { - const compilation = isServerSide - ? options.serverStats()?.compilation - : options.stats()?.compilation - if (compilation == null) { - return null - } + return { + map() { + return map + }, + } + } - const module = [...compilation.modules].find( - (searchModule) => - getModuleId(compilation, searchModule, options.isWebpack5) === id - ) - return getModuleSource(compilation, module, options.isWebpack5) - } catch (err) { - console.error(`Failed to lookup module by ID ("${id}"):`, err) + try { + if (compilation == null) { return null } + + const module = [...compilation.modules].find( + (searchModule) => + getModuleId(compilation, searchModule, isWebpack5) === id + ) + return getModuleSource(compilation, module, isWebpack5) + } catch (err) { + console.error(`Failed to lookup module by ID ("${id}"):`, err) + return null } +} +function getOverlayMiddleware(options: OverlayMiddlewareOptions) { return async function ( req: IncomingMessage, res: ServerResponse, @@ -254,10 +252,15 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) { let source: Source try { + const compilation = isServerSide + ? options.serverStats()?.compilation + : options.stats()?.compilation + source = await getSourceById( - isServerSide, frame.file.startsWith('file:'), - moduleId + moduleId, + compilation, + !!options.isWebpack5 ) } catch (err) { console.log('Failed to get source map:', err) diff --git a/test/integration/api-support/test/index.test.js b/test/integration/api-support/test/index.test.js index e5c95cea4690356..266c6eb46065f48 100644 --- a/test/integration/api-support/test/index.test.js +++ b/test/integration/api-support/test/index.test.js @@ -109,14 +109,24 @@ function runTests(dev = false) { const res = await fetchViaHTTP(appPort, '/api/user-error', null, {}) const text = await res.text() expect(res.status).toBe(500) - expect(text).toBe('Internal Server Error') + + if (dev) { + expect(text).toContain('User error') + } else { + expect(text).toBe('Internal Server Error') + } }) it('should throw Internal Server Error (async)', async () => { const res = await fetchViaHTTP(appPort, '/api/user-error-async', null, {}) const text = await res.text() expect(res.status).toBe(500) - expect(text).toBe('Internal Server Error') + + if (dev) { + expect(text).toContain('User error') + } else { + expect(text).toBe('Internal Server Error') + } }) it('should parse JSON body', async () => { diff --git a/test/integration/server-side-dev-errors/pages/api/blog/[slug].js b/test/integration/server-side-dev-errors/pages/api/blog/[slug].js new file mode 100644 index 000000000000000..4ec7a629790f470 --- /dev/null +++ b/test/integration/server-side-dev-errors/pages/api/blog/[slug].js @@ -0,0 +1,3 @@ +export default function handler(req, res) { + res.status(200).json({ slug: req.query.slug }) +} diff --git a/test/integration/server-side-dev-errors/pages/api/hello.js b/test/integration/server-side-dev-errors/pages/api/hello.js new file mode 100644 index 000000000000000..facbe2579e2af8a --- /dev/null +++ b/test/integration/server-side-dev-errors/pages/api/hello.js @@ -0,0 +1,3 @@ +export default function handler(req, res) { + res.status(200).json({ hello: 'world' }) +} diff --git a/test/integration/server-side-dev-errors/pages/blog/[slug].js b/test/integration/server-side-dev-errors/pages/blog/[slug].js new file mode 100644 index 000000000000000..189f594fcf4aa35 --- /dev/null +++ b/test/integration/server-side-dev-errors/pages/blog/[slug].js @@ -0,0 +1,9 @@ +export default function Page() { + return

dynamic getServerSideProps page

+} + +export async function getServerSideProps() { + return { + props: {}, + } +} diff --git a/test/integration/server-side-dev-errors/pages/gsp.js b/test/integration/server-side-dev-errors/pages/gsp.js new file mode 100644 index 000000000000000..57b37b5c5fefbfe --- /dev/null +++ b/test/integration/server-side-dev-errors/pages/gsp.js @@ -0,0 +1,9 @@ +export default function Page() { + return

getStaticProps page

+} + +export async function getStaticProps() { + return { + props: {}, + } +} diff --git a/test/integration/server-side-dev-errors/pages/gssp.js b/test/integration/server-side-dev-errors/pages/gssp.js new file mode 100644 index 000000000000000..3a4b63e1aa29811 --- /dev/null +++ b/test/integration/server-side-dev-errors/pages/gssp.js @@ -0,0 +1,9 @@ +export default function Page() { + return

getServerSideProps page

+} + +export async function getServerSideProps() { + return { + props: {}, + } +} diff --git a/test/integration/server-side-dev-errors/pages/uncaught-exception.js b/test/integration/server-side-dev-errors/pages/uncaught-exception.js new file mode 100644 index 000000000000000..603b4377151a24f --- /dev/null +++ b/test/integration/server-side-dev-errors/pages/uncaught-exception.js @@ -0,0 +1,12 @@ +export default function Page() { + return

getServerSideProps page

+} + +export async function getServerSideProps() { + setTimeout(() => { + throw new Error('catch this exception') + }, 10) + return { + props: {}, + } +} diff --git a/test/integration/server-side-dev-errors/pages/uncaught-rejection.js b/test/integration/server-side-dev-errors/pages/uncaught-rejection.js new file mode 100644 index 000000000000000..f90bb3ed90ac991 --- /dev/null +++ b/test/integration/server-side-dev-errors/pages/uncaught-rejection.js @@ -0,0 +1,12 @@ +export default function Page() { + return

getServerSideProps page

+} + +export async function getServerSideProps() { + setTimeout(() => { + Promise.reject(new Error('catch this rejection')) + }, 10) + return { + props: {}, + } +} diff --git a/test/integration/server-side-dev-errors/test/index.test.js b/test/integration/server-side-dev-errors/test/index.test.js new file mode 100644 index 000000000000000..d8c6915d3a448e8 --- /dev/null +++ b/test/integration/server-side-dev-errors/test/index.test.js @@ -0,0 +1,233 @@ +/* eslint-env jest */ + +import fs from 'fs-extra' +import { join } from 'path' +import webdriver from 'next-webdriver' +import { + killApp, + findPort, + launchApp, + check, + hasRedbox, + getRedboxSource, +} from 'next-test-utils' + +jest.setTimeout(1000 * 60 * 2) + +const appDir = join(__dirname, '../') +const gspPage = join(appDir, 'pages/gsp.js') +const gsspPage = join(appDir, 'pages/gssp.js') +const dynamicGsspPage = join(appDir, 'pages/blog/[slug].js') +const apiPage = join(appDir, 'pages/api/hello.js') +const dynamicApiPage = join(appDir, 'pages/api/blog/[slug].js') + +let stderr = '' +let appPort +let app + +describe('server-side dev errors', () => { + beforeAll(async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort, { + onStderr(msg) { + stderr += msg + }, + env: { + __NEXT_TEST_WITH_DEVTOOL: 1, + }, + }) + }) + afterAll(() => killApp(app)) + + it('should show server-side error for gsp page correctly', async () => { + const content = await fs.readFile(gspPage, 'utf8') + + try { + const stderrIdx = stderr.length + await fs.writeFile( + gspPage, + content.replace('return {', 'missingVar;return {') + ) + const browser = await webdriver(appPort, '/gsp') + + await check(async () => { + const err = stderr.substr(stderrIdx) + + return err.includes('pages/gsp.js') && + err.includes('6:2') && + err.includes('getStaticProps') && + err.includes('missingVar') + ? 'success' + : err + }, 'success') + + expect(await hasRedbox(browser, true)).toBe(true) + + expect(await getRedboxSource(browser)).toContain('missingVar') + await fs.writeFile(gspPage, content) + await hasRedbox(browser, false) + } finally { + await fs.writeFile(gspPage, content) + } + }) + + it('should show server-side error for gssp page correctly', async () => { + const content = await fs.readFile(gsspPage, 'utf8') + + try { + const stderrIdx = stderr.length + await fs.writeFile( + gsspPage, + content.replace('return {', 'missingVar;return {') + ) + const browser = await webdriver(appPort, '/gssp') + + await check(async () => { + const err = stderr.substr(stderrIdx) + + return err.includes('pages/gssp.js') && + err.includes('6:2') && + err.includes('getServerSideProps') && + err.includes('missingVar') + ? 'success' + : err + }, 'success') + + expect(await hasRedbox(browser, true)).toBe(true) + + expect(await getRedboxSource(browser)).toContain('missingVar') + await fs.writeFile(gsspPage, content) + await hasRedbox(browser, false) + } finally { + await fs.writeFile(gsspPage, content) + } + }) + + it('should show server-side error for dynamic gssp page correctly', async () => { + const content = await fs.readFile(dynamicGsspPage, 'utf8') + + try { + const stderrIdx = stderr.length + await fs.writeFile( + dynamicGsspPage, + content.replace('return {', 'missingVar;return {') + ) + const browser = await webdriver(appPort, '/blog/first') + + await check(async () => { + const err = stderr.substr(stderrIdx) + + return err.includes('pages/blog/[slug].js') && + err.includes('6:2') && + err.includes('getServerSideProps') && + err.includes('missingVar') + ? 'success' + : err + }, 'success') + + expect(await hasRedbox(browser, true)).toBe(true) + + expect(await getRedboxSource(browser)).toContain('missingVar') + await fs.writeFile(dynamicGsspPage, content) + await hasRedbox(browser, false) + } finally { + await fs.writeFile(dynamicGsspPage, content) + } + }) + + it('should show server-side error for api route correctly', async () => { + const content = await fs.readFile(apiPage, 'utf8') + + try { + const stderrIdx = stderr.length + await fs.writeFile( + apiPage, + content.replace('res.status', 'missingVar;res.status') + ) + const browser = await webdriver(appPort, '/api/hello') + + await check(async () => { + const err = stderr.substr(stderrIdx) + + return err.includes('pages/api/hello.js') && + err.includes('2:2') && + err.includes('default') && + err.includes('missingVar') + ? 'success' + : err + }, 'success') + + expect(await hasRedbox(browser, true)).toBe(true) + + expect(await getRedboxSource(browser)).toContain('missingVar') + await fs.writeFile(apiPage, content) + await hasRedbox(browser, false) + } finally { + await fs.writeFile(apiPage, content) + } + }) + + it('should show server-side error for dynamic api route correctly', async () => { + const content = await fs.readFile(dynamicApiPage, 'utf8') + + try { + const stderrIdx = stderr.length + await fs.writeFile( + dynamicApiPage, + content.replace('res.status', 'missingVar;res.status') + ) + const browser = await webdriver(appPort, '/api/blog/first') + + await check(async () => { + const err = stderr.substr(stderrIdx) + + return err.includes('pages/api/blog/[slug].js') && + err.includes('2:2') && + err.includes('default') && + err.includes('missingVar') + ? 'success' + : err + }, 'success') + + expect(await hasRedbox(browser, true)).toBe(true) + + expect(await getRedboxSource(browser)).toContain('missingVar') + await fs.writeFile(dynamicApiPage, content) + await hasRedbox(browser, false) + } finally { + await fs.writeFile(dynamicApiPage, content) + } + }) + + it('should show server-side error for uncaught rejection correctly', async () => { + const stderrIdx = stderr.length + await webdriver(appPort, '/uncaught-rejection') + + await check(async () => { + const err = stderr.substr(stderrIdx) + + return err.includes('pages/uncaught-rejection.js') && + err.includes('7:19') && + err.includes('getServerSideProps') && + err.includes('catch this rejection') + ? 'success' + : err + }, 'success') + }) + + it('should show server-side error for uncaught exception correctly', async () => { + const stderrIdx = stderr.length + await webdriver(appPort, '/uncaught-exception') + + await check(async () => { + const err = stderr.substr(stderrIdx) + + return err.includes('pages/uncaught-exception.js') && + err.includes('7:10') && + err.includes('getServerSideProps') && + err.includes('catch this exception') + ? 'success' + : err + }, 'success') + }) +})