diff --git a/CHANGELOG.md b/CHANGELOG.md index d343756795..b13ac64ed6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ ### Fixes +- Rethrow native exceptions from Sentry's Expo host handler so Android crashes terminate the process instead of leaving the app on a blank screen ([#6228](https://github.com/getsentry/sentry-react-native/pull/6228)) - Bound TTID/TTFD to prevent inflated transactions ([#6210](https://github.com/getsentry/sentry-react-native/pull/6210)) - Return `NO` from `requiresMainQueueSetup` to avoid unnecessary `dispatch_sync` on the main queue during bridge init ([#6202](https://github.com/getsentry/sentry-react-native/pull/6202)) diff --git a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/expo/SentryReactNativeHostHandlerTest.kt b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/expo/SentryReactNativeHostHandlerTest.kt index 8a1e0c3c6c..5759df7841 100644 --- a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/expo/SentryReactNativeHostHandlerTest.kt +++ b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/expo/SentryReactNativeHostHandlerTest.kt @@ -6,12 +6,16 @@ import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.JUnit4 import org.mockito.MockedStatic +import org.mockito.Mockito.doReturn import org.mockito.Mockito.mockStatic +import org.mockito.Mockito.spy import org.mockito.kotlin.any import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.never @@ -26,6 +30,20 @@ class SentryReactNativeHostHandlerTest { sentryMock?.close() } + /** Creates a handler that pretends `expo-updates` is on the classpath, so the rethrow is suppressed. */ + private fun handlerWithExpoUpdates(): SentryReactNativeHostHandler { + val handler = spy(SentryReactNativeHostHandler()) + doReturn(true).`when`(handler).isExpoUpdatesPresent() + return handler + } + + /** Creates a handler that pretends `expo-updates` is absent, so the original exception is rethrown. */ + private fun handlerWithoutExpoUpdates(): SentryReactNativeHostHandler { + val handler = spy(SentryReactNativeHostHandler()) + doReturn(false).`when`(handler).isExpoUpdatesPresent() + return handler + } + @Test fun `does not capture when in developer support mode`() { sentryMock = @@ -33,36 +51,45 @@ class SentryReactNativeHostHandlerTest { it.`when` { Sentry.isEnabled() }.thenReturn(true) } - val handler = SentryReactNativeHostHandler() + val handler = handlerWithoutExpoUpdates() + // In dev mode we bail early — no capture and no rethrow. handler.onReactInstanceException(true, RuntimeException("test")) sentryMock!!.verify({ Sentry.captureException(any()) }, never()) } @Test - fun `does not capture when sentry is not enabled`() { + fun `does not capture when sentry is not enabled but still rethrows`() { sentryMock = mockStatic(Sentry::class.java).also { it.`when` { Sentry.isEnabled() }.thenReturn(false) } - val handler = SentryReactNativeHostHandler() - handler.onReactInstanceException(false, RuntimeException("test")) + val handler = handlerWithoutExpoUpdates() + val originalException = RuntimeException("test") + + val thrown = assertThrows(RuntimeException::class.java) { + handler.onReactInstanceException(false, originalException) + } + assertSame(originalException, thrown) sentryMock!!.verify({ Sentry.captureException(any()) }, never()) } @Test - fun `captures exception with unhandled mechanism when sentry is enabled`() { + fun `captures exception with unhandled mechanism when sentry is enabled and rethrows`() { sentryMock = mockStatic(Sentry::class.java).also { it.`when` { Sentry.isEnabled() }.thenReturn(true) } - val handler = SentryReactNativeHostHandler() + val handler = handlerWithoutExpoUpdates() val originalException = IllegalStateException("Fabric crash") - handler.onReactInstanceException(false, originalException) + val thrown = assertThrows(IllegalStateException::class.java) { + handler.onReactInstanceException(false, originalException) + } + assertSame(originalException, thrown) val captor = argumentCaptor() sentryMock!!.verify { Sentry.captureException(captor.capture()) } @@ -82,15 +109,36 @@ class SentryReactNativeHostHandlerTest { } @Test - fun `does not throw when sentry capture fails`() { + fun `rethrows original exception even when sentry capture fails`() { sentryMock = mockStatic(Sentry::class.java).also { it.`when` { Sentry.isEnabled() }.thenReturn(true) - it.`when` { Sentry.captureException(any()) }.thenThrow(RuntimeException("Sentry internal error")) + it.`when` { Sentry.captureException(any()) } + .thenThrow(RuntimeException("Sentry internal error")) } - val handler = SentryReactNativeHostHandler() - // Should not throw + val handler = handlerWithoutExpoUpdates() + val originalException = IllegalStateException("test") + + // Sentry's internal failure must be swallowed, but the original native exception is still + // rethrown so Android's UncaughtExceptionHandler can terminate the process. + val thrown = assertThrows(IllegalStateException::class.java) { + handler.onReactInstanceException(false, originalException) + } + assertSame(originalException, thrown) + } + + @Test + fun `does not rethrow when expo-updates is present`() { + sentryMock = + mockStatic(Sentry::class.java).also { + it.`when` { Sentry.isEnabled() }.thenReturn(true) + } + + val handler = handlerWithExpoUpdates() + // Must not throw — expo-updates' error-recovery flow gets a chance to run. handler.onReactInstanceException(false, IllegalStateException("test")) + + sentryMock!!.verify { Sentry.captureException(any()) } } } diff --git a/packages/core/android/expo-handler/src/main/java/io/sentry/react/expo/SentryReactNativeHostHandler.java b/packages/core/android/expo-handler/src/main/java/io/sentry/react/expo/SentryReactNativeHostHandler.java index eda4ef2732..fea06e7f78 100644 --- a/packages/core/android/expo-handler/src/main/java/io/sentry/react/expo/SentryReactNativeHostHandler.java +++ b/packages/core/android/expo-handler/src/main/java/io/sentry/react/expo/SentryReactNativeHostHandler.java @@ -12,8 +12,9 @@ * *

