Merge f828fc7 into d2c32bb

Author: antonis

CHANGELOG.md

@@ -32,6 +32,7 @@
 
 ### Fixes
 
+- Prevents exception capture context from being overwritten by native scope sync ([#4124](https://github.com/getsentry/sentry-react-native/pull/4124))
 - Skips ignoring require cycle logs for RN 0.70 or newer ([#4214](https://github.com/getsentry/sentry-react-native/pull/4214))
 - Enhanced accuracy of time-to-display spans. ([#4189](https://github.com/getsentry/sentry-react-native/pull/4189))
 

packages/core/RNSentryAndroidTester/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()

packages/core/RNSentryCocoaTester/RNSentryCocoaTester.xcodeproj/project.pbxproj

@@ -12,6 +12,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 */
 
@@ -212,6 +213,7 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				AEFB00422CC90C4B00EC8A9A /* RNSentryBreadcrumbTests.swift in Sources */,
 				33AFDFF12B8D15E500AAB120 /* RNSentryDependencyContainerTests.m in Sources */,
 				336084392C32E382008CC412 /* RNSentryReplayBreadcrumbConverterTests.swift in Sources */,
 				33F58AD02977037D008F60EA /* RNSentryTests.mm in Sources */,

packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryBreadcrumbTests.swift

@@ -1,12 +1,13 @@
 import XCTest
 import Sentry
 
-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"

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":

packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java

@@ -27,6 +27,7 @@
 import com.facebook.react.bridge.WritableNativeArray;
 import com.facebook.react.bridge.WritableNativeMap;
 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;
@@ -905,6 +907,17 @@ public void fetchNativeDeviceContexts(Promise promise) {
     }
 
     final @Nullable IScope currentScope = InternalSentrySdk.getCurrentScope();
+    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 @NotNull Map<String, Object> serialized =
         InternalSentrySdk.serializeScope(context, (SentryAndroidOptions) options, currentScope);
     final @Nullable Object deviceContext = RNSentryMapConverter.convertToWritable(serialized);

packages/core/ios/RNSentry.mm

@@ -438,6 +438,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);
 }
 

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"]];

packages/core/src/js/integrations/devicecontext.ts

@@ -1,4 +1,5 @@
 /* eslint-disable complexity */
+import { getClient } from '@sentry/core';
 import type { Event, Integration } from '@sentry/types';
 import { logger, severityLevelFromString } from '@sentry/utils';
 import { AppState } from 'react-native';
@@ -83,7 +84,8 @@ async function processEvent(event: Event): Promise<Event> {
     ? native['breadcrumbs'].map(breadcrumbFromObject)
     : undefined;
   if (nativeBreadcrumbs) {
-    event.breadcrumbs = nativeBreadcrumbs;
+    const maxBreadcrumbs = getClient()?.getOptions().maxBreadcrumbs ?? 100; // Default is 100.
+    event.breadcrumbs = nativeBreadcrumbs.concat(event.breadcrumbs || []).slice(0, maxBreadcrumbs);
   }
 
   return event;

packages/core/test/integrations/devicecontext.test.ts

@@ -1,3 +1,4 @@
+import { getClient } from '@sentry/core';
 import type { Client, Event, EventHint, SeverityLevel } from '@sentry/types';
 
 import { deviceContextIntegration } from '../../src/js/integrations/devicecontext';
@@ -12,6 +13,13 @@ jest.mock('react-native', () => ({
   NativeModules: {},
   Platform: {},
 }));
+jest.mock('@sentry/core', () => ({
+  getClient: jest.fn().mockReturnValue({
+    getOptions: jest.fn().mockReturnValue({
+      maxBreadcrumbs: 100, // Default value
+    }),
+  }),
+}));
 
 describe('Device Context Integration', () => {
   it('add native user', async () => {
@@ -158,13 +166,55 @@ describe('Device Context Integration', () => {
     ).expectEvent.toStrictEqualMockEvent();
   });
 
-  it('use only native breadcrumbs', async () => {
+  it('merge native and event breadcrumbs', async () => {
+    const { processedEvent } = await processEventWith({
+      nativeContexts: { breadcrumbs: [{ message: 'native-breadcrumb-1' }, { message: 'native-breadcrumb-2' }] },
+      mockEvent: { breadcrumbs: [{ message: 'event-breadcrumb-1' }, { message: 'event-breadcrumb-2' }] },
+    });
+    expect(processedEvent).toStrictEqual({
+      breadcrumbs: [
+        { message: 'native-breadcrumb-1' },
+        { message: 'native-breadcrumb-2' },
+        { message: 'event-breadcrumb-1' },
+        { message: 'event-breadcrumb-2' },
+      ],
+    });
+  });
+
+  it('use only native breadcrumb if the maxBreadcrumbs limit does not leave space for event breadcrumbs', async () => {
+    getClient().getOptions().maxBreadcrumbs = 1;
+    const { processedEvent } = await processEventWith({
+      nativeContexts: { breadcrumbs: [{ message: 'native-breadcrumb' }] },
+      mockEvent: { breadcrumbs: [{ message: 'event-breadcrumb' }] },
+    });
+    expect(processedEvent).toStrictEqual({
+      breadcrumbs: [{ message: 'native-breadcrumb' }],
+    });
+  });
+
+  it('use native breadcrumbs and pick only the event breadcrumbs that fit the maxBreadcrumbs limit', async () => {
+    getClient().getOptions().maxBreadcrumbs = 3;
+    const { processedEvent } = await processEventWith({
+      nativeContexts: { breadcrumbs: [{ message: 'native-breadcrumb-1' }, { message: 'native-breadcrumb-2' }] },
+      mockEvent: { breadcrumbs: [{ message: 'event-breadcrumb-1' }, { message: 'event-breadcrumb-2' }] },
+    });
+    expect(processedEvent).toStrictEqual({
+      breadcrumbs: [
+        { message: 'native-breadcrumb-1' },
+        { message: 'native-breadcrumb-2' },
+        { message: 'event-breadcrumb-1' },
+      ],
+    });
+  });
+
+  it('do not use any breadcrumbs if the maxBreadcrumbs limit is set to zero', async () => {
+    getClient().getOptions().maxBreadcrumbs = 0;
     const { processedEvent } = await processEventWith({
-      nativeContexts: { breadcrumbs: [{ message: 'duplicate-breadcrumb' }, { message: 'native-breadcrumb' }] },
-      mockEvent: { breadcrumbs: [{ message: 'duplicate-breadcrumb' }, { message: 'event-breadcrumb' }] },
+      nativeContexts: { breadcrumbs: [{ message: 'native-breadcrumb' }] },
+      mockEvent: { breadcrumbs: [{ message: 'event-breadcrumb' }] },
     });
     expect(processedEvent).toStrictEqual({
-      breadcrumbs: [{ message: 'duplicate-breadcrumb' }, { message: 'native-breadcrumb' }],
+      breadcrumbs: [],
     });
   });
 

samples/react-native/src/Screens/ErrorsScreen.tsx

@@ -88,6 +88,13 @@ const ErrorsScreen = (_props: Props) => {
             Sentry.captureException(error);
           }}
         />
+        <Button
+          title="Capture exception with breadcrumb"
+          onPress={() => {
+            Sentry.captureException(new Error('Captured exception with breadcrumb'),
+              context => context.addBreadcrumb({ message: 'error with breadcrumb' }));
+          }}
+        />
         <Button
           title="Uncaught Thrown Error"
           onPress={() => {