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 16 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
7 changes: 5 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,10 @@ 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
2 changes: 1 addition & 1 deletion packages/react-error-overlay/src/components/overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function createOverlay(
applyStyles(header, headerStyle);

// Make message prettier
let finalMessage = message.match(/^\w*:/) ? name + ': ' + message : message;
let finalMessage = message.match(/^\w*:/) ? message : name + ': ' + message;
finalMessage = finalMessage
// TODO: maybe remove this prefix from fbjs?
// It's just scaring people
Expand Down
33 changes: 30 additions & 3 deletions packages/react-error-overlay/src/effects/proxyConsole.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,42 @@
/* @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
) => 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(' ');
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not quite what the browser does though. It also supports interpolations. For example console.warn('hello %s', 'friend')

Copy link
Contributor

Choose a reason for hiding this comment

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

For now I'll just use the first argument.

callback(message);
callback(message, ReactFrameStack[ReactFrameStack.length - 1]);
return orig.apply(this, arguments);
};
};

export { permanentRegister };
export { permanentRegister, registerReactStack, unregisterReactStack };
25 changes: 25 additions & 0 deletions 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 @@ -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,29 @@ function inject() {
registerPromise(window, error => crash(error, true));
registerShortcuts(window, shortcutHandler);
registerStackTraceLimit();

registerReactStack();
permanentRegisterConsole('error', (warning, stack) => {
const data = massageWarning(warning, stack);
if (data == null) return;
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
39 changes: 39 additions & 0 deletions packages/react-error-overlay/src/utils/warnings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// @flow
import type { ReactFrame } from '../effects/proxyConsole';

// This is a list of information to remove from React warnings, it's not particularly useful to show.
const removals = [/Check your code at (.*?:.*)[.]/];

function massage(
warning: string,
frames: ReactFrame[] | void
): { message: string, stack: string } | null {
if (!frames) {
return null;
}

let message = warning;
const nIndex = message.indexOf('\n');
Copy link
Contributor

Choose a reason for hiding this comment

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

This will crash if warning is not a string. Technically user can call warning with any types—we need to be more careful.

if (nIndex !== -1) message = message.substring(0, nIndex);

for (const trim of removals) {
message = message.replace(trim, '');
}

let stack = '';
for (let index = 0; index < frames.length; ++index) {
const { fileName, lineNumber } = frames[index];
if (fileName == null || lineNumber == null) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Code style: I'd like to reformat to always use if with braces. It's hard to track early exits.

let { functionName } = frames[index];
if (functionName == null && index === 0 && index + 1 < frames.length) {
functionName = frames[index + 1].functionName;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is hacky, I should fix this on my side instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, you shouldn't need this anymore. I updated facebook/react#9679.

if (functionName !== null) functionName = `(${functionName})`;
}
functionName = functionName || '(unknown 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