From 96206482184e22904344aadc8f8919a3b83796e6 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Sat, 13 May 2017 09:39:32 -0400 Subject: [PATCH] Improve unmapper file heuristic, add limited warning support, and ignore internal errors (#2128) * Browser sort is not stable * Fix ordering of final message * Register the warning capture * Display only createElement warnings * Use different method name * Fix regression * Ignore errors with only node_module files * Ignore null files, too * Revise count * Revise warning * Update overlay.js * Add support for https://github.com/facebook/react/pull/9679 * Use absolute paths * Trim path if it's absolute * Make sure it's an absolute path * Oops * Tweak for new behavior * Make it safer * More resilient warnings * Prettier output * Fix flow --- .../src/components/frame.js | 9 +++- .../src/components/overlay.js | 14 ++++-- .../src/effects/proxyConsole.js | 41 ++++++++++++++-- packages/react-error-overlay/src/overlay.js | 26 +++++++++- .../src/utils/errorRegister.js | 9 +++- .../react-error-overlay/src/utils/unmapper.js | 47 +++++++++++-------- .../react-error-overlay/src/utils/warnings.js | 29 ++++++++++++ .../config/webpack.config.dev.js | 2 +- 8 files changed, 145 insertions(+), 32 deletions(-) create mode 100644 packages/react-error-overlay/src/utils/warnings.js diff --git a/packages/react-error-overlay/src/components/frame.js b/packages/react-error-overlay/src/components/frame.js index 1b6a1d5d..db9812cc 100644 --- a/packages/react-error-overlay/src/components/frame.js +++ b/packages/react-error-overlay/src/components/frame.js @@ -127,13 +127,12 @@ function createFrame( lastElement: boolean ) { const { compiled } = frameSetting; - let { functionName } = frame; + let { functionName, _originalFileName: sourceFileName } = frame; const { fileName, lineNumber, columnNumber, _scriptCode: scriptLines, - _originalFileName: sourceFileName, _originalLineNumber: sourceLineNumber, _originalColumnNumber: sourceColumnNumber, _originalScriptCode: sourceLines, @@ -149,6 +148,12 @@ function createFrame( let url; if (!compiled && sourceFileName && sourceLineNumber) { + // Remove everything up to the first /src/ + const trimMatch = /^[/|\\].*?[/|\\](src[/|\\].*)/.exec(sourceFileName); + if (trimMatch && trimMatch[1]) { + sourceFileName = trimMatch[1]; + } + url = sourceFileName + ':' + sourceLineNumber; if (sourceColumnNumber) { url += ':' + sourceColumnNumber; diff --git a/packages/react-error-overlay/src/components/overlay.js b/packages/react-error-overlay/src/components/overlay.js index 3acd5b34..e812bb44 100644 --- a/packages/react-error-overlay/src/components/overlay.js +++ b/packages/react-error-overlay/src/components/overlay.js @@ -12,7 +12,7 @@ import type { SwitchCallback } from './additional'; function createOverlay( document: Document, - name: string, + name: ?string, message: string, frames: StackFrame[], contextSize: number, @@ -52,14 +52,20 @@ function createOverlay( applyStyles(header, headerStyle); // Make message prettier - let finalMessage = message.match(/^\w*:/) ? message : name + ': ' + message; + let finalMessage = message.match(/^\w*:/) || !name + ? message + : name + ': ' + message; + finalMessage = finalMessage // TODO: maybe remove this prefix from fbjs? // It's just scaring people - .replace('Invariant Violation: ', '') + .replace(/^Invariant Violation:\s*/, '') + // This is not helpful either: + .replace(/^Warning:\s*/, '') // Break the actionable part to the next line. // AFAIK React 16+ should already do this. - .replace(' Check the render method', '\n\nCheck the render method'); + .replace(' Check the render method', '\n\nCheck the render method') + .replace(' Check your code at', '\n\nCheck your code at'); // Put it in the DOM header.appendChild(document.createTextNode(finalMessage)); diff --git a/packages/react-error-overlay/src/effects/proxyConsole.js b/packages/react-error-overlay/src/effects/proxyConsole.js index a0a771f0..062055e0 100644 --- a/packages/react-error-overlay/src/effects/proxyConsole.js +++ b/packages/react-error-overlay/src/effects/proxyConsole.js @@ -1,15 +1,48 @@ /* @flow */ -type ConsoleProxyCallback = (message: string) => void; + +type ReactFrame = { + fileName: string | null, + lineNumber: number | null, + functionName: string | null, +}; +const reactFrameStack: Array = []; + +export type { ReactFrame }; + +const registerReactStack = () => { + // $FlowFixMe + console.stack = frames => reactFrameStack.push(frames); + // $FlowFixMe + console.stackEnd = frames => reactFrameStack.pop(); +}; + +const unregisterReactStack = () => { + // $FlowFixMe + console.stack = undefined; + // $FlowFixMe + console.stackEnd = undefined; +}; + +type ConsoleProxyCallback = (message: string, frames: ReactFrame[]) => void; const permanentRegister = function proxyConsole( type: string, callback: ConsoleProxyCallback ) { const orig = console[type]; console[type] = function __stack_frame_overlay_proxy_console__() { - const message = [].slice.call(arguments).join(' '); - callback(message); + try { + const message = arguments[0]; + if (typeof message === 'string' && reactFrameStack.length > 0) { + callback(message, reactFrameStack[reactFrameStack.length - 1]); + } + } catch (err) { + // Warnings must never crash. Rethrow with a clean stack. + setTimeout(function() { + throw err; + }); + } return orig.apply(this, arguments); }; }; -export { permanentRegister }; +export { permanentRegister, registerReactStack, unregisterReactStack }; diff --git a/packages/react-error-overlay/src/overlay.js b/packages/react-error-overlay/src/overlay.js index 63c91940..fe29a6c7 100644 --- a/packages/react-error-overlay/src/overlay.js +++ b/packages/react-error-overlay/src/overlay.js @@ -19,6 +19,12 @@ import { register as registerStackTraceLimit, unregister as unregisterStackTraceLimit, } from './effects/stackTraceLimit'; +import { + permanentRegister as permanentRegisterConsole, + registerReactStack, + unregisterReactStack, +} from './effects/proxyConsole'; +import { massage as massageWarning } from './utils/warnings'; import { consume as consumeError, @@ -66,7 +72,7 @@ const css = [ '}', ].join('\n'); -function render(name: string, message: string, resolvedFrames: StackFrame[]) { +function render(name: ?string, message: string, resolvedFrames: StackFrame[]) { disposeCurrentView(); const iframe = window.document.createElement('iframe'); @@ -156,6 +162,9 @@ function crash(error: Error, unhandledRejection = false) { } consumeError(error, unhandledRejection, CONTEXT_SIZE) .then(ref => { + if (ref == null) { + return; + } errorReferences.push(ref); if (iframeReference !== null && additionalReference !== null) { updateAdditional( @@ -205,6 +214,20 @@ function inject() { registerPromise(window, error => crash(error, true)); registerShortcuts(window, shortcutHandler); registerStackTraceLimit(); + + registerReactStack(); + permanentRegisterConsole('error', (warning, stack) => { + const data = massageWarning(warning, stack); + crash( + // $FlowFixMe + { + message: data.message, + stack: data.stack, + __unmap_source: '/static/js/bundle.js', + }, + false + ); + }); } function uninject() { @@ -212,6 +235,7 @@ function uninject() { unregisterShortcuts(window); unregisterPromise(window); unregisterError(window); + unregisterReactStack(); } export { inject, uninject }; diff --git a/packages/react-error-overlay/src/utils/errorRegister.js b/packages/react-error-overlay/src/utils/errorRegister.js index f14ff9d9..3fc1ab6b 100644 --- a/packages/react-error-overlay/src/utils/errorRegister.js +++ b/packages/react-error-overlay/src/utils/errorRegister.js @@ -19,7 +19,7 @@ function consume( error: Error, unhandledRejection: boolean = false, contextSize: number = 3 -): Promise { +): Promise { const parsedFrames = parse(error); let enhancedFramesPromise; if (error.__unmap_source) { @@ -33,6 +33,13 @@ function consume( enhancedFramesPromise = map(parsedFrames, contextSize); } return enhancedFramesPromise.then(enhancedFrames => { + if ( + enhancedFrames + .map(f => f._originalFileName) + .filter(f => f != null && f.indexOf('node_modules') === -1).length === 0 + ) { + return null; + } enhancedFrames = enhancedFrames.filter( ({ functionName }) => functionName == null || diff --git a/packages/react-error-overlay/src/utils/unmapper.js b/packages/react-error-overlay/src/utils/unmapper.js index 415d18dc..bf6e5917 100644 --- a/packages/react-error-overlay/src/utils/unmapper.js +++ b/packages/react-error-overlay/src/utils/unmapper.js @@ -4,6 +4,19 @@ import { getSourceMap } from './getSourceMap'; import { getLinesAround } from './getLinesAround'; import path from 'path'; +function count(search: string, string: string): number { + // Count starts at -1 becuse a do-while loop always runs at least once + let count = -1, index = -1; + do { + // First call or the while case evaluated true, meaning we have to make + // count 0 or we found a character + ++count; + // Find the index of our search string, starting after the previous index + index = string.indexOf(search, index + 1); + } while (index !== -1); + return count; +} + /** * Turns a set of mapped StackFrames back into their generated code position and enhances them with code. * @param {string} fileUri The URI of the bundle.js file. @@ -39,28 +52,23 @@ async function unmap( return frame; } const fN: string = fileName; - const splitCache1: any = {}, splitCache2: any = {}, splitCache3: any = {}; const source = map .getSources() .map(s => s.replace(/[\\]+/g, '/')) - .filter(s => { - s = path.normalize(s); - return s.indexOf(fN) === s.length - fN.length; - }) - .sort((a, b) => { - let a2 = splitCache1[a] || (splitCache1[a] = a.split(path.sep)), - b2 = splitCache1[b] || (splitCache1[b] = b.split(path.sep)); - return Math.sign(a2.length - b2.length); - }) - .sort((a, b) => { - let a2 = splitCache2[a] || (splitCache2[a] = a.split('node_modules')), - b2 = splitCache2[b] || (splitCache2[b] = b.split('node_modules')); - return Math.sign(a2.length - b2.length); + .filter(p => { + p = path.normalize(p); + const i = p.lastIndexOf(fN); + return i !== -1 && i === p.length - fN.length; }) + .map(p => ({ + token: p, + seps: count(path.sep, path.normalize(p)), + penalties: count('node_modules', p) + count('~', p), + })) .sort((a, b) => { - let a2 = splitCache3[a] || (splitCache3[a] = a.split('~')), - b2 = splitCache3[b] || (splitCache3[b] = b.split('~')); - return Math.sign(a2.length - b2.length); + const s = Math.sign(a.seps - b.seps); + if (s !== 0) return s; + return Math.sign(a.penalties - b.penalties); }); if (source.length < 1 || lineNumber == null) { return new StackFrame( @@ -76,13 +84,14 @@ async function unmap( null ); } + const sourceT = source[0].token; const { line, column } = map.getGeneratedPosition( - source[0], + sourceT, lineNumber, // $FlowFixMe columnNumber ); - const originalSource = map.getSource(source[0]); + const originalSource = map.getSource(sourceT); return new StackFrame( functionName, fileUri, diff --git a/packages/react-error-overlay/src/utils/warnings.js b/packages/react-error-overlay/src/utils/warnings.js new file mode 100644 index 00000000..b2fc34bb --- /dev/null +++ b/packages/react-error-overlay/src/utils/warnings.js @@ -0,0 +1,29 @@ +// @flow +import type { ReactFrame } from '../effects/proxyConsole'; + +function stripInlineStacktrace(message: string): string { + return message.split('\n').filter(line => !line.match(/^\s*in/)).join('\n'); // " in Foo" +} + +function massage( + warning: string, + frames: ReactFrame[] +): { message: string, stack: string } { + let message = stripInlineStacktrace(warning); + + // Reassemble the stack with full filenames provided by React + let stack = ''; + for (let index = 0; index < frames.length; ++index) { + const { fileName, lineNumber } = frames[index]; + if (fileName == null || lineNumber == null) { + continue; + } + let { functionName } = frames[index]; + functionName = functionName || '(anonymous function)'; + stack += `in ${functionName} (at ${fileName}:${lineNumber})\n`; + } + + return { message, stack }; +} + +export { massage }; diff --git a/packages/react-scripts/config/webpack.config.dev.js b/packages/react-scripts/config/webpack.config.dev.js index e7ab84a2..3d18991c 100644 --- a/packages/react-scripts/config/webpack.config.dev.js +++ b/packages/react-scripts/config/webpack.config.dev.js @@ -77,7 +77,7 @@ module.exports = { publicPath: publicPath, // Point sourcemap entries to original disk location devtoolModuleFilenameTemplate: info => - path.relative(paths.appSrc, info.absoluteResourcePath), + path.resolve(info.absoluteResourcePath), }, resolve: { // This allows you to set a fallback for where Webpack should look for modules.