Don't collapse unintentional top-level errors (facebook#2145)

* Don't collapse unintentional top-level errors * Linkify internal stack frames too
romaindso · Jul 10, 2017 · 30c85ef · 30c85ef
1 parent d53559a
commit 30c85ef
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 51 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 };