Skip to content

Commit 30c85ef

Browse files
gaearonromaindso
authored andcommitted
Don't collapse unintentional top-level errors (facebook#2145)
* Don't collapse unintentional top-level errors * Linkify internal stack frames too
1 parent d53559a commit 30c85ef

File tree

5 files changed

+135
-51
lines changed

5 files changed

+135
-51
lines changed

packages/react-error-overlay/src/components/code.js

+4-9
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ function createCode(
2020
columnNum: number | null,
2121
contextSize: number,
2222
main: boolean,
23-
clickToOpenFileName: ?string,
24-
clickToOpenLineNumber: ?number
23+
onSourceClick: ?Function
2524
) {
2625
const sourceCode = [];
2726
let whiteSpace = Infinity;
@@ -86,15 +85,11 @@ function createCode(
8685
applyStyles(pre, preStyle);
8786
pre.appendChild(code);
8887

89-
if (clickToOpenFileName) {
88+
if (typeof onSourceClick === 'function') {
89+
let handler = onSourceClick;
9090
pre.style.cursor = 'pointer';
9191
pre.addEventListener('click', function() {
92-
fetch(
93-
'/__open-stack-frame-in-editor?fileName=' +
94-
window.encodeURIComponent(clickToOpenFileName) +
95-
'&lineNumber=' +
96-
window.encodeURIComponent(clickToOpenLineNumber || 1)
97-
).then(() => {}, () => {});
92+
handler();
9893
});
9994
}
10095

packages/react-error-overlay/src/components/frame.js

+112-31
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,13 @@ function insertBeforeBundle(
7979
parent.insertBefore(div, first);
8080
}
8181

82-
function frameDiv(document: Document, functionName, url, internalUrl) {
82+
function frameDiv(
83+
document: Document,
84+
functionName,
85+
url,
86+
internalUrl,
87+
onSourceClick: ?Function
88+
) {
8389
const frame = document.createElement('div');
8490
const frameFunctionName = document.createElement('div');
8591

@@ -112,9 +118,69 @@ function frameDiv(document: Document, functionName, url, internalUrl) {
112118
frameLink.appendChild(frameAnchor);
113119
frame.appendChild(frameLink);
114120

121+
if (typeof onSourceClick === 'function') {
122+
let handler = onSourceClick;
123+
frameAnchor.style.cursor = 'pointer';
124+
frameAnchor.addEventListener('click', function() {
125+
handler();
126+
});
127+
}
128+
115129
return frame;
116130
}
117131

132+
function isBultinErrorName(errorName: ?string) {
133+
switch (errorName) {
134+
case 'EvalError':
135+
case 'InternalError':
136+
case 'RangeError':
137+
case 'ReferenceError':
138+
case 'SyntaxError':
139+
case 'TypeError':
140+
case 'URIError':
141+
return true;
142+
default:
143+
return false;
144+
}
145+
}
146+
147+
function getPrettyURL(
148+
sourceFileName: ?string,
149+
sourceLineNumber: ?number,
150+
sourceColumnNumber: ?number,
151+
fileName: ?string,
152+
lineNumber: ?number,
153+
columnNumber: ?number,
154+
compiled: boolean
155+
): string {
156+
let prettyURL;
157+
if (!compiled && sourceFileName && typeof sourceLineNumber === 'number') {
158+
// Remove everything up to the first /src/ or /node_modules/
159+
const trimMatch = /^[/|\\].*?[/|\\]((src|node_modules)[/|\\].*)/.exec(
160+
sourceFileName
161+
);
162+
if (trimMatch && trimMatch[1]) {
163+
prettyURL = trimMatch[1];
164+
} else {
165+
prettyURL = sourceFileName;
166+
}
167+
prettyURL += ':' + sourceLineNumber;
168+
// Note: we intentionally skip 0's because they're produced by cheap Webpack maps
169+
if (sourceColumnNumber) {
170+
prettyURL += ':' + sourceColumnNumber;
171+
}
172+
} else if (fileName && typeof lineNumber === 'number') {
173+
prettyURL = fileName + ':' + lineNumber;
174+
// Note: we intentionally skip 0's because they're produced by cheap Webpack maps
175+
if (columnNumber) {
176+
prettyURL += ':' + columnNumber;
177+
}
178+
} else {
179+
prettyURL = 'unknown';
180+
}
181+
return prettyURL;
182+
}
183+
118184
function createFrame(
119185
document: Document,
120186
frameSetting: FrameSetting,
@@ -124,7 +190,8 @@ function createFrame(
124190
omits: OmitsObject,
125191
omitBundle: number,
126192
parentContainer: HTMLDivElement,
127-
lastElement: boolean
193+
lastElement: boolean,
194+
errorName: ?string
128195
) {
129196
const { compiled } = frameSetting;
130197
let { functionName, _originalFileName: sourceFileName } = frame;
@@ -149,35 +216,33 @@ function createFrame(
149216
functionName = '(anonymous function)';
150217
}
151218

152-
let url;
153-
if (!compiled && sourceFileName && sourceLineNumber) {
154-
// Remove everything up to the first /src/
155-
const trimMatch = /^[/|\\].*?[/|\\](src[/|\\].*)/.exec(sourceFileName);
156-
if (trimMatch && trimMatch[1]) {
157-
sourceFileName = trimMatch[1];
158-
}
219+
const prettyURL = getPrettyURL(
220+
sourceFileName,
221+
sourceLineNumber,
222+
sourceColumnNumber,
223+
fileName,
224+
lineNumber,
225+
columnNumber,
226+
compiled
227+
);
159228

160-
url = sourceFileName + ':' + sourceLineNumber;
161-
if (sourceColumnNumber) {
162-
url += ':' + sourceColumnNumber;
163-
}
164-
} else if (fileName && lineNumber) {
165-
url = fileName + ':' + lineNumber;
166-
if (columnNumber) {
167-
url += ':' + columnNumber;
168-
}
169-
} else {
170-
url = 'unknown';
229+
let needsHidden = false;
230+
const isInternalUrl = isInternalFile(sourceFileName, fileName);
231+
const isThrownIntentionally = !isBultinErrorName(errorName);
232+
const shouldCollapse = isInternalUrl &&
233+
(isThrownIntentionally || omits.hasReachedAppCode);
234+
235+
if (!isInternalUrl) {
236+
omits.hasReachedAppCode = true;
171237
}
172238

173-
let needsHidden = false;
174-
const internalUrl = isInternalFile(url, sourceFileName);
175-
if (internalUrl) {
239+
if (shouldCollapse) {
176240
++omits.value;
177241
needsHidden = true;
178242
}
243+
179244
let collapseElement = null;
180-
if (!internalUrl || lastElement) {
245+
if (!shouldCollapse || lastElement) {
181246
if (omits.value > 0) {
182247
const capV = omits.value;
183248
const omittedFrames = getGroupToggle(document, capV, omitBundle);
@@ -190,7 +255,7 @@ function createFrame(
190255
omittedFrames
191256
);
192257
});
193-
if (lastElement && internalUrl) {
258+
if (lastElement && shouldCollapse) {
194259
collapseElement = omittedFrames;
195260
} else {
196261
parentContainer.appendChild(omittedFrames);
@@ -200,14 +265,32 @@ function createFrame(
200265
omits.value = 0;
201266
}
202267

203-
const elem = frameDiv(document, functionName, url, internalUrl);
268+
let onSourceClick = null;
269+
if (sourceFileName) {
270+
onSourceClick = () => {
271+
fetch(
272+
'/__open-stack-frame-in-editor?fileName=' +
273+
window.encodeURIComponent(sourceFileName) +
274+
'&lineNumber=' +
275+
window.encodeURIComponent(sourceLineNumber || 1)
276+
).then(() => {}, () => {});
277+
};
278+
}
279+
280+
const elem = frameDiv(
281+
document,
282+
functionName,
283+
prettyURL,
284+
shouldCollapse,
285+
onSourceClick
286+
);
204287
if (needsHidden) {
205288
applyStyles(elem, hiddenStyle);
206289
elem.setAttribute('name', 'bundle-' + omitBundle);
207290
}
208291

209292
let hasSource = false;
210-
if (!internalUrl) {
293+
if (!shouldCollapse) {
211294
if (
212295
compiled && scriptLines && scriptLines.length !== 0 && lineNumber != null
213296
) {
@@ -219,8 +302,7 @@ function createFrame(
219302
columnNumber,
220303
contextSize,
221304
critical,
222-
frame._originalFileName,
223-
frame._originalLineNumber
305+
onSourceClick
224306
)
225307
);
226308
hasSource = true;
@@ -238,8 +320,7 @@ function createFrame(
238320
sourceColumnNumber,
239321
contextSize,
240322
critical,
241-
frame._originalFileName,
242-
frame._originalLineNumber
323+
onSourceClick
243324
)
244325
);
245326
hasSource = true;

packages/react-error-overlay/src/components/frames.js

+10-4
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import { traceStyle, toggleStyle } from '../styles';
55
import { enableTabClick } from '../utils/dom/enableTabClick';
66
import { createFrame } from './frame';
77

8-
type OmitsObject = { value: number, bundle: number };
8+
type OmitsObject = {
9+
value: number,
10+
bundle: number,
11+
hasReachedAppCode: boolean,
12+
};
913
type FrameSetting = { compiled: boolean };
1014
export type { OmitsObject, FrameSetting };
1115

@@ -68,7 +72,8 @@ function createFrames(
6872
document: Document,
6973
resolvedFrames: StackFrame[],
7074
frameSettings: FrameSetting[],
71-
contextSize: number
75+
contextSize: number,
76+
errorName: ?string
7277
) {
7378
if (resolvedFrames.length !== frameSettings.length) {
7479
throw new Error(
@@ -80,7 +85,7 @@ function createFrames(
8085

8186
let index = 0;
8287
let critical = true;
83-
const omits: OmitsObject = { value: 0, bundle: 1 };
88+
const omits: OmitsObject = { value: 0, bundle: 1, hasReachedAppCode: false };
8489
resolvedFrames.forEach(function(frame) {
8590
const lIndex = index++;
8691
const elem = createFrameWrapper(
@@ -96,7 +101,8 @@ function createFrames(
96101
omits,
97102
omits.bundle,
98103
trace,
99-
index === resolvedFrames.length
104+
index === resolvedFrames.length,
105+
errorName
100106
),
101107
lIndex,
102108
frameSettings,

packages/react-error-overlay/src/components/overlay.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ function createOverlay(
7373

7474
// Create trace
7575
container.appendChild(
76-
createFrames(document, frames, frameSettings, contextSize)
76+
createFrames(document, frames, frameSettings, contextSize, name)
7777
);
7878

7979
// Show message
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
/* @flow */
2-
function isInternalFile(url: string, sourceFileName: string | null | void) {
3-
return url.indexOf('/~/') !== -1 ||
4-
url.indexOf('/node_modules/') !== -1 ||
5-
url.trim().indexOf(' ') !== -1 ||
6-
sourceFileName == null ||
7-
sourceFileName.length === 0;
2+
function isInternalFile(sourceFileName: ?string, fileName: ?string) {
3+
return sourceFileName == null ||
4+
sourceFileName === '' ||
5+
sourceFileName.indexOf('/~/') !== -1 ||
6+
sourceFileName.indexOf('/node_modules/') !== -1 ||
7+
sourceFileName.trim().indexOf(' ') !== -1 ||
8+
fileName == null ||
9+
fileName === '';
810
}
911

1012
export { isInternalFile };

0 commit comments

Comments
 (0)