On Expo SDK 53+, certain native exceptions (e.g., IllegalStateException from Fabric's * SurfaceMountingManager) are caught by React Native and routed to {@code handleInstanceException}. - * Expo's implementation iterates registered host handlers but does not rethrow, so the exception - * never reaches Java's {@code UncaughtExceptionHandler} which Sentry relies on for crash capture. + * Expo's implementation iterates registered host handlers but does not rethrow when at least one + * handler is registered, so the exception never reaches Java's {@code UncaughtExceptionHandler} + * which Sentry relies on for crash capture. * *

This handler captures those exceptions directly via {@code Sentry.captureException} with an * unhandled mechanism, ensuring they appear as crashes in Sentry. @@ -21,6 +22,7 @@ public class SentryReactNativeHostHandler implements ReactNativeHostHandler { private static final String MECHANISM_TYPE = "expoReactHost"; + private static final String EXPO_UPDATES_MARKER_CLASS = "expo.modules.updates.UpdatesPackage"; @Override public void onReactInstanceException(boolean useDeveloperSupport, @NonNull Exception exception) { @@ -28,21 +30,40 @@ public void onReactInstanceException(boolean useDeveloperSupport, @NonNull Excep return; } - if (!Sentry.isEnabled()) { - return; - } + if (Sentry.isEnabled()) { + try { + final Mechanism mechanism = new Mechanism(); + mechanism.setType(MECHANISM_TYPE); + mechanism.setHandled(false); - try { // NOPMD - We don't want to crash in any case - final Mechanism mechanism = new Mechanism(); - mechanism.setType(MECHANISM_TYPE); - mechanism.setHandled(false); + final ExceptionMechanismException mechanismException = + new ExceptionMechanismException(mechanism, exception, Thread.currentThread()); + + Sentry.captureException(mechanismException); + } catch (Throwable ignored) { // NOPMD - We don't want to crash in any case + // ignore + } + } - final ExceptionMechanismException mechanismException = - new ExceptionMechanismException(mechanism, exception, Thread.currentThread()); + // Restore React Native's default crash behavior that Expo's host-handler loop swallows when at + // least one handler is registered. + if (!isExpoUpdatesPresent()) { + sneakyThrow(exception); + } + } - Sentry.captureException(mechanismException); - } catch (Throwable ignored) { // NOPMD - We don't want to crash in any case - // ignore + // Visible for testing. + boolean isExpoUpdatesPresent() { + try { + Class.forName(EXPO_UPDATES_MARKER_CLASS, false, getClass().getClassLoader()); + return true; + } catch (ClassNotFoundException e) { + return false; } } + + @SuppressWarnings("unchecked") + private static void sneakyThrow(@NonNull Throwable t) throws E { + throw (E) t; + } }