Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve unmapper file heuristic, add limited warning support, and ignore internal errors #2128

Merged
merged 23 commits into from
May 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions packages/react-error-overlay/src/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down
14 changes: 10 additions & 4 deletions packages/react-error-overlay/src/components/overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { SwitchCallback } from './additional';

function createOverlay(
document: Document,
name: string,
name: ?string,
message: string,
frames: StackFrame[],
contextSize: number,
Expand Down Expand Up @@ -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));
Expand Down
41 changes: 37 additions & 4 deletions packages/react-error-overlay/src/effects/proxyConsole.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,48 @@
/* @flow */
type ConsoleProxyCallback = (message: string) => void;

type ReactFrame = {
fileName: string | null,
lineNumber: number | null,
functionName: string | null,
};
const reactFrameStack: Array<ReactFrame[]> = [];

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 };
26 changes: 25 additions & 1 deletion packages/react-error-overlay/src/overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -205,13 +214,28 @@ 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() {
unregisterStackTraceLimit();
unregisterShortcuts(window);
unregisterPromise(window);
unregisterError(window);
unregisterReactStack();
}

export { inject, uninject };
9 changes: 8 additions & 1 deletion packages/react-error-overlay/src/utils/errorRegister.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function consume(
error: Error,
unhandledRejection: boolean = false,
contextSize: number = 3
): Promise<ErrorRecordReference> {
): Promise<ErrorRecordReference | null> {
const parsedFrames = parse(error);
let enhancedFramesPromise;
if (error.__unmap_source) {
Expand All @@ -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 ||
Expand Down
47 changes: 28 additions & 19 deletions packages/react-error-overlay/src/utils/unmapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's better thanks!

return count;
}

/**
* Turns a set of mapped <code>StackFrame</code>s back into their generated code position and enhances them with code.
* @param {string} fileUri The URI of the <code>bundle.js</code> file.
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand Down
29 changes: 29 additions & 0 deletions packages/react-error-overlay/src/utils/warnings.js
Original file line number Diff line number Diff line change
@@ -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 };
2 changes: 1 addition & 1 deletion packages/react-scripts/config/webpack.config.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down