console.error gets replaced

[Morriz]

In your code the following line is causing havoc:

console.error = (message, error) => global.ErrorUtils.reportError(error); // sending console.error so that it can be caught

This seems very bad. I can't console.error any more, because your (message, error) signature is incompatible, which swallows all output I normally expect console.error(error) to produce.

[ufolux]

same problem

[a7ul]

Could you please raise a PR for this and we can take a look?

[Morriz]

Your code is just 33 lines, but I just don't understand why you do this:

console.error = (message, error) => global.ErrorUtils.reportError(error); // sending console.error so that it can be caught

And without understanding why, I can't make a PR

[Morriz]

I refer to that specific line in this report

[a7ul]

alright makes sense: How about we do it this way @Morriz and @ufolux ?

const consoleError = console.error;
console.error = (...args) => {
  const error = args[1];
  global.ErrorUtils.reportError(error);
  consoleError(...args);
}

[Morriz]

Already better, but I would say all of args needs to be reported, so I would make it:

const consoleError = console.error;
console.error = (...args) => {
  global.ErrorUtils.reportError.apply(this, args);
  consoleError(...args);
}

But I have not looked into what global.ErrorUtils.reportError accepts. Maybe the args need to be joined first.

[Morriz]

Actually, I only think console.error should be hijacked in DEV, as I now get crashes in production when calling console.error.

[evelant]

I've torn my hair out for several days trying to figure out why my app is crashing in production. It was because of this issue. A library should NEVER overwrite the behavior of builtins like this! Sorry, I don't mean to sound ungrateful, I really appreciate this lib and it is quite helpful. This issue was very frustrating to track down however.

[robrechtme]

Hi. I see this is still an issue and I'd like to submit a PR. Any particular reason why you would report console.error in the first place? I would omit it entirely if possible.

[markymc]

Any news on this? I agree that overriding console.error feels dangerous, and led us to spend a fair amount of time wondering why our PropTypes weren't being checked, which turned out that they were, but the errors weren't being shown.

[diegovilar]

Isn't the goal of a global error handler to provide some logic to deal with uncaught errors?. A logged error (ie console.error(e)) is not an uncaught error; it's something that was logged through the runtime's console error channel.

The code bellow

global.ErrorUtils.setGlobalHandler(customHandler);
const consoleError = console.error;
console.error = (...args) => {
  const error = args[1];
  global.ErrorUtils.reportError(error);
  consoleError(...args);
}

effectively turns console.error(error) into customHandler(error) because global.ErrorUtils.reportError(error) will call customHandler(error) - that's what reportError does, it calls the handler defined by setGlobalHandler (source) in global.ErrorUtils.setGlobalHandler(customHandler).

In other words, simply console.erroring ends up triggering your global error handler, which seems like an undesirable side effect of using setJSExceptionHandler. God forbid you call console.error in your global handler, it's gonna overflow the stack 😮

Shouldn't the whole console.error hijacking here be dropped?

[bitcrumb]

For what it is worth, and since this library still seems to be keen on overriding console.error (even though the changes in https://github.com/a7ul/react-native-exception-handler/pull/149 already provide a cleaner override). You can easily undo the override as follows:

// Keep a reference to the original console.error
const originalConsoleError = console.error;
// Now register your custom JS exception handler, this will temporary override the console.error behavior
setJSExceptionHandler(error => {
  // you're error handling here
}, true);
// Restore the old behavior of console.error
console.error = originalConsoleError;

I agree that attaching error reporting behavior to console.error by this library is totally out-of-place.

Invoking console.error should only be done for displaying errors, and should not lead to this error being reported. That's why you register a global JS exception handler yourself in the first place.