Re-enable Sentry on Android, with no libsentry.so at all #5782
Assignees
Labels
Comments
gnprice
added
a-Android
upstream: other
gnprice
added a commit
to gnprice/zulip-mobile
that referenced
this issue
Oct 31, 2023
Toward zulip#5782. I've tested, on a build with this change and then Sentry enabled, that: * The APK is free of any native libraries that have an obvious relationship to Sentry, in particular `libsentry.so` and `libsentry-android.so` which appear without this change. * In brief manual testing of the app, nothing goes obviously wrong. So at least Sentry doesn't, say, cause a crash at startup whenever it can't find `libsentry.so`. * Upon adding some artificial warnings and errors, they show up in the Sentry dashboard. This includes `logging.warn` in JS; uncaught exceptions in JS; `ZLog.w` in Kotlin; and an uncaught exception in Kotlin in handling a notification. I also tried an uncaught exception in Kotlin in a `@ReactMethod` implementation, and that one didn't work. But I think that's mostly React Native's fault, and is likely not a regression: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/re-enabling.20Se ntry/near/1654173 The big remaining question to resolve is whether this causes some crashes in the wild. Given that the native code is gone, that isn't real likely; but having been burned before, we'll want to be a bit cautious, by validating this with a solid beta period.
gnprice
added a commit
to gnprice/zulip-mobile
that referenced
this issue
Oct 31, 2023
This reverts commit 033a9d6. Fixes: zulip#5782
chrisbobbe
pushed a commit
that referenced
this issue
Nov 1, 2023
Toward #5782. I've tested, on a build with this change and then Sentry enabled, that: * The APK is free of any native libraries that have an obvious relationship to Sentry, in particular `libsentry.so` and `libsentry-android.so` which appear without this change. * In brief manual testing of the app, nothing goes obviously wrong. So at least Sentry doesn't, say, cause a crash at startup whenever it can't find `libsentry.so`. * Upon adding some artificial warnings and errors, they show up in the Sentry dashboard. This includes `logging.warn` in JS; uncaught exceptions in JS; `ZLog.w` in Kotlin; and an uncaught exception in Kotlin in handling a notification. I also tried an uncaught exception in Kotlin in a `@ReactMethod` implementation, and that one didn't work. But I think that's mostly React Native's fault, and is likely not a regression: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/re-enabling.20Se ntry/near/1654173 The big remaining question to resolve is whether this causes some crashes in the wild. Given that the native code is gone, that isn't real likely; but having been burned before, we'll want to be a bit cautious, by validating this with a solid beta period.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is a reprise of #5766. We effectively reverted #5765 in commit 033a9d6, because it turned out that Sentry's new
enableNdk: falseoption was not as effective as advertised in disabling Sentry's native code — thanks to Anders for the forensics — and that enabling Sentry with that in place brought the [crashes right back](https://chat.zulip.org/#narrow/stream/48-mobile/topic/Android.20crash.20.23M5778/near/1659732).
Instead, Sentry upstream now suggests a way to exclude the Sentry native code at the Gradle level:
getsentry/sentry-java#2955 (comment)
Hopefully that will cause
libsentry.soto not even exist in the app, and will cause the crashes to stop.The text was updated successfully, but these errors were encountered: