Skip to content

Commit

Permalink
Remove framesToPop support from ExceptionsManager and parseErrorStack
Browse files Browse the repository at this point in the history
Summary:
Removes support for the non-standard `framesToPop` error property from React Native. Redboxes will now ignore this field. The way to skip uninformative frames in stack traces going forward is to use Metro's `customizeFrame` config option, for which the React Native CLI ships useful defaults (see: react-native-community/cli#596, react-native-community/cli#780)

Changelog: [General] [Removed] - Remove support for framesToPop from ExceptionsManager

Reviewed By: rickhanlonii

Differential Revision: D17877444

fbshipit-source-id: 04aa332c45ad35a99ae20e05fb87b34c91a557ab
  • Loading branch information
motiz88 authored and facebook-github-bot committed Oct 22, 2019
1 parent def3ebe commit 8bc02fd
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 23 deletions.
6 changes: 2 additions & 4 deletions Libraries/Core/Devtools/__tests__/parseErrorStack-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,15 @@ describe('parseErrorStack', function() {
expect(firstFrame.file).toMatch(/parseErrorStack-test\.js$/);
});

it('supports framesToPop', function() {
it('does not support framesToPop', function() {
function getWrappedError() {
const error = getFakeError();
error.framesToPop = 1;
return error;
}

// Make sure framesToPop == 1 causes it to ignore getFakeError
// stack frame
const stack = parseErrorStack(getWrappedError());
expect(stack[0].methodName).toEqual('getWrappedError');
expect(stack[0].methodName).toEqual('getFakeError');
});

it('ignores bad inputs', function() {
Expand Down
5 changes: 0 additions & 5 deletions Libraries/Core/Devtools/parseErrorStack.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import type {StackFrame} from '../NativeExceptionsManager';

export type ExtendedError = Error & {
framesToPop?: number,
jsEngine?: string,
preventSymbolication?: boolean,
componentStack?: string,
Expand All @@ -29,10 +28,6 @@ function parseErrorStack(e: ExtendedError): Array<StackFrame> {
? e.stack
: stacktraceParser.parse(e.stack);

let framesToPop = typeof e.framesToPop === 'number' ? e.framesToPop : 0;
while (framesToPop--) {
stack.shift();
}
return stack;
}

Expand Down
2 changes: 0 additions & 2 deletions Libraries/Core/ExceptionsManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ function reportException(e: ExtendedError, isFatal: boolean) {
extraData: {
jsEngine: e.jsEngine,
rawStack: e.stack,
framesPopped: e.framesToPop,
},
});

Expand Down Expand Up @@ -169,7 +168,6 @@ function reactConsoleErrorHandler() {
}
const error: ExtendedError = new SyntheticError(str);
error.name = 'console.error';
error.framesToPop = (error.framesToPop || 0) + 1;
reportException(error, /* isFatal */ false);
}
}
Expand Down
29 changes: 17 additions & 12 deletions Libraries/Core/__tests__/ExceptionsManager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'use strict';

const fs = require('fs');
const path = require('path');

const capturedErrorDefaults = {
componentName: 'A',
Expand Down Expand Up @@ -87,7 +88,7 @@ describe('ExceptionsManager', () => {
expect(console.error).toBeCalledWith(formattedMessage);
});

test('pops frames off the stack with framesToPop', () => {
test('does not pop frames off the stack with framesToPop', () => {
function createError() {
const error = new Error('Some error happened');
error.framesToPop = 1;
Expand All @@ -103,7 +104,7 @@ describe('ExceptionsManager', () => {
expect(nativeReportException.mock.calls.length).toBe(1);
const exceptionData = nativeReportException.mock.calls[0][0];
expect(getLineFromFrame(exceptionData.stack[0])).toBe(
'const error = createError();',
"const error = new Error('Some error happened');",
);
});

Expand Down Expand Up @@ -305,9 +306,9 @@ describe('ExceptionsManager', () => {
);
expect(exceptionData.originalMessage).toBe('"Some error happened"');
expect(exceptionData.name).toBe('console.error');
expect(getLineFromFrame(exceptionData.stack[0])).toBe(
'console.error(message);',
);
expect(
getLineFromFrame(getFirstFrameInThisFile(exceptionData.stack)),
).toBe('console.error(message);');
expect(exceptionData.isFatal).toBe(false);
expect(mockError.mock.calls[0]).toEqual([message]);
});
Expand All @@ -326,9 +327,9 @@ describe('ExceptionsManager', () => {
'42, true, ["symbol" failed to stringify], {"y":null}',
);
expect(exceptionData.name).toBe('console.error');
expect(getLineFromFrame(exceptionData.stack[0])).toBe(
'console.error(...args);',
);
expect(
getLineFromFrame(getFirstFrameInThisFile(exceptionData.stack)),
).toBe('console.error(...args);');
expect(exceptionData.isFatal).toBe(false);

expect(mockError).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -367,7 +368,7 @@ describe('ExceptionsManager', () => {
expect(mockError.mock.calls[0]).toEqual([message]);
});

test('pops frames off the stack with framesToPop', () => {
test('does not pop frames off the stack with framesToPop', () => {
function createError() {
const error = new Error('Some error happened');
error.framesToPop = 1;
Expand All @@ -380,7 +381,7 @@ describe('ExceptionsManager', () => {
expect(nativeReportException.mock.calls.length).toBe(1);
const exceptionData = nativeReportException.mock.calls[0][0];
expect(getLineFromFrame(exceptionData.stack[0])).toBe(
'const error = createError();',
"const error = new Error('Some error happened');",
);
});
});
Expand Down Expand Up @@ -441,7 +442,7 @@ describe('ExceptionsManager', () => {
expect(console.error.mock.calls[0]).toEqual([message]);
});

test('pops frames off the stack with framesToPop', () => {
test('does not pop frames off the stack with framesToPop', () => {
function createError() {
const error = new Error('Some error happened');
error.framesToPop = 1;
Expand All @@ -454,7 +455,7 @@ describe('ExceptionsManager', () => {
expect(nativeReportException.mock.calls.length).toBe(1);
const exceptionData = nativeReportException.mock.calls[0][0];
expect(getLineFromFrame(exceptionData.stack[0])).toBe(
'const error = createError();',
"const error = new Error('Some error happened');",
);
});
});
Expand Down Expand Up @@ -550,6 +551,10 @@ function getLineFromFrame({lineNumber /* 1-based */, file}) {
return (lines[lineNumber - 1] || '').trim();
}

function getFirstFrameInThisFile(stack) {
return stack.find(({file}) => file.endsWith(path.basename(module.filename)));
}

// Works around a parseErrorStack bug involving `new X` stack frames.
function cleanFileName(file) {
return file.replace(/^.+? \((?=\/)/, '');
Expand Down

0 comments on commit 8bc02fd

Please sign in to comment.