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

Use eval-source-map for Server Side Errors #13123

Merged
merged 4 commits into from
May 20, 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
6 changes: 3 additions & 3 deletions packages/next/build/webpack/config/blocks/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ export const base = curry(function base(

// https://webpack.js.org/configuration/devtool/#development
if (ctx.isDevelopment) {
if (ctx.isServer || process.platform === 'win32') {
if (process.platform === 'win32') {
// Non-eval based source maps are slow to rebuild, so we only enable
// them for the server and Windows. Unfortunately, eval source maps
// are not supported by Node.js, and are slow on Windows.
// them for Windows. Unfortunately, eval source maps are flagged as
// suspicious by Windows Defender and block HMR.
config.devtool = 'inline-source-map'
} else {
// `eval-source-map` provides full-fidelity source maps for the
Expand Down
23 changes: 18 additions & 5 deletions packages/next/server/hot-reloader.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import reactDevOverlayMiddleware from '@next/react-dev-overlay/lib/middleware'
import { getOverlayMiddleware } from '@next/react-dev-overlay/lib/middleware'
import { NextHandleFunction } from 'connect'
import { IncomingMessage, ServerResponse } from 'http'
import WebpackDevMiddleware from 'next/dist/compiled/webpack-dev-middleware'
Expand All @@ -9,7 +9,7 @@ import webpack from 'webpack'
import { createEntrypoints, createPagesMapping } from '../build/entries'
import { watchCompilers } from '../build/output'
import getBaseWebpackConfig from '../build/webpack-config'
import { NEXT_PROJECT_ROOT_DIST_CLIENT, API_ROUTE } from '../lib/constants'
import { API_ROUTE, NEXT_PROJECT_ROOT_DIST_CLIENT } from '../lib/constants'
import { recursiveDelete } from '../lib/recursive-delete'
import {
BLOCKED_PAGES,
Expand Down Expand Up @@ -133,6 +133,7 @@ export default class HotReloader {
private initialized: boolean
private config: any
private stats: any
private serverStats: any
private serverPrevDocumentHash: string | null
private prevChunkNames?: Set<any>
private onDemandEntries: any
Expand Down Expand Up @@ -160,6 +161,7 @@ export default class HotReloader {
this.webpackHotMiddleware = null
this.initialized = false
this.stats = null
this.serverStats = null
this.serverPrevDocumentHash = null

this.config = config
Expand Down Expand Up @@ -309,7 +311,11 @@ export default class HotReloader {
const buildTools = await this.prepareBuildTools(multiCompiler)
this.assignBuildTools(buildTools)

this.stats = ((await this.waitUntilValid()) as any).stats[0]
// [Client, Server]
;[
this.stats,
this.serverStats,
] = ((await this.waitUntilValid()) as any).stats
}

async stop(
Expand All @@ -328,14 +334,19 @@ export default class HotReloader {

async reload(): Promise<void> {
this.stats = null
this.serverStats = null

await this.clean()

const configs = await this.getWebpackConfig()
const compiler = webpack(configs)

const buildTools = await this.prepareBuildTools(compiler)
this.stats = await this.waitUntilValid(buildTools.webpackDevMiddleware)
// [Client, Server]
;[
this.stats,
this.serverStats,
] = ((await this.waitUntilValid()) as any).stats

const oldWebpackDevMiddleware = this.webpackDevMiddleware

Expand All @@ -361,9 +372,10 @@ export default class HotReloader {
onDemandEntries.middleware(),
webpackHotMiddleware,
errorOverlayMiddleware({ dir: this.dir }),
reactDevOverlayMiddleware({
getOverlayMiddleware({
rootDirectory: this.dir,
stats: () => this.stats,
serverStats: () => this.serverStats,
}),
]
}
Expand All @@ -375,6 +387,7 @@ export default class HotReloader {
multiCompiler.compilers[1].hooks.done.tap(
'NextjsHotReloaderForServer',
(stats) => {
this.serverStats = stats
if (!this.initialized) {
return
}
Expand Down
5 changes: 4 additions & 1 deletion packages/react-dev-overlay/src/internal/container/Errors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ async function getErrorByType(
id,
runtime: true,
error: event.reason,
frames: await getOriginalStackFrames(event.frames),
frames: await getOriginalStackFrames(
isNodeError(event.reason),
event.frames
),
}
}
default: {
Expand Down
11 changes: 9 additions & 2 deletions packages/react-dev-overlay/src/internal/helpers/stack-frame.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,22 @@ export type OriginalStackFrame =
originalCodeFrame: null
}

export function getOriginalStackFrames(frames: StackFrame[]) {
return Promise.all(frames.map((frame) => getOriginalStackFrame(frame)))
export function getOriginalStackFrames(
isServerSide: boolean,
frames: StackFrame[]
) {
return Promise.all(
frames.map((frame) => getOriginalStackFrame(isServerSide, frame))
)
}

export function getOriginalStackFrame(
isServerSide: boolean,
source: StackFrame
): Promise<OriginalStackFrame> {
async function _getOriginalStackFrame(): Promise<OriginalStackFrame> {
const params = new URLSearchParams()
params.append('isServerSide', String(isServerSide))
for (const key in source) {
params.append(key, (source[key] ?? '').toString())
}
Expand Down
24 changes: 19 additions & 5 deletions packages/react-dev-overlay/src/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { launchEditor } from './internal/helpers/launchEditor'
export type OverlayMiddlewareOptions = {
rootDirectory: string
stats(): webpack.Stats
serverStats(): webpack.Stats
}

export type OriginalStackFrameResponse = {
Expand All @@ -26,7 +27,11 @@ export type OriginalStackFrameResponse = {
type Source = { map: () => RawSourceMap } | null

function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
async function getSourceById(isFile: boolean, id: string): Promise<Source> {
async function getSourceById(
isServerSide: boolean,
isFile: boolean,
id: string
): Promise<Source> {
if (isFile) {
const fileContent: string | null = await fs
.readFile(id, 'utf-8')
Expand All @@ -49,7 +54,9 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
}

try {
const compilation = options.stats()?.compilation
const compilation = isServerSide
? options.serverStats()?.compilation
: options.stats()?.compilation
const m = compilation?.modules?.find((m) => m.id === id)
return (
m?.source(
Expand All @@ -71,7 +78,9 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
const { pathname, query } = url.parse(req.url, true)

if (pathname === '/__nextjs_original-stack-frame') {
const frame = (query as unknown) as StackFrame
const frame = (query as unknown) as StackFrame & {
isServerSide: 'true' | 'false'
}
if (
!(
(frame.file?.startsWith('webpack-internal:///') ||
Expand All @@ -84,14 +93,19 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
return res.end()
}

const isServerSide = frame.isServerSide === 'true'
const moduleId: string = frame.file.replace(
/^(webpack-internal:\/\/\/|file:\/\/)/,
''
)

let source: Source
try {
source = await getSourceById(frame.file.startsWith('file:'), moduleId)
source = await getSourceById(
isServerSide,
frame.file.startsWith('file:'),
moduleId
)
} catch (err) {
console.log('Failed to get source map:', err)
res.statusCode = 500
Expand Down Expand Up @@ -212,4 +226,4 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
}
}

export default getOverlayMiddleware
export { getOverlayMiddleware }
2 changes: 1 addition & 1 deletion test/acceptance/ReactRefreshLogBox.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ test('module init error not shown', async () => {

expect(await session.hasRedbox(true)).toBe(true)
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"index.js (4:12) @ Module../index.js
"index.js (4:12) @ eval

2 | // top offset for snapshot
3 | import * as React from 'react';
Expand Down