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

[JS] JS API for global handling of unhandled exceptions #1194

Closed
ide opened this issue May 7, 2015 · 31 comments
Closed

[JS] JS API for global handling of unhandled exceptions #1194

ide opened this issue May 7, 2015 · 31 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@ide
Copy link
Contributor

ide commented May 7, 2015

The API I envision is similar to window.onerror but not necessarily the same. This would allow us to make RCTExceptionsManager.reportUnhandledException a lot smaller (we'd move much of the code into JS to keep the current behavior) and make it easier to do custom error handling.

@brentvatne brentvatne changed the title JS API for global handling of unhandled exceptions [JS] JS API for global handling of unhandled exceptions May 30, 2015
@d-vine
Copy link

d-vine commented Jun 11, 2015

@ide Hi. I would very much appreciate such an API. In the mean time is there a way to prevent an production app from crashing on any random JavaScript exception?

@JoeStanton
Copy link
Contributor

This would be great for adding something like Bugsnag to report exceptions when apps are in production.

Edit: In fact, I think ErrorUtils.setGlobalHandler((err, isFatal) => { ... }); might do the job here.

@richardgill
Copy link

We're looking to log something anytime there is an error, so we can catch bugs early. This would be very useful.

@ghost
Copy link

ghost commented Aug 5, 2015

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

@despairblue
Copy link
Contributor

would be definitely useful!:+1:

@sjmueller
Copy link
Contributor

@ide, what is the current recommended approach (or hack) for custom handling of global errors?

@ide
Copy link
Contributor Author

ide commented Sep 24, 2015

You could either hook into the native layer or monkeypatch the JS ExceptionsManager.

@mkonicek
Copy link
Contributor

@atticoos
Copy link
Contributor

Is it a bad idea to set my own global error handler?

My scenario was to

  • Report exception to Sentry
  • Possibly reload the bridge to avoid being stuck in the error state

@qbig
Copy link
Contributor

qbig commented Jan 26, 2016

Hi @ajwhite. Any luck completing the 2 points you mentioned?

@qbig
Copy link
Contributor

qbig commented Jan 27, 2016

Here: https://github.com/facebook/react-native/blob/d33b554f5d4bb7856cf2fc21175bab23d8225fe4/packager/react-packager/src/Resolver/polyfills/error-guard.js

var ErrorUtils = {
  _inGuard: 0,
  _globalHandler: null,
  setGlobalHandler: function(fun) {
    ErrorUtils._globalHandler = fun;
  },
//...

'setGlobalHandler' would overwrite a lot of the RN behaviour (eg. RedBox in development). Understanding that we could just monkey patch globalHandler, the "" seems alarming. So maybe an API like addGlobalHandler, so that we could add custom handler without breaking the default behaviour and and without messing with "internal" variables ? @ide @satya164

@atticoos
Copy link
Contributor

In production mode I call setGlobalHandler with my own handler, which does the two things listed above -- logging to sentry & reloading the bridge. Seems to work just fine.

@qbig
Copy link
Contributor

qbig commented Jan 27, 2016

@ajwhite Cool. How do you reload the bridge? Would really appreciate it if you could share a gist or something.

@qbig
Copy link
Contributor

qbig commented Jan 27, 2016

@ajwhite And you have a DEV check I suppose?

@atticoos
Copy link
Contributor

Exactly @qbig

Basically

if (!__DEV__) {
  ErrorUtils.setGlobalHandler(error => {
    sentry.capture(error);
    NativeModules.BridgeReloader.reload()
  });
}

Then your BridgeReloader would be an objC module that simply calls [_bridge reload]

Or in an Android module, you just get the running activity, finish it, and restart it

@ReactMethod
public void reload() {
  Activity activity = getCurrentActivity();
  Intent intent = activity.getIntent();
  activity.finish();
  activity.startActivity(intent);
}

@qbig
Copy link
Contributor

qbig commented Jan 27, 2016

That's neat:) @ajwhite Thanks so much!
I guess you are not using the raven-js plugin? which does something like :

    ErrorUtils.setGlobalHandler(Raven.captureException.bind(Raven));

But I don't like the fact that it is overwriting the default exception handler.

So I was considering something like

    var defaultHandler = ErrorUtils._globalHandler;
    ErrorUtils._globalHandler = function(...args){
      defaultHandler(...args);
      Raven.captureException(...args);
      // other custom handler
    };

@atticoos
Copy link
Contributor

Just took a read through the latest Raven source. The version I had used months back would report exceptions by loading an image.. That obviously doesn't work well for us here in React Native land. Their latest version looks like it has full support now though!

But I don't like the fact that it is overwriting the default exception handler.

That would work. I just re-read the source for the native error handler. It looks like it does two things:

I had no idea until now that it reloads the bridge by default. I don't know if this is a new development or not, but that's pretty cool. Unfortunately the application I work on is basically a "kiosk mode" type of thing running 24/7. I notice the handler has a maxRetries, which wouldn't work for me, so I'll likely bypass this as I am currently.

So keeping a reference to the original handler and calling it is probably a good thing in your example

@qbig
Copy link
Contributor

qbig commented Jan 27, 2016

@ide @satya164 is there any suggested way to add the handler ?

@ide
Copy link
Contributor Author

ide commented Jan 27, 2016

I think ErrorUtils.setGlobalHandler is the recommended way. Maybe the API should be changed to let you get access to the default handler.

@qbig
Copy link
Contributor

qbig commented Jan 27, 2016

@ide like a getter for "_globalHandler" ?

@ide
Copy link
Contributor Author

ide commented Jan 27, 2016

Yes, making it part of the public API.

@qbig
Copy link
Contributor

qbig commented Jan 27, 2016

var ErrorUtils = {
  _inGuard: 0,
  _globalHandler: null,
  getGlobalHandler : function() {
    return  ErrorUtils._globalHandler;
  },
  setGlobalHandler: function(fun) {
    ErrorUtils._globalHandler = fun;
  },
  reportError: function(error) {
    ErrorUtils._globalHandler && ErrorUtils._globalHandler(error);
  },
  reportFatalError: function(error) {
    ErrorUtils._globalHandler && ErrorUtils._globalHandler(error, true);
  },
// ...

Would do ? I'd be more than happy to PR

@satya164
Copy link
Contributor

@qbig Looks reasonable.

ghost pushed a commit that referenced this issue Feb 18, 2016
Summary:As discussed here #1194 . This add an getter the default handler, so that custom handler can be added (monkey patch?) without overwriting the default handler
Closes #5575

Differential Revision: D2948994

fb-gh-sync-id: 2b6d1619cfe68f78b326c6d232b9bf57c489c45d
shipit-source-id: 2b6d1619cfe68f78b326c6d232b9bf57c489c45d
pglotov pushed a commit to pglotov/react-native that referenced this issue Mar 15, 2016
Summary:As discussed here facebook#1194 . This add an getter the default handler, so that custom handler can be added (monkey patch?) without overwriting the default handler
Closes facebook#5575

Differential Revision: D2948994

fb-gh-sync-id: 2b6d1619cfe68f78b326c6d232b9bf57c489c45d
shipit-source-id: 2b6d1619cfe68f78b326c6d232b9bf57c489c45d
@jsierles
Copy link
Contributor

This issue appears to have been solved by #5575. Closing this issue out for now.

@jlo1
Copy link

jlo1 commented Apr 12, 2016

The ErrorUtils.setGlobalHandler works great when the app's process is attached to the debugger (regardless if in debug / release bundle): I see a red screen with the error message, and my own custom handler is called fine.

But how do I handle errors for production apps, where a debugger won't be attached? When a debugger isn't attached and a crash occurs, the app doesn't show a red screen and just closes the app and goes to the home screen. In this case, my custom handler isn't called at all.

(Specifically describing the flow for iOS bundled app here)

@npomfret
Copy link
Contributor

npomfret commented Jun 3, 2016

Hi, I'm fairly new to both RN and javascript. I've put the following into my code... It catches errors and warnings in an array before reporting them as normal to the console.

It sits at the top of my main.js file. It seems to work, but is this a terrible idea?

const orig_console_warn = console.warn;
const orig_console_error = console.error;

let errors = [];
let warnings = [];

console.warn = function() {
  warnings.push(arguments);
  orig_console_warn.apply(console, arguments);
};

console.error = function() {
  errors.push(arguments);
  orig_console_error.apply(console, arguments);
};

I then periodically clear the array and send the data to some reporting system.

@GeoffreyPlitt
Copy link

@jlo1 Did you ever find a solution for non-debug mode?

@antoinerousseau
Copy link
Contributor

I'm pretty sure that global.ErrorUtils.setGlobalHandler works in production mode too on Android, fyi.

@GeoffreyPlitt
Copy link

What about iOS? Is there a platform-agnostic way to do this?

@npomfret
Copy link
Contributor

I ended up trapping console.log, console.info, console.warn and console.error and writing the data to an in-memory buffer that I can view in my app. I also send error and warn messages to a server that records the data so I can see error from my clients in one central place.

@nsyujian
Copy link

iOS
  RCTSetFatalHandler(^(NSError *error) {
    [Bugly reportError:error];
    
     [rootView.bridge reload];
  });

@facebook facebook locked as resolved and limited conversation to collaborators Jul 22, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests