Improve integration with sentry

[koke]

We use Sentry for crash reporting in the apps, and they have a react-native client that allows for better stack traces. They even support mixed stack traces on iOS.

I did some initial experiments a while back but I recall the initialization of the JS client was running into some issues because we already had a client initialized on the iOS side.

We need more research to see how we can make this work.

[koke]

They even support mixed stack traces on iOS.

As I was writing this, I realized that maybe this was the problem, and deactivating those would help. What I remember seeing is that the RN client was trying to take over native reporting, which was problematic because that was already initialized on the app side.

Maybe if we initialize the JS client with deactivateStacktraceMerging: false it would work. We would be giving up on mixed stacktraces, but we don't have those on Android either.

My question would still be: if there's a JS exception the JS client would log it, but the app would still crash? Will the native client log a second crash just like it was doing until now? Would there be a way to pair the two?

[jkmassel]

As I was writing this, I realized that maybe this was the problem, and deactivating those would help. What I remember seeing is that the RN client was trying to take over native reporting, which was problematic because that was already initialized on the app side.

That can be worked around without much difficulty by adjusting the Tracks+Sentry code to manage its own entire instance of the Sentry client without using the shared one. I'll try to get a PR for that done today.

if there's a JS exception the JS client would log it, but the app would still crash?

My knowledge of RN is rather limited, but assuming it runs in a JSContext, we should be able to attach an exception handler that prevents the entire app from crashing. But I'm not entirely certain how RN works under the hood.

IMHO going for mixed stack traces is ideal, if possible, so I'm happy to prioritize any work required on the native side to make this work.

[jkmassel]

I spent some time today digging into this a little bit, and I have some thoughts on implementation.

First off, using the https://github.com/getsentry/sentry-react-native framework may not be the best solution. It appears to be designed for react-native only apps, not embedded libraries. The full-stack capabilities seem to be more designed for tracing native dependencies of the react native app than for embedding the react-native project inside a larger app (that also uses Sentry).

The main way this manifests is in the fact that RNSentry bundles the entire Sentry for iOS framework. At the moment, this would actually work, as Sentry is being dynamically linked by WPiOS as a dependency of Tracks, but we're hoping to move towards statically linking all dependencies for faster app startup. It seems the react native bridge is already compiled this way (which incidentally, would break the app if Sentry were included in this manner). If we were to statically link all dependencies, we'd get duplicate symbol errors.

That said, there might be another way around this. The editor defines an error boundary component that contains componentDidCatch. If we created a mobile version of this component, rather than crashing the app, we could (possibly?) catch the error and display a helpful notice. The error, once caught, could be sent to the bridge for further processing. From there, Tracks / Gutenberg could use a shared protocol to inject an object that'd deal with error handling (and possibly Tracks logging as well?)

I'd expect that the gutenberg-mobile would emit an encoded version of the JavaScript err.stack property. From there, it could be provided to Sentry's parseJavaScriptStacktrace method. Interestingly, Sentry also has a convertReactNativeStacktrace method. I'm not 100% sure how that one works, but might be worth looking into further, as it likely has even better information for us.

---

The Android implementation would be a little bit more work. Sentry on Android doesn't bundle JS exception handling logic. That said, if we have a common format being sent by the RN bundle, reverse-engineering the parsing logic for Android shouldn't be terribly difficult – we could just translate the Obj-C implementation.

---

Happy to hear your thoughts on this – I'm pretty new to RN, so I defer to y'all's collective wisdom on this!

[koke]

I did some quick tests and it seems componentDidCatch would only catch errors on render, but not on any other part of the code. Instead, it seems it's possible to use ErrorUtils to register a global handler:

import React from 'react';
import { View } from 'react-native';
import CrashButton from '@wordpress-mobile/crash-button';

class SimpleReactNativeApp extends React.Component {
  constructor( props ) {
    super( props );

    const defaultHanlder = ErrorUtils.getGlobalHandler();
    const appErrorHandler = ( e, isFatal ) => {
      // Send error to Sentry
      defaultHanlder( e, isFatal );
    };
    ErrorUtils.setGlobalHandler( appErrorHandler );
  }

  render() {
    return (
      <View>
        <CrashButton
          title='🙈 Crash me'
          errorMessage='🙊 I crashed!'
        />
      </View>
    );  
  }
}

I would have hoped we could use their library to save us some effort, but if we can't, the proposed approach sounds reasonable.

[jkmassel]

Sounds good – I'll get started on implementation along these lines right away. If anyone has any objections to this, please let me know! 😄

[hypest]

I'll get started on implementation along these lines right away

👋 @jkmassel , I wonder if there are any updates on the implementation side of this. Any blockers found perhaps?

In the meantime, removing this from the Open Beta target, as decided over a planning meeting.

[fluiddot]

I tackled this issue as my HACK week project (June 2021), I only managed to integrate the iOS part but so far the tests I've run everything is working nice 🎊 .

Here goes a sneak peek 👀:

In the following links you can find the different PRs with the implementation:

• Gutenberg - [RNMobile] Add Sentry native module
• Gutenberg-Mobile - Add Sentry SDK
• WordPress-iOS - [Gutenberg] Add Sentry React native SDK
• Automattic-Tracks-iOS - Crash logging: Add hybrid SDK helpers to allow better stack traces for other platforms

Besides, feel free to check the recap of my work and the approach I proposed in this internal reference (p9ugOq-249-p2).