Automattic / Automattic-Tracks-iOS

Client library for tracking user events for later analysis
GNU General Public License v2.0
40 stars 12 forks source link

Add support to log JavaScript exceptions to Sentry #278

Closed fluiddot closed 4 months ago

fluiddot commented 5 months ago

Related PRs:

This PR expands the Crash logging service to support logging JavaScript exceptions to Sentry. This will allow hybrid sources like React Native (used in Gutenberg Mobile) to log exceptions with detailed information and symbolicated stack traces, which will greatly simplify the crash debugging process.

The approach is based on previous attempts to log JavaScript exceptions (p9ugOq-249-p2). However, in this case, we are following an SDK-less implementation where the exception is bubbled up to the host app and sent as a Sentry event using the Sentry iOS SDK. As can be seen in the implementation, the Sentry event is adjusted to ensure that Sentry processes it as a JavaScript exception and hence, symbolicate the stack trace with previously uploaded source maps.

The process to log a JavaScript exception has the following steps:

Regarding the source maps, they are uploaded as part of the build process of the host app (reference).

More information about this approach is detailed in p9ugOq-4p6-p2.

To test

Since it involves testing exceptions, we need to modify the code to force them and generate an installable build. A test PR has been created for this purpose, follow the testing instructions from https://github.com/wordpress-mobile/gutenberg-mobile/pull/6654.


fluiddot commented 4 months ago

I'm not familiar with this codebase and not sure how feasible/easy it is to add unit tests, but it'd be nice to add some unit tests to ensure the event payload scheme is correct, i.e. no threads and debug_meta.

Yep, I haven't added unit tests yet to this repository either. I briefly checked the unit tests of CrashLogging class and noticed that the ones related to error reporting are disabled (reference). I can take a look in a separate PR to incorporate tests for the logic around parsing a JavaScript exception.

Thank you so much @crazytonyli for your suggestions and for reviewing the PR 🙇 !

fluiddot commented 4 months ago

I noticed that the job buildkite/tracks-ios/validating-podspec is failing in trunk (reference) and other PRs (reference). In fact, I saw https://github.com/Automattic/Automattic-Tracks-iOS/pull/274#issuecomment-1861711772 on a past PR stating that should be safe to merge. Hence, I'm going to proceed with merging the PR.