diff --git a/CHANGELOG.md b/CHANGELOG.md index 59c865e689..6ecdd4f3f0 100644 --- 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. +## 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/packages/core/RNSentryAndroidTester/app/src/androidTest/java/io/sentry/react/RNSentryModuleImplTest.kt b/packages/core/RNSentryAndroidTester/app/src/androidTest/java/io/sentry/react/RNSentryModuleImplTest.kt new file mode 100644 index 0000000000..d314420758 --- /dev/null +++ b/packages/core/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/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/rnsentryandroidtester/RNSentryBreadcrumbTest.kt b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/rnsentryandroidtester/RNSentryBreadcrumbTest.kt index a41c3b964d..e2eceab44a 100644 --- a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/rnsentryandroidtester/RNSentryBreadcrumbTest.kt +++ b/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() diff --git a/packages/core/RNSentryCocoaTester/RNSentryCocoaTester.xcodeproj/project.pbxproj b/packages/core/RNSentryCocoaTester/RNSentryCocoaTester.xcodeproj/project.pbxproj index 82d6c51fd5..59c10b17ad 100644 --- 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 index d58931b295..eb314766e8 100644 --- 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 index cc5bf0fb2b..45885adc9c 100644 --- 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 index 8191108ad2..5ae47a0ae6 100644 --- 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 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 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 index e58d5e7c43..67803b8491 100644 --- a/packages/core/ios/RNSentry.mm +++ b/packages/core/ios/RNSentry.mm @@ -442,6 +442,16 @@ - (NSDictionary *)fetchNativeStackFramesBy:(NSArray *)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 index c849b5a5f9..db9ffce2d6 100644 --- 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 index 942ca5210d..9948557c8a 100644 --- 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 { +async function processEvent(event: Event, _hint: EventHint, client: Client): Promise { let native: NativeDeviceContextsResponse | null = null; try { native = await NATIVE.fetchNativeDeviceContexts(); @@ -83,7 +83,11 @@ async function processEvent(event: Event): Promise { ? 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; diff --git a/packages/core/test/integrations/devicecontext.test.ts b/packages/core/test/integrations/devicecontext.test.ts index ff46e5f3c1..d8ad810d8e 100644 --- a/packages/core/test/integrations/devicecontext.test.ts +++ b/packages/core/test/integrations/devicecontext.test.ts @@ -6,6 +6,12 @@ import { NATIVE } from '../../src/js/wrapper'; let mockCurrentAppState: string = 'unknown'; +const mockClient = { + getOptions: jest.fn().mockReturnValue({ + maxBreadcrumbs: undefined, // Default 100 + }), +} as unknown as Client; + jest.mock('../../src/js/wrapper'); jest.mock('react-native', () => ({ AppState: new Proxy({}, { get: () => mockCurrentAppState }), @@ -158,13 +164,81 @@ describe('Device Context Integration', () => { ).expectEvent.toStrictEqualMockEvent(); }); - it('use only native breadcrumbs', async () => { - const { processedEvent } = await processEventWith({ - nativeContexts: { breadcrumbs: [{ message: 'duplicate-breadcrumb' }, { message: 'native-breadcrumb' }] }, - mockEvent: { breadcrumbs: [{ message: 'duplicate-breadcrumb' }, { message: 'event-breadcrumb' }] }, + 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' }] }, + }, + mockClient, + ); + expect(processedEvent).toStrictEqual({ + breadcrumbs: [ + { message: 'native-breadcrumb-1' }, + { message: 'native-breadcrumb-2' }, + { message: 'event-breadcrumb-1' }, + { message: 'event-breadcrumb-2' }, + ], + }); + }); + + it('respect breadcrumb order when merging', async () => { + const { processedEvent } = await processEventWith( + { + nativeContexts: { + breadcrumbs: [ + { message: 'native-breadcrumb-3', timestamp: 'Thursday, November 7, 2024 3:24:59 PM GMT+02:00' }, // 1730985899 + { message: 'native-breadcrumb-1', timestamp: 'Thursday, November 7, 2024 3:24:57 PM GMT+02:00' }, // 1730985897 + ], + }, + mockEvent: { + breadcrumbs: [ + { message: 'event-breadcrumb-4', timestamp: 1730985999 }, + { message: 'event-breadcrumb-2', timestamp: 1730985898 }, + ], + }, + }, + mockClient, + ); + expect(processedEvent).toStrictEqual({ + breadcrumbs: [ + { message: 'native-breadcrumb-1', timestamp: 1730985897 }, + { message: 'event-breadcrumb-2', timestamp: 1730985898 }, + { message: 'native-breadcrumb-3', timestamp: 1730985899 }, + { message: 'event-breadcrumb-4', timestamp: 1730985999 }, + ], }); + }); + + it('keep the last maxBreadcrumbs when merging', async () => { + const mockClient = { + getOptions: jest.fn().mockReturnValue({ + maxBreadcrumbs: 3, + }), + } as unknown as Client; + const { processedEvent } = await processEventWith( + { + nativeContexts: { + breadcrumbs: [ + { message: 'native-breadcrumb-3', timestamp: 'Thursday, November 7, 2024 3:24:59 PM GMT+02:00' }, // 1730985899 + { message: 'native-breadcrumb-1', timestamp: 'Thursday, November 7, 2024 3:24:57 PM GMT+02:00' }, // 1730985897 + ], + }, + mockEvent: { + breadcrumbs: [ + { message: 'event-breadcrumb-4', timestamp: 1730985999 }, + { message: 'event-breadcrumb-2', timestamp: 1730985898 }, + ], + }, + }, + mockClient, + ); expect(processedEvent).toStrictEqual({ - breadcrumbs: [{ message: 'duplicate-breadcrumb' }, { message: 'native-breadcrumb' }], + breadcrumbs: [ + { message: 'event-breadcrumb-2', timestamp: 1730985898 }, + { message: 'native-breadcrumb-3', timestamp: 1730985899 }, + { message: 'event-breadcrumb-4', timestamp: 1730985999 }, + ], }); }); @@ -213,13 +287,16 @@ describe('Device Context Integration', () => { }); }); -async function processEventWith({ - nativeContexts, - mockEvent, -}: { - nativeContexts: Record; - mockEvent?: Event; -}): Promise<{ +async function processEventWith( + { + nativeContexts, + mockEvent, + }: { + nativeContexts: Record; + mockEvent?: Event; + }, + client: Client = {} as undefined as Client, +): Promise<{ processedEvent: Event | null; expectEvent: { toStrictEqualToNativeContexts: () => void; @@ -231,7 +308,7 @@ async function processEventWith({ ); const originalNativeContexts = { ...nativeContexts }; const originalMockEvent = { ...mockEvent }; - const processedEvent = await processEvent(mockEvent ?? {}); + const processedEvent = await processEvent(mockEvent ?? {}, client); return { processedEvent, expectEvent: { @@ -241,6 +318,6 @@ async function processEventWith({ }; } -function processEvent(mockedEvent: Event): Event | null | PromiseLike { - return deviceContextIntegration().processEvent!(mockedEvent, {} as EventHint, {} as Client); +function processEvent(mockedEvent: Event, client: Client): Event | null | PromiseLike { + return deviceContextIntegration().processEvent!(mockedEvent, {} as EventHint, client); } diff --git a/samples/react-native/src/Screens/ErrorsScreen.tsx b/samples/react-native/src/Screens/ErrorsScreen.tsx index 6430440295..2890f8d19d 100644 --- a/samples/react-native/src/Screens/ErrorsScreen.tsx +++ b/samples/react-native/src/Screens/ErrorsScreen.tsx @@ -88,6 +88,13 @@ const ErrorsScreen = (_props: Props) => { Sentry.captureException(error); }} /> +