From 30c85efca3dc182ca0ed55877f37acbc105d60df Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sun, 14 May 2017 15:21:42 +0100 Subject: [PATCH] Don't collapse unintentional top-level errors (#2145) * Don't collapse unintentional top-level errors * Linkify internal stack frames too --- .../src/components/code.js | 13 +- .../src/components/frame.js | 143 ++++++++++++++---- .../src/components/frames.js | 14 +- .../src/components/overlay.js | 2 +- .../src/utils/isInternalFile.js | 14 +- 5 files changed, 135 insertions(+), 51 deletions(-) diff --git a/packages/react-error-overlay/src/components/code.js b/packages/react-error-overlay/src/components/code.js index e27e72fd7b3..e7e86c8f3e0 100644 --- a/packages/react-error-overlay/src/components/code.js +++ b/packages/react-error-overlay/src/components/code.js @@ -20,8 +20,7 @@ function createCode( columnNum: number | null, contextSize: number, main: boolean, - clickToOpenFileName: ?string, - clickToOpenLineNumber: ?number + onSourceClick: ?Function ) { const sourceCode = []; let whiteSpace = Infinity; @@ -86,15 +85,11 @@ function createCode( applyStyles(pre, preStyle); pre.appendChild(code); - if (clickToOpenFileName) { + if (typeof onSourceClick === 'function') { + let handler = onSourceClick; pre.style.cursor = 'pointer'; pre.addEventListener('click', function() { - fetch( - '/__open-stack-frame-in-editor?fileName=' + - window.encodeURIComponent(clickToOpenFileName) + - '&lineNumber=' + - window.encodeURIComponent(clickToOpenLineNumber || 1) - ).then(() => {}, () => {}); + handler(); }); } diff --git a/packages/react-error-overlay/src/components/frame.js b/packages/react-error-overlay/src/components/frame.js index 5bcbe1fbe68..05a2e4913ca 100644 --- a/packages/react-error-overlay/src/components/frame.js +++ b/packages/react-error-overlay/src/components/frame.js @@ -79,7 +79,13 @@ function insertBeforeBundle( parent.insertBefore(div, first); } -function frameDiv(document: Document, functionName, url, internalUrl) { +function frameDiv( + document: Document, + functionName, + url, + internalUrl, + onSourceClick: ?Function +) { const frame = document.createElement('div'); const frameFunctionName = document.createElement('div'); @@ -112,9 +118,69 @@ function frameDiv(document: Document, functionName, url, internalUrl) { frameLink.appendChild(frameAnchor); frame.appendChild(frameLink); + if (typeof onSourceClick === 'function') { + let handler = onSourceClick; + frameAnchor.style.cursor = 'pointer'; + frameAnchor.addEventListener('click', function() { + handler(); + }); + } + return frame; } +function isBultinErrorName(errorName: ?string) { + switch (errorName) { + case 'EvalError': + case 'InternalError': + case 'RangeError': + case 'ReferenceError': + case 'SyntaxError': + case 'TypeError': + case 'URIError': + return true; + default: + return false; + } +} + +function getPrettyURL( + sourceFileName: ?string, + sourceLineNumber: ?number, + sourceColumnNumber: ?number, + fileName: ?string, + lineNumber: ?number, + columnNumber: ?number, + compiled: boolean +): string { + let prettyURL; + if (!compiled && sourceFileName && typeof sourceLineNumber === 'number') { + // Remove everything up to the first /src/ or /node_modules/ + const trimMatch = /^[/|\\].*?[/|\\]((src|node_modules)[/|\\].*)/.exec( + sourceFileName + ); + if (trimMatch && trimMatch[1]) { + prettyURL = trimMatch[1]; + } else { + prettyURL = sourceFileName; + } + prettyURL += ':' + sourceLineNumber; + // Note: we intentionally skip 0's because they're produced by cheap Webpack maps + if (sourceColumnNumber) { + prettyURL += ':' + sourceColumnNumber; + } + } else if (fileName && typeof lineNumber === 'number') { + prettyURL = fileName + ':' + lineNumber; + // Note: we intentionally skip 0's because they're produced by cheap Webpack maps + if (columnNumber) { + prettyURL += ':' + columnNumber; + } + } else { + prettyURL = 'unknown'; + } + return prettyURL; +} + function createFrame( document: Document, frameSetting: FrameSetting, @@ -124,7 +190,8 @@ function createFrame( omits: OmitsObject, omitBundle: number, parentContainer: HTMLDivElement, - lastElement: boolean + lastElement: boolean, + errorName: ?string ) { const { compiled } = frameSetting; let { functionName, _originalFileName: sourceFileName } = frame; @@ -149,35 +216,33 @@ function createFrame( functionName = '(anonymous function)'; } - 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]; - } + const prettyURL = getPrettyURL( + sourceFileName, + sourceLineNumber, + sourceColumnNumber, + fileName, + lineNumber, + columnNumber, + compiled + ); - url = sourceFileName + ':' + sourceLineNumber; - if (sourceColumnNumber) { - url += ':' + sourceColumnNumber; - } - } else if (fileName && lineNumber) { - url = fileName + ':' + lineNumber; - if (columnNumber) { - url += ':' + columnNumber; - } - } else { - url = 'unknown'; + let needsHidden = false; + const isInternalUrl = isInternalFile(sourceFileName, fileName); + const isThrownIntentionally = !isBultinErrorName(errorName); + const shouldCollapse = isInternalUrl && + (isThrownIntentionally || omits.hasReachedAppCode); + + if (!isInternalUrl) { + omits.hasReachedAppCode = true; } - let needsHidden = false; - const internalUrl = isInternalFile(url, sourceFileName); - if (internalUrl) { + if (shouldCollapse) { ++omits.value; needsHidden = true; } + let collapseElement = null; - if (!internalUrl || lastElement) { + if (!shouldCollapse || lastElement) { if (omits.value > 0) { const capV = omits.value; const omittedFrames = getGroupToggle(document, capV, omitBundle); @@ -190,7 +255,7 @@ function createFrame( omittedFrames ); }); - if (lastElement && internalUrl) { + if (lastElement && shouldCollapse) { collapseElement = omittedFrames; } else { parentContainer.appendChild(omittedFrames); @@ -200,14 +265,32 @@ function createFrame( omits.value = 0; } - const elem = frameDiv(document, functionName, url, internalUrl); + let onSourceClick = null; + if (sourceFileName) { + onSourceClick = () => { + fetch( + '/__open-stack-frame-in-editor?fileName=' + + window.encodeURIComponent(sourceFileName) + + '&lineNumber=' + + window.encodeURIComponent(sourceLineNumber || 1) + ).then(() => {}, () => {}); + }; + } + + const elem = frameDiv( + document, + functionName, + prettyURL, + shouldCollapse, + onSourceClick + ); if (needsHidden) { applyStyles(elem, hiddenStyle); elem.setAttribute('name', 'bundle-' + omitBundle); } let hasSource = false; - if (!internalUrl) { + if (!shouldCollapse) { if ( compiled && scriptLines && scriptLines.length !== 0 && lineNumber != null ) { @@ -219,8 +302,7 @@ function createFrame( columnNumber, contextSize, critical, - frame._originalFileName, - frame._originalLineNumber + onSourceClick ) ); hasSource = true; @@ -238,8 +320,7 @@ function createFrame( sourceColumnNumber, contextSize, critical, - frame._originalFileName, - frame._originalLineNumber + onSourceClick ) ); hasSource = true; diff --git a/packages/react-error-overlay/src/components/frames.js b/packages/react-error-overlay/src/components/frames.js index ba3b7effdf8..3d8d3156c24 100644 --- a/packages/react-error-overlay/src/components/frames.js +++ b/packages/react-error-overlay/src/components/frames.js @@ -5,7 +5,11 @@ import { traceStyle, toggleStyle } from '../styles'; import { enableTabClick } from '../utils/dom/enableTabClick'; import { createFrame } from './frame'; -type OmitsObject = { value: number, bundle: number }; +type OmitsObject = { + value: number, + bundle: number, + hasReachedAppCode: boolean, +}; type FrameSetting = { compiled: boolean }; export type { OmitsObject, FrameSetting }; @@ -68,7 +72,8 @@ function createFrames( document: Document, resolvedFrames: StackFrame[], frameSettings: FrameSetting[], - contextSize: number + contextSize: number, + errorName: ?string ) { if (resolvedFrames.length !== frameSettings.length) { throw new Error( @@ -80,7 +85,7 @@ function createFrames( let index = 0; let critical = true; - const omits: OmitsObject = { value: 0, bundle: 1 }; + const omits: OmitsObject = { value: 0, bundle: 1, hasReachedAppCode: false }; resolvedFrames.forEach(function(frame) { const lIndex = index++; const elem = createFrameWrapper( @@ -96,7 +101,8 @@ function createFrames( omits, omits.bundle, trace, - index === resolvedFrames.length + index === resolvedFrames.length, + errorName ), lIndex, frameSettings, diff --git a/packages/react-error-overlay/src/components/overlay.js b/packages/react-error-overlay/src/components/overlay.js index e812bb44154..8633524be13 100644 --- a/packages/react-error-overlay/src/components/overlay.js +++ b/packages/react-error-overlay/src/components/overlay.js @@ -73,7 +73,7 @@ function createOverlay( // Create trace container.appendChild( - createFrames(document, frames, frameSettings, contextSize) + createFrames(document, frames, frameSettings, contextSize, name) ); // Show message diff --git a/packages/react-error-overlay/src/utils/isInternalFile.js b/packages/react-error-overlay/src/utils/isInternalFile.js index f55123e258c..8678f13e4b4 100644 --- a/packages/react-error-overlay/src/utils/isInternalFile.js +++ b/packages/react-error-overlay/src/utils/isInternalFile.js @@ -1,10 +1,12 @@ /* @flow */ -function isInternalFile(url: string, sourceFileName: string | null | void) { - return url.indexOf('/~/') !== -1 || - url.indexOf('/node_modules/') !== -1 || - url.trim().indexOf(' ') !== -1 || - sourceFileName == null || - sourceFileName.length === 0; +function isInternalFile(sourceFileName: ?string, fileName: ?string) { + return sourceFileName == null || + sourceFileName === '' || + sourceFileName.indexOf('/~/') !== -1 || + sourceFileName.indexOf('/node_modules/') !== -1 || + sourceFileName.trim().indexOf(' ') !== -1 || + fileName == null || + fileName === ''; } export { isInternalFile };