diff --git a/client/index.js b/client/index.js index 149e222d..942a2079 100644 --- a/client/index.js +++ b/client/index.js @@ -1,4 +1,4 @@ -import { createElement } from 'react' +import React from 'react' import ReactDOM from 'react-dom' import HeadManager from './head-manager' import { createRouter } from '../lib/router' @@ -66,33 +66,44 @@ const errorContainer = document.getElementById('__next-error') let lastAppProps export let router export let ErrorComponent +let HotAppContainer let ErrorDebugComponent let Component let App let stripAnsi = (s) => s +let applySourcemaps = (e) => e export const emitter = new EventEmitter() -export default async ({ ErrorDebugComponent: passedDebugComponent, stripAnsi: passedStripAnsi } = {}) => { +export default async ({ + HotAppContainer: passedHotAppContainer, + ErrorDebugComponent: passedDebugComponent, + stripAnsi: passedStripAnsi, + applySourcemaps: passedApplySourcemaps +} = {}) => { // Wait for all the dynamic chunks to get loaded for (const chunkName of chunks) { await pageLoader.waitForChunk(chunkName) } stripAnsi = passedStripAnsi || stripAnsi + applySourcemaps = passedApplySourcemaps || applySourcemaps + HotAppContainer = passedHotAppContainer ErrorDebugComponent = passedDebugComponent ErrorComponent = await pageLoader.loadPage('/_error') App = await pageLoader.loadPage('/_app') + let initialErr = err + try { Component = await pageLoader.loadPage(page) if (typeof Component !== 'function') { throw new Error(`The default export is not a React Component in page: "${pathname}"`) } - } catch (err) { - console.error(stripAnsi(`${err.message}\n${err.stack}`)) - Component = ErrorComponent + } catch (error) { + // This catches errors like throwing in the top level of a module + initialErr = error } router = createRouter(pathname, query, asPath, { @@ -101,7 +112,7 @@ export default async ({ ErrorDebugComponent: passedDebugComponent, stripAnsi: pa App, Component, ErrorComponent, - err + err: initialErr }) router.subscribe(({ Component, props, hash, err }) => { @@ -109,14 +120,14 @@ export default async ({ ErrorDebugComponent: passedDebugComponent, stripAnsi: pa }) const hash = location.hash.substring(1) - render({ Component, props, hash, err, emitter }) + render({ Component, props, hash, err: initialErr, emitter }) return emitter } export async function render (props) { if (props.err) { - await renderError(props.err) + await renderError(props) return } @@ -124,31 +135,37 @@ export async function render (props) { await doRender(props) } catch (err) { if (err.abort) return - await renderError(err) + await renderError({...props, err}) } } // This method handles all runtime and debug errors. // 404 and 500 errors are special kind of errors // and they are still handle via the main render method. -export async function renderError (error) { - const prod = process.env.NODE_ENV === 'production' - // We need to unmount the current app component because it's - // in the inconsistant state. - // Otherwise, we need to face issues when the issue is fixed and - // it's get notified via HMR - ReactDOM.unmountComponentAtNode(appContainer) +export async function renderError (props) { + const {err} = props - const errorMessage = `${error.message}\n${error.stack}` - console.error(stripAnsi(errorMessage)) - - if (prod) { - const initProps = {Component: ErrorComponent, router, ctx: {err: error, pathname, query, asPath}} - const props = await loadGetInitialProps(ErrorComponent, initProps) - renderReactElement(createElement(ErrorComponent, props), errorContainer) - } else { - renderReactElement(createElement(ErrorDebugComponent, { error }), errorContainer) + // In development we apply sourcemaps to the error + if (process.env.NODE_ENV !== 'production') { + await applySourcemaps(err) } + + const str = stripAnsi(`${err.message}\n${err.stack}${err.info ? `\n\n${err.info.componentStack}` : ''}`) + console.error(str) + + if (process.env.NODE_ENV !== 'production') { + // We need to unmount the current app component because it's + // in the inconsistant state. + // Otherwise, we need to face issues when the issue is fixed and + // it's get notified via HMR + ReactDOM.unmountComponentAtNode(appContainer) + renderReactElement(, errorContainer) + return + } + + // In production we do a normal render with the `ErrorComponent` as component. + // `App` will handle the calling of `getInitialProps`, which will include the `err` on the context + await doRender({...props, err, Component: ErrorComponent}) } async function doRender ({ Component, props, hash, err, emitter: emitterProp = emitter }) { @@ -172,7 +189,15 @@ async function doRender ({ Component, props, hash, err, emitter: emitterProp = e // We need to clear any existing runtime error messages ReactDOM.unmountComponentAtNode(errorContainer) - renderReactElement(createElement(App, appProps), appContainer) + + // In development we render react-hot-loader's wrapper component + if (HotAppContainer) { + renderReactElement( + + , appContainer) + } else { + renderReactElement(, appContainer) + } emitterProp.emit('after-reactdom-render', { Component, ErrorComponent, appProps }) } diff --git a/client/next-dev.js b/client/next-dev.js index 7f5e13e0..07e86f79 100644 --- a/client/next-dev.js +++ b/client/next-dev.js @@ -1,14 +1,14 @@ import stripAnsi from 'strip-ansi' import initNext, * as next from './' -import ErrorDebugComponent from '../lib/error-debug' +import {ClientDebug} from '../lib/error-debug' import initOnDemandEntries from './on-demand-entries-client' import initWebpackHMR from './webpack-hot-middleware-client' - -require('@zeit/source-map-support/browser-source-map-support') +import {AppContainer as HotAppContainer} from 'react-hot-loader' +import {applySourcemaps} from './source-map-support' window.next = next -initNext({ ErrorDebugComponent, stripAnsi }) +initNext({ HotAppContainer, ErrorDebugComponent: ClientDebug, applySourcemaps, stripAnsi }) .then((emitter) => { initOnDemandEntries() initWebpackHMR() diff --git a/client/source-map-support.js b/client/source-map-support.js new file mode 100644 index 00000000..f29526fe --- /dev/null +++ b/client/source-map-support.js @@ -0,0 +1,54 @@ +// @flow +import fetch from 'unfetch' +const filenameRE = /\(([^)]+\.js):(\d+):(\d+)\)$/ + +export async function applySourcemaps (e: any): Promise { + if (!e || typeof e.stack !== 'string' || e.sourceMapsApplied) { + return + } + + const lines = e.stack.split('\n') + + const result = await Promise.all(lines.map((line) => { + return rewriteTraceLine(line) + })) + + e.stack = result.join('\n') + // This is to make sure we don't apply the sourcemaps twice on the same object + e.sourceMapsApplied = true +} + +async function rewriteTraceLine (trace: string): Promise { + const m = trace.match(filenameRE) + if (m == null) { + return trace + } + + const filePath = m[1] + if (filePath.match(/node_modules/)) { + return trace + } + + const mapPath = `${filePath}.map` + + const res = await fetch(mapPath) + if (res.status !== 200) { + return trace + } + + const mapContents = await res.json() + const {SourceMapConsumer} = require('source-map') + const map = new SourceMapConsumer(mapContents) + const originalPosition = map.originalPositionFor({ + line: Number(m[2]), + column: Number(m[3]) + }) + + if (originalPosition.source != null) { + const { source, line, column } = originalPosition + const mappedPosition = `(${source.replace(/^webpack:\/\/\//, '')}:${String(line)}:${String(column)})` + return trace.replace(filenameRE, mappedPosition) + } + + return trace +} diff --git a/lib/app.js b/lib/app.js index c749f9ef..df899cf7 100644 --- a/lib/app.js +++ b/lib/app.js @@ -5,10 +5,6 @@ import { execOnce, warn, loadGetInitialProps } from './utils' import { makePublicRouterInstance } from './router' export default class App extends Component { - state = { - hasError: null - } - static displayName = 'App' static async getInitialProps ({ Component, router, ctx }) { @@ -24,18 +20,25 @@ export default class App extends Component { getChildContext () { const { headManager } = this.props - const {hasError} = this.state return { headManager, router: makePublicRouterInstance(this.props.router), - _containerProps: {...this.props, hasError} + _containerProps: {...this.props} } } - componentDidCatch (error, info) { - error.stack = `${error.stack}\n\n${info.componentStack}` - window.next.renderError(error) - this.setState({ hasError: true }) + componentDidCatch (err, info) { + // To provide clearer stacktraces in error-debug.js in development + // To provide clearer stacktraces in app.js in production + err.info = info + + if (process.env.NODE_ENV === 'production') { + // In production we render _error.js + window.next.renderError({err}) + } else { + // In development we throw the error up to AppContainer from react-hot-loader + throw err + } } render () { @@ -78,28 +81,8 @@ export class Container extends Component { } render () { - const { hasError } = this.context._containerProps - - if (hasError) { - return null - } - const {children} = this.props - - if (process.env.NODE_ENV === 'production') { - return <>{children} - } else { - const ErrorDebug = require('./error-debug').default - const { AppContainer } = require('react-hot-loader') - - // includes AppContainer which bypasses shouldComponentUpdate method - // https://github.com/gaearon/react-hot-loader/issues/442 - return ( - - {children} - - ) - } + return <>{children} } } diff --git a/lib/error-debug.js b/lib/error-debug.js index ecb04c39..a3ca80e1 100644 --- a/lib/error-debug.js +++ b/lib/error-debug.js @@ -1,27 +1,68 @@ import React from 'react' import ansiHTML from 'ansi-html' import Head from './head' +import {applySourcemaps} from '../client/source-map-support' -export default ({ error, error: { name, message, module } }) => ( -
- - - - {module ?

Error in {module.rawRequest}

: null} - { - name === 'ModuleBuildError' - ?
-        : 
+// On the client side the error can come from multiple places for example react-hot-loader or client/index.js
+// `componentDidCatch` doesn't support asynchronous execution, so we have to handle sourcemap support here
+export class ClientDebug extends React.Component {
+  state = {
+    mappedError: null
+  }
+  componentDidMount () {
+    const {error} = this.props
+
+    // If sourcemaps were already applied there is no need to set the state
+    if (error.sourceMapsApplied) {
+      return
     }
-  
-) -const StackTrace = ({ error: { name, message, stack } }) => ( + // Since componentDidMount doesn't handle errors we use then/catch here + applySourcemaps(error).then(() => { + this.setState({mappedError: error}) + }).catch(console.error) + } + + render () { + const {mappedError} = this.state + const {error} = this.props + if (!error.sourceMapsApplied && mappedError === null) { + return
+

Loading stacktrace...

+
+ } + + return + } +} + +// On the server side the error has sourcemaps already applied, so `ErrorDebug` is rendered directly. +export default function ErrorDebug ({error}) { + const { name, message, module } = error + return ( +
+ + + + {module ?

Error in {module.rawRequest}

: null} + { + name === 'ModuleBuildError' + ?
+          : 
+      }
+    
+ ) +} + +const StackTrace = ({ error: { name, message, stack, info } }) => (
{message || name}
       {stack}
     
+ {info &&
+      {info.componentStack}
+    
}
) @@ -36,7 +77,8 @@ const styles = { right: 0, top: 0, bottom: 0, - zIndex: 9999 + zIndex: 9999, + color: '#b3adac' }, stack: { diff --git a/package.json b/package.json index eec7f5ad..64326a98 100644 --- a/package.json +++ b/package.json @@ -64,7 +64,6 @@ "@babel/runtime": "7.0.0-beta.42", "@babel/template": "7.0.0-beta.42", "@zeit/check-updates": "1.1.1", - "@zeit/source-map-support": "0.6.2", "ansi-html": "0.0.7", "babel-core": "7.0.0-bridge.0", "babel-loader": "8.0.0-beta.2", diff --git a/server/build/babel/preset.js b/server/build/babel/preset.js index 60007844..57a7fa95 100644 --- a/server/build/babel/preset.js +++ b/server/build/babel/preset.js @@ -23,17 +23,6 @@ function styledJsxOptions (opts) { return opts } -const envPlugins = { - 'development': [ - require('@babel/plugin-transform-react-jsx-source') - ], - 'production': [ - require('babel-plugin-transform-react-remove-prop-types') - ] -} - -const plugins = envPlugins[process.env.NODE_ENV] || envPlugins['development'] - module.exports = (context, opts = {}) => ({ presets: [ [require('@babel/preset-env'), { @@ -53,6 +42,6 @@ module.exports = (context, opts = {}) => ({ regenerator: true }], [require('styled-jsx/babel'), styledJsxOptions(opts['styled-jsx'])], - ...plugins - ] + process.env.NODE_ENV === 'production' && require('babel-plugin-transform-react-remove-prop-types') + ].filter(Boolean) }) diff --git a/server/build/webpack.js b/server/build/webpack.js index 33c65ee5..d5c0971c 100644 --- a/server/build/webpack.js +++ b/server/build/webpack.js @@ -16,6 +16,7 @@ import BuildManifestPlugin from './plugins/build-manifest-plugin' const presetItem = createConfigItem(require('./babel/preset'), {type: 'preset'}) const hotLoaderItem = createConfigItem(require('react-hot-loader/babel'), {type: 'plugin'}) +const reactJsxSourceItem = createConfigItem(require('@babel/plugin-transform-react-jsx-source'), {type: 'plugin'}) const nextDir = path.join(__dirname, '..', '..', '..') const nextNodeModulesDir = path.join(nextDir, 'node_modules') @@ -34,7 +35,8 @@ function babelConfig (dir, {isServer, dev}) { cacheDirectory: true, presets: [], plugins: [ - dev && !isServer && hotLoaderItem + dev && !isServer && hotLoaderItem, + dev && reactJsxSourceItem ].filter(Boolean) } @@ -119,7 +121,7 @@ export default async function getBaseWebpackConfig (dir, {dev = false, isServer } : {} let webpackConfig = { - devtool: dev ? 'source-map' : false, + devtool: dev ? 'cheap-module-source-map' : false, name: isServer ? 'server' : 'client', cache: true, target: isServer ? 'node' : 'web', @@ -139,14 +141,7 @@ export default async function getBaseWebpackConfig (dir, {dev = false, isServer libraryTarget: 'commonjs2', // This saves chunks with the name given via require.ensure() chunkFilename: '[name]-[chunkhash].js', - strictModuleExceptionHandling: true, - devtoolModuleFilenameTemplate (info) { - if (dev) { - return info.absoluteResourcePath - } - - return `${info.absoluteResourcePath.replace(dir, '.').replace(nextDir, './node_modules/next')}` - } + strictModuleExceptionHandling: true }, performance: { hints: false }, resolve: { diff --git a/server/index.js b/server/index.js index 05f1f2b2..09e2de17 100644 --- a/server/index.js +++ b/server/index.js @@ -1,5 +1,4 @@ /* eslint-disable import/first, no-return-await */ -require('@zeit/source-map-support').install() import { resolve, join, sep } from 'path' import { parse as parseUrl } from 'url' import { parse as parseQs } from 'querystring' @@ -337,6 +336,8 @@ export default class Server { res.statusCode = 404 return this.renderErrorToHTML(null, req, res, pathname, query) } else { + const {applySourcemaps} = require('./lib/source-map-support') + await applySourcemaps(err) if (!this.quiet) console.error(err) res.statusCode = 500 return this.renderErrorToHTML(err, req, res, pathname, query) diff --git a/server/lib/source-map-support.js b/server/lib/source-map-support.js new file mode 100644 index 00000000..fdd6c466 --- /dev/null +++ b/server/lib/source-map-support.js @@ -0,0 +1,57 @@ +// @flow +const filenameRE = /\(([^)]+\.js):(\d+):(\d+)\)$/ + +export async function applySourcemaps (e: any): Promise { + if (!e || typeof e.stack !== 'string' || e.sourceMapsApplied) { + return + } + + const lines = e.stack.split('\n') + + const result = await Promise.all(lines.map((line) => { + return rewriteTraceLine(line) + })) + + e.stack = result.join('\n') + // This is to make sure we don't apply the sourcemaps twice on the same object + e.sourceMapsApplied = true +} + +async function rewriteTraceLine (trace: string): Promise { + const m = trace.match(filenameRE) + if (m == null) { + return trace + } + + const filePath = m[1] + const mapPath = `${filePath}.map` + + // Load these on demand. + const fs = require('fs') + const promisify = require('./promisify') + + const readFile = promisify(fs.readFile) + const access = promisify(fs.access) + + try { + await access(mapPath, (fs.constants || fs).R_OK) + } catch (err) { + return trace + } + + const mapContents = await readFile(mapPath) + const {SourceMapConsumer} = require('source-map') + const map = new SourceMapConsumer(JSON.parse(mapContents)) + const originalPosition = map.originalPositionFor({ + line: Number(m[2]), + column: Number(m[3]) + }) + + if (originalPosition.source != null) { + const { source, line, column } = originalPosition + const mappedPosition = `(${source.replace(/^webpack:\/\/\//, '')}:${String(line)}:${String(column)})` + return trace.replace(filenameRE, mappedPosition) + } + + return trace +} diff --git a/server/render.js b/server/render.js index 4da7bd03..016e465a 100644 --- a/server/render.js +++ b/server/render.js @@ -12,6 +12,7 @@ import Head, { defaultHead } from '../lib/head' import ErrorDebug from '../lib/error-debug' import { flushChunks } from '../lib/dynamic' import { BUILD_MANIFEST } from '../lib/constants' +import { applySourcemaps } from './lib/source-map-support' const logger = console @@ -49,6 +50,8 @@ async function doRender (req, res, pathname, query, { } = {}) { page = page || pathname + await applySourcemaps(err) + if (hotReloader) { // In dev mode we use on demand entries to compile the page before rendering await ensurePage(page, { dir, hotReloader }) } @@ -95,7 +98,7 @@ async function doRender (req, res, pathname, query, { if (err && dev) { errorHtml = render(createElement(ErrorDebug, { error: err })) } else if (err) { - errorHtml = render(app) + html = render(app) } else { html = render(app) } diff --git a/test/integration/basic/pages/error-in-the-browser-global-scope.js b/test/integration/basic/pages/error-in-the-browser-global-scope.js new file mode 100644 index 00000000..e9a5b0e9 --- /dev/null +++ b/test/integration/basic/pages/error-in-the-browser-global-scope.js @@ -0,0 +1,5 @@ +if (typeof window !== 'undefined') { + throw new Error('An Expected error occured') +} + +export default () =>
diff --git a/test/integration/basic/pages/error-inside-browser-page.js b/test/integration/basic/pages/error-inside-browser-page.js new file mode 100644 index 00000000..0047b6c2 --- /dev/null +++ b/test/integration/basic/pages/error-inside-browser-page.js @@ -0,0 +1,9 @@ +import React from 'react' +export default class ErrorInRenderPage extends React.Component { + render () { + if (typeof window !== 'undefined') { + throw new Error('An Expected error occured') + } + return
+ } +} diff --git a/test/integration/basic/test/client-navigation.js b/test/integration/basic/test/client-navigation.js index 7fbec21b..192cc523 100644 --- a/test/integration/basic/test/client-navigation.js +++ b/test/integration/basic/test/client-navigation.js @@ -1,6 +1,7 @@ /* global describe, it, expect */ import webdriver from 'next-webdriver' +import {waitFor} from 'next-test-utils' export default (context, render) => { describe('Client Navigation', () => { @@ -454,6 +455,24 @@ export default (context, render) => { }) }) + describe('runtime errors', () => { + it('should show ErrorDebug when a client side error is thrown inside a component', async () => { + const browser = await webdriver(context.appPort, '/error-inside-browser-page') + await waitFor(2000) + const text = await browser.elementByCss('body').text() + expect(text).toMatch(/An Expected error occured/) + expect(text).toMatch(/pages\/error-inside-browser-page\.js:5:0/) + }) + + it('should show ErrorDebug when a client side error is thrown outside a component', async () => { + const browser = await webdriver(context.appPort, '/error-in-the-browser-global-scope') + await waitFor(2000) + const text = await browser.elementByCss('body').text() + expect(text).toMatch(/An Expected error occured/) + expect(text).toMatch(/pages\/error-in-the-browser-global-scope\.js:2:0/) + }) + }) + describe('with 404 pages', () => { it('should 404 on not existent page', async () => { const browser = await webdriver(context.appPort, '/non-existent') diff --git a/test/integration/basic/test/rendering.js b/test/integration/basic/test/rendering.js index 239ba3a2..3ac0a82b 100644 --- a/test/integration/basic/test/rendering.js +++ b/test/integration/basic/test/rendering.js @@ -104,11 +104,14 @@ export default function ({ app }, suiteName, render, fetch) { test('error-inside-page', async () => { const $ = await get$('/error-inside-page') expect($('pre').text()).toMatch(/This is an expected error/) + // Check if the the source map line is correct + expect($('body').text()).toMatch(/pages\/error-inside-page\.js:2:0/) }) test('error-in-the-global-scope', async () => { const $ = await get$('/error-in-the-global-scope') expect($('pre').text()).toMatch(/aa is not defined/) + expect($('body').text()).toMatch(/pages\/error-in-the-global-scope\.js:1:0/) }) test('asPath', async () => { diff --git a/test/integration/production/pages/error-in-browser-render.js b/test/integration/production/pages/error-in-browser-render.js new file mode 100644 index 00000000..0047b6c2 --- /dev/null +++ b/test/integration/production/pages/error-in-browser-render.js @@ -0,0 +1,9 @@ +import React from 'react' +export default class ErrorInRenderPage extends React.Component { + render () { + if (typeof window !== 'undefined') { + throw new Error('An Expected error occured') + } + return
+ } +} diff --git a/test/integration/production/pages/error-in-ssr-render.js b/test/integration/production/pages/error-in-ssr-render.js new file mode 100644 index 00000000..9b2d4568 --- /dev/null +++ b/test/integration/production/pages/error-in-ssr-render.js @@ -0,0 +1,6 @@ +import React from 'react' +export default class ErrorInRenderPage extends React.Component { + render () { + throw new Error('An Expected error occured') + } +} diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index 8402d449..f298ad93 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -74,6 +74,26 @@ describe('Production Usage', () => { }) }) + describe('Runtime errors', () => { + it('should render a server side error on the client side', async () => { + const browser = await webdriver(appPort, '/error-in-ssr-render') + await waitFor(2000) + const text = await browser.elementByCss('body').text() + // this makes sure we don't leak the actual error to the client side in production + expect(text).toMatch(/Internal Server Error\./) + const headingText = await browser.elementByCss('h1').text() + // This makes sure we render statusCode on the client side correctly + expect(headingText).toBe('500') + }) + + it('should render a client side component error', async () => { + const browser = await webdriver(appPort, '/error-in-browser-render') + await waitFor(2000) + const text = await browser.elementByCss('body').text() + expect(text).toMatch(/An unexpected error has occurred\./) + }) + }) + describe('Misc', () => { it('should handle already finished responses', async () => { const res = { diff --git a/yarn.lock b/yarn.lock index 3ad3f8a2..92b93fcd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -759,12 +759,6 @@ postcss-loader "^2.0.10" style-loader "^0.19.1" -"@zeit/source-map-support@0.6.2": - version "0.6.2" - resolved "https://registry.yarnpkg.com/@zeit/source-map-support/-/source-map-support-0.6.2.tgz#0efd478f24a606726948165e53a8efe89e24036f" - dependencies: - source-map "^0.6.0" - abab@^1.0.3: version "1.0.4" resolved "https://registry.yarnpkg.com/abab/-/abab-1.0.4.tgz#5faad9c2c07f60dd76770f71cf025b62a63cfd4e" @@ -6929,7 +6923,7 @@ source-map-url@^0.4.0: version "0.4.0" resolved "https://registry.yarnpkg.com/source-map-url/-/source-map-url-0.4.0.tgz#3e935d7ddd73631b97659956d55128e87b5084a3" -source-map@0.6.1, source-map@^0.6.0, source-map@^0.6.1, source-map@~0.6.1: +source-map@0.6.1, source-map@^0.6.1, source-map@~0.6.1: version "0.6.1" resolved "https://registry.yarnpkg.com/source-map/-/source-map-0.6.1.tgz#74722af32e9614e9c287a8d0bbde48b5e2f1a263"