Skip to content

Commit

Permalink
ExceptionsManager: Minor Code Cleanup
Browse files Browse the repository at this point in the history
Summary:
Cleans up `reactConsoleErrorHandler` in `ExceptionsManager` using modern language features, and fixes a minor edge case with how warning-like errors are handled.

Changelog:
[General][Fixed] - Avoid downgrading `console.error` when passed warning-like objects.

Reviewed By: GijsWeterings

Differential Revision: D28418488

fbshipit-source-id: 394e8608c2c81c794c9a0fc155142dcfcfe1c661
  • Loading branch information
yungsters authored and facebook-github-bot committed May 19, 2021
1 parent eeb8e58 commit 0dba0af
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 17 deletions.
36 changes: 19 additions & 17 deletions Libraries/Core/ExceptionsManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,9 @@ function handleException(e: mixed, isFatal: boolean) {
}
}

function reactConsoleErrorHandler() {
function reactConsoleErrorHandler(...args) {
// bubble up to any original handlers
console._errorOriginal.apply(console, arguments);
console._errorOriginal(...args);
if (!console.reportErrorsAsExceptions) {
return;
}
Expand Down Expand Up @@ -213,31 +213,33 @@ function reactConsoleErrorHandler() {
return;
}

if (arguments[0] && arguments[0].stack) {
let error;

const firstArg = args[0];
if (firstArg?.stack) {
// reportException will console.error this with high enough fidelity.
reportException(
arguments[0],
/* isFatal */ false,
/*reportToConsole*/ false,
);
error = firstArg;
} else {
const stringifySafe = require('../Utilities/stringifySafe').default;
const str = Array.prototype.map
.call(arguments, value =>
typeof value === 'string' ? value : stringifySafe(value),
)
.join(' ');

if (str.slice(0, 9) === 'Warning: ') {
if (typeof firstArg === 'string' && firstArg.startsWith('Warning: ')) {
// React warnings use console.error so that a stack trace is shown, but
// we don't (currently) want these to show a redbox
// (Note: Logic duplicated in polyfills/console.js.)
return;
}
const error: ExtendedError = new SyntheticError(str);
const message = args
.map(arg => (typeof arg === 'string' ? arg : stringifySafe(arg)))
.join(' ');

error = new SyntheticError(message);
error.name = 'console.error';
reportException(error, /* isFatal */ false, /*reportToConsole*/ false);
}

reportException(
error,
false, // isFatal
false, // reportToConsole
);
}

/**
Expand Down
12 changes: 12 additions & 0 deletions Libraries/Core/__tests__/ExceptionsManager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,18 @@ describe('ExceptionsManager', () => {
expect(mockError.mock.calls[0]).toEqual(args);
});

test('logging a warning-looking object', () => {
// Forces `strignifySafe` to invoke `toString()`.
const object = {toString: () => 'Warning: Some error may have happened'};
object.cycle = object;

const args = [object];

console.error(...args);

expect(nativeReportException).toHaveBeenCalled();
});

test('reportErrorsAsExceptions = false', () => {
console.reportErrorsAsExceptions = false;
const message = 'Some error happened';
Expand Down

0 comments on commit 0dba0af

Please sign in to comment.