Skip to content

Commit f1dbc92

Browse files
authored
Ensure dev server side errors are correct (#28520)
1 parent 27c2937 commit f1dbc92

File tree

18 files changed

+476
-54
lines changed

18 files changed

+476
-54
lines changed

packages/next/client/dev/on-demand-entries-client.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ export default async ({ assetPrefix }) => {
99
)
1010
})
1111

12-
setupPing(assetPrefix, () => Router.pathname, currentPage)
12+
setupPing(
13+
assetPrefix,
14+
() => Router.query.__NEXT_PAGE || Router.pathname,
15+
currentPage
16+
)
1317

1418
// prevent HMR connection from being closed when running tests
1519
if (!process.env.__NEXT_TEST_MODE) {

packages/next/client/dev/on-demand-entries-utils.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ export function setupPing(assetPrefix, pathnameFn, retry) {
2929
if (event.data.indexOf('{') === -1) return
3030
try {
3131
const payload = JSON.parse(event.data)
32-
if (payload.invalid) {
32+
// don't attempt fetching the page if we're already showing
33+
// the dev overlay as this can cause the error to be triggered
34+
// repeatedly
35+
if (payload.invalid && !self.__NEXT_DATA__.err) {
3336
// Payload can be invalid even if the page does not exist.
3437
// So, we need to make sure it exists before reloading.
3538
fetch(location.href, {

packages/next/client/next-dev.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ initNext({ webpackHMR })
5959
} else if (event.data.indexOf('serverOnlyChanges') !== -1) {
6060
const { pages } = JSON.parse(event.data)
6161

62+
// Make sure to reload when the dev-overlay is showing for an
63+
// API route
64+
if (pages.includes(router.query.__NEXT_PAGE)) {
65+
return window.location.reload()
66+
}
67+
6268
if (!router.clc && pages.includes(router.pathname)) {
6369
console.log('Refreshing page data due to server-side change')
6470

packages/next/server/api-utils.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ export async function apiResolver(
2525
query: any,
2626
resolverModule: any,
2727
apiContext: __ApiPreviewProps,
28-
propagateError: boolean
28+
propagateError: boolean,
29+
dev?: boolean,
30+
page?: string
2931
): Promise<void> {
3032
const apiReq = req as NextApiRequest
3133
const apiRes = res as NextApiResponse
@@ -117,6 +119,13 @@ export async function apiResolver(
117119
if (err instanceof ApiError) {
118120
sendError(apiRes, err.statusCode, err.message)
119121
} else {
122+
if (dev) {
123+
if (err) {
124+
err.page = page
125+
}
126+
throw err
127+
}
128+
120129
console.error(err)
121130
if (propagateError) {
122131
throw err

packages/next/server/dev/hot-reloader.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ export default class HotReloader {
134134
private webpackHotMiddleware: (NextHandleFunction & any) | null
135135
private config: NextConfigComplete
136136
private stats: webpack.Stats | null
137-
private serverStats: webpack.Stats | null
137+
public serverStats: webpack.Stats | null
138138
private clientError: Error | null = null
139139
private serverError: Error | null = null
140140
private serverPrevDocumentHash: string | null

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

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import crypto from 'crypto'
22
import fs from 'fs'
3+
import chalk from 'chalk'
34
import { IncomingMessage, ServerResponse } from 'http'
45
import { Worker } from 'jest-worker'
56
import AmpHtmlValidator from 'next/dist/compiled/amphtml-validator'
@@ -47,6 +48,12 @@ import {
4748
loadDefaultErrorComponents,
4849
} from '../load-components'
4950
import { DecodeError } from '../../shared/lib/utils'
51+
import { parseStack } from '@next/react-dev-overlay/lib/internal/helpers/parseStack'
52+
import {
53+
createOriginalStackFrame,
54+
getSourceById,
55+
} from '@next/react-dev-overlay/lib/middleware'
56+
import * as Log from '../../build/output/log'
5057

5158
// Load ReactDevOverlay only when needed
5259
let ReactDevOverlayImpl: React.FunctionComponent
@@ -325,6 +332,15 @@ export default class DevServer extends Server {
325332
)
326333
// This is required by the tracing subsystem.
327334
setGlobal('telemetry', telemetry)
335+
336+
process.on('unhandledRejection', (reason) => {
337+
this.logErrorWithOriginalStack(reason, 'unhandledRejection').catch(
338+
() => {}
339+
)
340+
})
341+
process.on('uncaughtException', (err) => {
342+
this.logErrorWithOriginalStack(err, 'uncaughtException').catch(() => {})
343+
})
328344
}
329345

330346
protected async close(): Promise<void> {
@@ -431,8 +447,85 @@ export default class DevServer extends Server {
431447
// if they should match against the basePath or not
432448
parsedUrl.pathname = originalPathname
433449
}
450+
try {
451+
return await super.run(req, res, parsedUrl)
452+
} catch (err) {
453+
res.statusCode = 500
454+
try {
455+
this.logErrorWithOriginalStack(err).catch(() => {})
456+
return await this.renderError(err, req, res, pathname!, {
457+
__NEXT_PAGE: err?.page || pathname,
458+
})
459+
} catch (internalErr) {
460+
console.error(internalErr)
461+
res.end('Internal Server Error')
462+
}
463+
}
464+
}
434465

435-
return super.run(req, res, parsedUrl)
466+
private async logErrorWithOriginalStack(
467+
possibleError?: any,
468+
type?: 'unhandledRejection' | 'uncaughtException'
469+
) {
470+
let usedOriginalStack = false
471+
472+
if (possibleError?.name && possibleError?.stack && possibleError?.message) {
473+
const err: Error & { stack: string } = possibleError
474+
try {
475+
const frames = parseStack(err.stack)
476+
const frame = frames[0]
477+
478+
if (frame.lineNumber && frame?.file) {
479+
const compilation = this.hotReloader?.serverStats?.compilation
480+
const moduleId = frame.file!.replace(
481+
/^(webpack-internal:\/\/\/|file:\/\/)/,
482+
''
483+
)
484+
485+
const source = await getSourceById(
486+
!!frame.file?.startsWith(sep) || !!frame.file?.startsWith('file:'),
487+
moduleId,
488+
compilation,
489+
this.hotReloader!.isWebpack5
490+
)
491+
492+
const originalFrame = await createOriginalStackFrame({
493+
line: frame.lineNumber!,
494+
column: frame.column,
495+
source,
496+
frame,
497+
modulePath: moduleId,
498+
rootDirectory: this.dir,
499+
})
500+
501+
if (originalFrame) {
502+
const { originalCodeFrame, originalStackFrame } = originalFrame
503+
const { file, lineNumber, column, methodName } = originalStackFrame
504+
505+
console.error(
506+
chalk.red('error') +
507+
' - ' +
508+
`${file} (${lineNumber}:${column}) @ ${methodName}`
509+
)
510+
console.error(`${chalk.red(err.name)}: ${err.message}`)
511+
console.error(originalCodeFrame)
512+
usedOriginalStack = true
513+
}
514+
}
515+
} catch (_) {
516+
// failed to load original stack using source maps
517+
// this un-actionable by users so we don't show the
518+
// internal error and only show the provided stack
519+
}
520+
}
521+
522+
if (!usedOriginalStack) {
523+
if (type) {
524+
Log.error(`${type}:`, possibleError)
525+
} else {
526+
Log.error(possibleError)
527+
}
528+
}
436529
}
437530

438531
// override production loading of routes-manifest

packages/next/server/next-server.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ export default class Server {
487487
try {
488488
return await this.run(req, res, parsedUrl)
489489
} catch (err) {
490-
if (this.minimalMode) {
490+
if (this.minimalMode || this.renderOpts.dev) {
491491
throw err
492492
}
493493
this.logError(err)
@@ -1125,7 +1125,9 @@ export default class Server {
11251125
query,
11261126
pageModule,
11271127
this.renderOpts.previewProps,
1128-
this.minimalMode
1128+
this.minimalMode,
1129+
this.renderOpts.dev,
1130+
page
11291131
)
11301132
return true
11311133
}
@@ -1857,6 +1859,7 @@ export default class Server {
18571859
ctx: RequestContext
18581860
): Promise<ResponsePayload | null> {
18591861
const { res, query, pathname } = ctx
1862+
let page = pathname
18601863
const bubbleNoFallback = !!query._nextBubbleNoFallback
18611864
delete query._nextBubbleNoFallback
18621865

@@ -1888,6 +1891,7 @@ export default class Server {
18881891
)
18891892
if (dynamicRouteResult) {
18901893
try {
1894+
page = dynamicRoute.page
18911895
return await this.renderToResponseWithComponents(
18921896
{
18931897
...ctx,
@@ -1929,7 +1933,10 @@ export default class Server {
19291933
)
19301934

19311935
if (!isWrappedError) {
1932-
if (this.minimalMode) {
1936+
if (this.minimalMode || this.renderOpts.dev) {
1937+
if (err) {
1938+
err.page = page
1939+
}
19331940
throw err
19341941
}
19351942
this.logError(err)

packages/next/server/render.tsx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -822,10 +822,7 @@ export async function renderToHTML(
822822
;(renderOpts as any).pageData = props
823823
}
824824
} catch (dataFetchError) {
825-
if (isDataReq || !dev || !dataFetchError) throw dataFetchError
826-
ctx.err = dataFetchError
827-
renderOpts.err = dataFetchError
828-
console.error(dataFetchError)
825+
throw dataFetchError
829826
}
830827

831828
if (

packages/react-dev-overlay/src/middleware.ts

Lines changed: 43 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -177,52 +177,50 @@ export async function createOriginalStackFrame({
177177
}
178178
}
179179

180-
function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
181-
async function getSourceById(
182-
isServerSide: boolean,
183-
isFile: boolean,
184-
id: string
185-
): Promise<Source> {
186-
if (isFile) {
187-
const fileContent: string | null = await fs
188-
.readFile(id, 'utf-8')
189-
.catch(() => null)
190-
191-
if (fileContent == null) {
192-
return null
193-
}
194-
195-
const map = getRawSourceMap(fileContent)
196-
if (map == null) {
197-
return null
198-
}
180+
export async function getSourceById(
181+
isFile: boolean,
182+
id: string,
183+
compilation: any,
184+
isWebpack5: boolean
185+
): Promise<Source> {
186+
if (isFile) {
187+
const fileContent: string | null = await fs
188+
.readFile(id, 'utf-8')
189+
.catch(() => null)
190+
191+
if (fileContent == null) {
192+
return null
193+
}
199194

200-
return {
201-
map() {
202-
return map
203-
},
204-
}
195+
const map = getRawSourceMap(fileContent)
196+
if (map == null) {
197+
return null
205198
}
206199

207-
try {
208-
const compilation = isServerSide
209-
? options.serverStats()?.compilation
210-
: options.stats()?.compilation
211-
if (compilation == null) {
212-
return null
213-
}
200+
return {
201+
map() {
202+
return map
203+
},
204+
}
205+
}
214206

215-
const module = [...compilation.modules].find(
216-
(searchModule) =>
217-
getModuleId(compilation, searchModule, options.isWebpack5) === id
218-
)
219-
return getModuleSource(compilation, module, options.isWebpack5)
220-
} catch (err) {
221-
console.error(`Failed to lookup module by ID ("${id}"):`, err)
207+
try {
208+
if (compilation == null) {
222209
return null
223210
}
211+
212+
const module = [...compilation.modules].find(
213+
(searchModule) =>
214+
getModuleId(compilation, searchModule, isWebpack5) === id
215+
)
216+
return getModuleSource(compilation, module, isWebpack5)
217+
} catch (err) {
218+
console.error(`Failed to lookup module by ID ("${id}"):`, err)
219+
return null
224220
}
221+
}
225222

223+
function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
226224
return async function (
227225
req: IncomingMessage,
228226
res: ServerResponse,
@@ -254,10 +252,15 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
254252

255253
let source: Source
256254
try {
255+
const compilation = isServerSide
256+
? options.serverStats()?.compilation
257+
: options.stats()?.compilation
258+
257259
source = await getSourceById(
258-
isServerSide,
259260
frame.file.startsWith('file:'),
260-
moduleId
261+
moduleId,
262+
compilation,
263+
!!options.isWebpack5
261264
)
262265
} catch (err) {
263266
console.log('Failed to get source map:', err)

test/integration/api-support/test/index.test.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,24 @@ function runTests(dev = false) {
109109
const res = await fetchViaHTTP(appPort, '/api/user-error', null, {})
110110
const text = await res.text()
111111
expect(res.status).toBe(500)
112-
expect(text).toBe('Internal Server Error')
112+
113+
if (dev) {
114+
expect(text).toContain('User error')
115+
} else {
116+
expect(text).toBe('Internal Server Error')
117+
}
113118
})
114119

115120
it('should throw Internal Server Error (async)', async () => {
116121
const res = await fetchViaHTTP(appPort, '/api/user-error-async', null, {})
117122
const text = await res.text()
118123
expect(res.status).toBe(500)
119-
expect(text).toBe('Internal Server Error')
124+
125+
if (dev) {
126+
expect(text).toContain('User error')
127+
} else {
128+
expect(text).toBe('Internal Server Error')
129+
}
120130
})
121131

122132
it('should parse JSON body', async () => {
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default function handler(req, res) {
2+
res.status(200).json({ slug: req.query.slug })
3+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default function handler(req, res) {
2+
res.status(200).json({ hello: 'world' })
3+
}

0 commit comments

Comments
 (0)