fix(core): Breadcrumbs added on forked context are now captured (#4124)

getsentry · Nov 18, 2024 · fd54b38 · fd54b38
1 parent e22745e
commit fd54b38
Show file tree

Hide file tree

Showing 12 changed files with 297 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,12 @@
 > make sure you follow our [migration guide](https://docs.sentry.io/platforms/react-native/migration/) first.
 <!-- prettier-ignore-end -->
 
+## Unreleased
+
+### Fixes
+
+- Prevents exception capture context from being overwritten by native scope sync ([#4124](https://github.com/getsentry/sentry-react-native/pull/4124))
+
 ## 6.2.0
 
 ### Features

diff --git a/.../RNSentryAndroidTester/app/src/androidTest/java/io/sentry/react/RNSentryModuleImplTest.kt b/.../RNSentryAndroidTester/app/src/androidTest/java/io/sentry/react/RNSentryModuleImplTest.kt
@@ -0,0 +1,86 @@
+package io.sentry.react
+
+import android.content.Context
+import androidx.test.platform.app.InstrumentationRegistry
+import com.facebook.react.bridge.PromiseImpl
+import com.facebook.react.bridge.ReactApplicationContext
+import com.facebook.react.bridge.WritableMap
+import com.facebook.soloader.SoLoader
+import io.sentry.Breadcrumb
+import io.sentry.Scope
+import io.sentry.SentryOptions
+import io.sentry.android.core.SentryAndroidOptions
+import org.junit.Assert.assertEquals
+import org.junit.Assert.fail
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+@RunWith(JUnit4::class)
+class RNSentryModuleImplTest {
+
+    private lateinit var module: RNSentryModuleImpl
+    private lateinit var context: Context
+
+    @Before
+    fun setUp() {
+        context = InstrumentationRegistry.getInstrumentation().targetContext
+        SoLoader.init(context, false)
+        val reactContext = ReactApplicationContext(context)
+        module = RNSentryModuleImpl(reactContext)
+    }
+
+    @Test
+    fun fetchNativeDeviceContextsWithNullContext() {
+        val options = SentryAndroidOptions()
+        val scope = Scope(options)
+        val promise = PromiseImpl({
+            assertEquals(1, it.size)
+            assertEquals(null, it[0])
+        }, {
+            fail("Promise was rejected unexpectedly")
+        })
+        module.fetchNativeDeviceContexts(promise, options, null, scope)
+    }
+
+    @Test
+    fun fetchNativeDeviceContextsWithInvalidSentryOptions() {
+        class NotAndroidSentryOptions : SentryOptions()
+
+        val options = NotAndroidSentryOptions()
+        val scope = Scope(options)
+        val promise = PromiseImpl({
+            assertEquals(1, it.size)
+            assertEquals(null, it[0])
+        }, {
+            fail("Promise was rejected unexpectedly")
+        })
+        module.fetchNativeDeviceContexts(promise, options, context, scope)
+    }
+
+    @Test
+    fun fetchNativeDeviceContextsFiltersBreadcrumbs() {
+        val options = SentryAndroidOptions().apply { maxBreadcrumbs = 5 }
+        val scope = Scope(options)
+        scope.addBreadcrumb(Breadcrumb("Breadcrumb1-RN").apply { origin = "react-native" })
+        scope.addBreadcrumb(Breadcrumb("Breadcrumb2-Native"))
+        scope.addBreadcrumb(Breadcrumb("Breadcrumb3-Native").apply { origin = "java" })
+        scope.addBreadcrumb(Breadcrumb("Breadcrumb2-RN").apply { origin = "react-native" })
+        scope.addBreadcrumb(Breadcrumb("Breadcrumb2-RN").apply { origin = "react-native" })
+
+        val promise = PromiseImpl({
+            assertEquals(1, it.size)
+            assertEquals(true, it[0] is WritableMap)
+            val actual = it[0] as WritableMap
+            val breadcrumbs = actual.getArray("breadcrumbs")
+            assertEquals(2, breadcrumbs?.size())
+            assertEquals("Breadcrumb2-Native", breadcrumbs?.getMap(0)?.getString("message"))
+            assertEquals("Breadcrumb3-Native", breadcrumbs?.getMap(1)?.getString("message"))
+        }, {
+            fail("Promise was rejected unexpectedly")
+        })
+
+        module.fetchNativeDeviceContexts(promise, options, context, scope)
+    }
+}
diff --git a/...AndroidTester/app/src/test/java/io/sentry/rnsentryandroidtester/RNSentryBreadcrumbTest.kt b/...AndroidTester/app/src/test/java/io/sentry/rnsentryandroidtester/RNSentryBreadcrumbTest.kt
@@ -1,6 +1,7 @@
 package io.sentry.rnsentryandroidtester
 
 import com.facebook.react.bridge.JavaOnlyMap
+import io.sentry.SentryLevel
 import io.sentry.react.RNSentryBreadcrumb
 import junit.framework.TestCase.assertEquals
 import org.junit.Test
@@ -10,6 +11,38 @@ import org.junit.runners.JUnit4
 @RunWith(JUnit4::class)
 class RNSentryBreadcrumbTest {
 
+    @Test
+    fun generatesSentryBreadcrumbFromMap() {
+        val testData = JavaOnlyMap.of(
+            "test", "data",
+        )
+        val map = JavaOnlyMap.of(
+            "level", "error",
+            "category", "testCategory",
+            "origin", "testOrigin",
+            "type", "testType",
+            "message", "testMessage",
+            "data", testData,
+        )
+        val actual = RNSentryBreadcrumb.fromMap(map)
+        assertEquals(SentryLevel.ERROR, actual.level)
+        assertEquals("testCategory", actual.category)
+        assertEquals("testOrigin", actual.origin)
+        assertEquals("testType", actual.type)
+        assertEquals("testMessage", actual.message)
+        assertEquals(testData.toHashMap(), actual.data)
+    }
+
+    @Test
+    fun reactNativeForMissingOrigin() {
+        val map = JavaOnlyMap.of(
+            "message", "testMessage",
+        )
+        val actual = RNSentryBreadcrumb.fromMap(map)
+        assertEquals("testMessage", actual.message)
+        assertEquals("react-native", actual.origin)
+    }
+
     @Test
     fun nullForMissingCategory() {
         val map = JavaOnlyMap.of()

diff --git a/packages/core/RNSentryCocoaTester/RNSentryCocoaTester.xcodeproj/project.pbxproj b/packages/core/RNSentryCocoaTester/RNSentryCocoaTester.xcodeproj/project.pbxproj
@@ -13,6 +13,7 @@
 		33AFDFED2B8D14B300AAB120 /* RNSentryFramesTrackerListenerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 33AFDFEC2B8D14B300AAB120 /* RNSentryFramesTrackerListenerTests.m */; };
 		33AFDFF12B8D15E500AAB120 /* RNSentryDependencyContainerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 33AFDFF02B8D15E500AAB120 /* RNSentryDependencyContainerTests.m */; };
 		33F58AD02977037D008F60EA /* RNSentryTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 33F58ACF2977037D008F60EA /* RNSentryTests.mm */; };
+		AEFB00422CC90C4B00EC8A9A /* RNSentryBreadcrumbTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3360843C2C340C76008CC412 /* RNSentryBreadcrumbTests.swift */; };
 		B5859A50A3E865EF5E61465A /* libPods-RNSentryCocoaTesterTests.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 650CB718ACFBD05609BF2126 /* libPods-RNSentryCocoaTesterTests.a */; };
 /* End PBXBuildFile section */
 
@@ -221,6 +222,7 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				AEFB00422CC90C4B00EC8A9A /* RNSentryBreadcrumbTests.swift in Sources */,
 				332D33472CDBDBB600547D76 /* RNSentryReplayOptionsTests.swift in Sources */,
 				33AFDFF12B8D15E500AAB120 /* RNSentryDependencyContainerTests.m in Sources */,
 				336084392C32E382008CC412 /* RNSentryReplayBreadcrumbConverterTests.swift in Sources */,

diff --git a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryBreadcrumbTests.swift b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryBreadcrumbTests.swift
@@ -1,12 +1,13 @@
 import Sentry
 import XCTest
 
-class RNSentryBreadcrumbTests: XCTestCase {
+final class RNSentryBreadcrumbTests: XCTestCase {
 
     func testGeneratesSentryBreadcrumbFromNSDictionary() {
         let actualCrumb = RNSentryBreadcrumb.from([
             "level": "error",
             "category": "testCategory",
+            "origin": "testOrigin",
             "type": "testType",
             "message": "testMessage",
             "data": [
@@ -16,11 +17,29 @@ class RNSentryBreadcrumbTests: XCTestCase {
 
         XCTAssertEqual(actualCrumb!.level, SentryLevel.error)
         XCTAssertEqual(actualCrumb!.category, "testCategory")
+        XCTAssertEqual(actualCrumb!.origin, "testOrigin")
         XCTAssertEqual(actualCrumb!.type, "testType")
         XCTAssertEqual(actualCrumb!.message, "testMessage")
         XCTAssertEqual((actualCrumb!.data)!["test"] as! String, "data")
     }
 
+    func testUsesReactNativeAsDefaultOrigin() {
+        let actualCrumb = RNSentryBreadcrumb.from([
+            "message": "testMessage"
+        ])
+
+        XCTAssertEqual(actualCrumb!.origin, "react-native")
+    }
+
+    func testKeepsOriginIfSet() {
+        let actualCrumb = RNSentryBreadcrumb.from([
+            "message": "testMessage",
+            "origin": "someOrigin"
+        ])
+
+        XCTAssertEqual(actualCrumb!.origin, "someOrigin")
+    }
+
     func testUsesInfoAsDefaultSentryLevel() {
         let actualCrumb = RNSentryBreadcrumb.from([
             "message": "testMessage"

diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryBreadcrumb.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryBreadcrumb.java
@@ -51,6 +51,12 @@ public static Breadcrumb fromMap(ReadableMap from) {
       breadcrumb.setCategory(from.getString("category"));
     }
 
+    if (from.hasKey("origin")) {
+      breadcrumb.setOrigin(from.getString("origin"));
+    } else {
+      breadcrumb.setOrigin("react-native");
+    }
+
     if (from.hasKey("level")) {
       switch (from.getString("level")) {
         case "fatal":

diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java
@@ -28,6 +28,7 @@
 import com.facebook.react.bridge.WritableNativeMap;
 import com.facebook.react.common.JavascriptException;
 import com.facebook.react.modules.core.DeviceEventManagerModule;
+import io.sentry.Breadcrumb;
 import io.sentry.HubAdapter;
 import io.sentry.ILogger;
 import io.sentry.IScope;
@@ -76,6 +77,7 @@
 import java.io.InputStream;
 import java.nio.charset.Charset;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -886,18 +888,35 @@ private String readStringFromFile(File path) throws IOException {
 
   public void fetchNativeDeviceContexts(Promise promise) {
     final @NotNull SentryOptions options = HubAdapter.getInstance().getOptions();
+    final @Nullable Context context = this.getReactApplicationContext().getApplicationContext();
+    final @Nullable IScope currentScope = InternalSentrySdk.getCurrentScope();
+    fetchNativeDeviceContexts(promise, options, context, currentScope);
+  }
+
+  protected void fetchNativeDeviceContexts(
+      Promise promise,
+      final @NotNull SentryOptions options,
+      final @Nullable Context context,
+      final @Nullable IScope currentScope) {
     if (!(options instanceof SentryAndroidOptions)) {
       promise.resolve(null);
       return;
     }
-
-    final @Nullable Context context = this.getReactApplicationContext().getApplicationContext();
     if (context == null) {
       promise.resolve(null);
       return;
     }
+    if (currentScope != null) {
+      // Remove react-native breadcrumbs
+      Iterator<Breadcrumb> breadcrumbsIterator = currentScope.getBreadcrumbs().iterator();
+      while (breadcrumbsIterator.hasNext()) {
+        Breadcrumb breadcrumb = breadcrumbsIterator.next();
+        if ("react-native".equals(breadcrumb.getOrigin())) {
+          breadcrumbsIterator.remove();
+        }
+      }
+    }
 
-    final @Nullable IScope currentScope = InternalSentrySdk.getCurrentScope();
     final @NotNull Map<String, Object> serialized =
         InternalSentrySdk.serializeScope(context, (SentryAndroidOptions) options, currentScope);
     final @Nullable Object deviceContext = RNSentryMapConverter.convertToWritable(serialized);

diff --git a/packages/core/ios/RNSentry.mm b/packages/core/ios/RNSentry.mm
@@ -442,6 +442,16 @@ - (NSDictionary *)fetchNativeStackFramesBy:(NSArray<NSNumber *> *)instructionsAd
 
     [serializedScope setValue:contexts forKey:@"contexts"];
     [serializedScope removeObjectForKey:@"context"];
+
+    // Remove react-native breadcrumbs
+    NSPredicate *removeRNBreadcrumbsPredicate =
+        [NSPredicate predicateWithBlock:^BOOL(NSDictionary *breadcrumb, NSDictionary *bindings) {
+            return ![breadcrumb[@"origin"] isEqualToString:@"react-native"];
+        }];
+    NSArray *breadcrumbs = [[serializedScope[@"breadcrumbs"] mutableCopy]
+        filteredArrayUsingPredicate:removeRNBreadcrumbsPredicate];
+    [serializedScope setValue:breadcrumbs forKey:@"breadcrumbs"];
+
     resolve(serializedScope);
 }
 

diff --git a/packages/core/ios/RNSentryBreadcrumb.m b/packages/core/ios/RNSentryBreadcrumb.m
@@ -23,6 +23,12 @@ + (SentryBreadcrumb *)from:(NSDictionary *)dict
 
     [crumb setLevel:sentryLevel];
     [crumb setCategory:dict[@"category"]];
+    id origin = dict[@"origin"];
+    if (origin != nil) {
+        [crumb setOrigin:origin];
+    } else {
+        [crumb setOrigin:@"react-native"];
+    }
     [crumb setType:dict[@"type"]];
     [crumb setMessage:dict[@"message"]];
     [crumb setData:dict[@"data"]];

diff --git a/packages/core/src/js/integrations/devicecontext.ts b/packages/core/src/js/integrations/devicecontext.ts
@@ -1,5 +1,5 @@
 /* eslint-disable complexity */
-import type { Event, Integration } from '@sentry/types';
+import type { Client, Event, EventHint, Integration } from '@sentry/types';
 import { logger, severityLevelFromString } from '@sentry/utils';
 import { AppState } from 'react-native';
 
@@ -20,7 +20,7 @@ export const deviceContextIntegration = (): Integration => {
   };
 };
 
-async function processEvent(event: Event): Promise<Event> {
+async function processEvent(event: Event, _hint: EventHint, client: Client): Promise<Event> {
   let native: NativeDeviceContextsResponse | null = null;
   try {
     native = await NATIVE.fetchNativeDeviceContexts();
@@ -83,7 +83,11 @@ async function processEvent(event: Event): Promise<Event> {
     ? native['breadcrumbs'].map(breadcrumbFromObject)
     : undefined;
   if (nativeBreadcrumbs) {
-    event.breadcrumbs = nativeBreadcrumbs;
+    const maxBreadcrumbs = client?.getOptions().maxBreadcrumbs ?? 100; // Default is 100.
+    event.breadcrumbs = nativeBreadcrumbs
+      .concat(event.breadcrumbs || []) // concatenate the native and js breadcrumbs
+      .sort((a, b) => (a.timestamp ?? 0) - (b.timestamp ?? 0)) // sort by timestamp
+      .slice(-maxBreadcrumbs); // keep the last maxBreadcrumbs
   }
 
   return event;