Merge remote-tracking branch 'origin/main' into v6

Author: krystofwoldrich

CHANGELOG.md

@@ -4,6 +4,15 @@
 
 ### Fixes
 
+- TimetoTisplay correctly warns about not supporting the new React Native architecture ([#4160](https://github.com/getsentry/sentry-react-native/pull/4160))
+- Native Wrapper method `setContext` ensures only values convertible to NativeMap are passed ([#4168](https://github.com/getsentry/sentry-react-native/pull/4168))
+- Native Wrapper method `setExtra` ensures only stringified values are passed ([#4168](https://github.com/getsentry/sentry-react-native/pull/4168))
+- `setContext('key', null)` removes the key value also from platform context ([#4168](https://github.com/getsentry/sentry-react-native/pull/4168))
+
+## 5.34.0
+
+### Fixes
+
 - Handles error with string cause ([#4163](https://github.com/getsentry/sentry-react-native/pull/4163))
 - Use `appLaunchedInForeground` to determine invalid app start data on Android ([#4146](https://github.com/getsentry/sentry-react-native/pull/4146))
 

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

@@ -639,18 +639,29 @@ public void clearBreadcrumbs() {
     }
 
     public void setExtra(String key, String extra) {
+        if (key == null || extra == null) {
+            logger.log(SentryLevel.ERROR, "RNSentry.setExtra called with null key or value, can't change extra.");
+            return;
+        }
+
         Sentry.configureScope(scope -> {
             scope.setExtra(key, extra);
         });
     }
 
     public void setContext(final String key, final ReadableMap context) {
-        if (key == null || context == null) {
+        if (key == null) {
+            logger.log(SentryLevel.ERROR, "RNSentry.setContext called with null key, can't change context.");
             return;
         }
+
         Sentry.configureScope(scope -> {
-            final HashMap<String, Object> contextHashMap = context.toHashMap();
+            if (context == null) {
+                scope.removeContexts(key);
+                return;
+            }
 
+            final HashMap<String, Object> contextHashMap = context.toHashMap();
             scope.setContexts(key, contextHashMap);
         });
     }

packages/core/ios/RNSentry.mm

@@ -587,8 +587,16 @@ - (NSDictionary*) fetchNativeStackFramesBy: (NSArray<NSNumber*>*)instructionsAdd
                   context:(NSDictionary *)context
 )
 {
+    if (key == nil) {
+        return;
+    }
+
     [SentrySDK configureScope:^(SentryScope * _Nonnull scope) {
-        [scope setContextValue:context forKey:key];
+        if (context == nil) {
+            [scope removeContextForKey:key];
+        } else {
+            [scope setContextValue:context forKey:key];
+        }
     }];
 }
 

packages/core/src/js/tracing/timetodisplay.tsx

@@ -3,6 +3,7 @@ import type { Span,StartSpanOptions  } from '@sentry/types';
 import { fill, logger } from '@sentry/utils';
 import * as React from 'react';
 
+import { isTurboModuleEnabled } from '../utils/environment';
 import { SPAN_ORIGIN_AUTO_UI_TIME_TO_DISPLAY, SPAN_ORIGIN_MANUAL_UI_TIME_TO_DISPLAY } from './origin';
 import { getRNSentryOnDrawReporter, nativeComponentExists } from './timetodisplaynative';
 import type {RNSentryOnDrawNextFrameEvent } from './timetodisplaynative.types';
@@ -60,12 +61,14 @@ function TimeToDisplay(props: {
   fullDisplay?: boolean;
 }): React.ReactElement {
   const RNSentryOnDrawReporter = getRNSentryOnDrawReporter();
+  const isNewArchitecture = isTurboModuleEnabled();
 
-  if (__DEV__ && !nativeComponentMissingLogged && !nativeComponentExists) {
+  if (__DEV__ && (isNewArchitecture || (!nativeComponentExists && !nativeComponentMissingLogged))){
     nativeComponentMissingLogged = true;
     // Using setTimeout with a delay of 0 milliseconds to defer execution and avoid printing the React stack trace.
     setTimeout(() => {
-      logger.warn('TimeToInitialDisplay and TimeToFullDisplay are not supported on the web, Expo Go and New Architecture. Run native build or report an issue at https://github.com/getsentry/sentry-react-native');
+      logger.warn(
+          'TimeToInitialDisplay and TimeToFullDisplay are not supported on the web, Expo Go and New Architecture. Run native build or report an issue at https://github.com/getsentry/sentry-react-native');
     }, 0);
   }
 

packages/core/src/js/wrapper.ts

@@ -28,6 +28,7 @@ import type { NativeAndroidProfileEvent, NativeProfileEvent } from './profiling/
 import type { MobileReplayOptions } from './replay/mobilereplay';
 import type { RequiredKeysUser } from './user';
 import { isTurboModuleEnabled } from './utils/environment';
+import { convertToNormalizedObject } from './utils/normalize';
 import { ReactNativeLibraries } from './utils/rnlibraries';
 import { base64StringFromByteArray, utf8ToBytes } from './vendor';
 
@@ -84,7 +85,8 @@ interface SentryNativeWrapper {
   enableNativeFramesTracking(): void;
 
   addBreadcrumb(breadcrumb: Breadcrumb): void;
-  setContext(key: string, context: { [key: string]: unknown } | null): void;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  setContext(key: string, context: { [key: string]: any } | null): void;
   clearBreadcrumbs(): void;
   setExtra(key: string, extra: unknown): void;
   setUser(user: User | null): void;
@@ -396,10 +398,25 @@ export const NATIVE: SentryNativeWrapper = {
       throw this._NativeClientError;
     }
 
-    // we stringify the extra as native only takes in strings.
-    const stringifiedExtra = typeof extra === 'string' ? extra : JSON.stringify(extra);
+    if (typeof extra === 'string') {
+      return RNSentry.setExtra(key, extra);
+    }
+    if (typeof extra === 'undefined') {
+      return RNSentry.setExtra(key, 'undefined');
+    }
+
+    let stringifiedExtra: string | undefined;
+    try {
+      const normalizedExtra = normalize(extra);
+      stringifiedExtra = JSON.stringify(normalizedExtra);
+    } catch (e) {
+      logger.error('Extra for key ${key} not passed to native SDK, because it contains non-stringifiable values', e);
+    }
 
-    RNSentry.setExtra(key, stringifiedExtra);
+    if (typeof stringifiedExtra === 'string') {
+      return RNSentry.setExtra(key, stringifiedExtra);
+    }
+    return RNSentry.setExtra(key, '**non-stringifiable**');
   },
 
   /**
@@ -436,19 +453,35 @@ export const NATIVE: SentryNativeWrapper = {
   },
 
   /**
-   * Sets context on the native scope. Not implemented in Android yet.
+   * Sets context on the native scope.
    * @param key string
    * @param context key-value map
    */
-  setContext(key: string, context: { [key: string]: unknown } | null): void {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  setContext(key: string, context: { [key: string]: any } | null): void {
     if (!this.enableNative) {
       return;
     }
     if (!this._isModuleLoaded(RNSentry)) {
       throw this._NativeClientError;
     }
 
-    RNSentry.setContext(key, context !== null ? normalize(context) : null);
+    if (context === null) {
+      return RNSentry.setContext(key, null);
+    }
+
+    let normalizedContext: Record<string, unknown> | undefined;
+    try {
+      normalizedContext = convertToNormalizedObject(context);
+    } catch (e) {
+      logger.error('Context for key ${key} not passed to native SDK, because it contains non-serializable values', e);
+    }
+
+    if (normalizedContext) {
+      RNSentry.setContext(key, normalizedContext);
+    } else {
+      RNSentry.setContext(key, { error: '**non-serializable**' });
+    }
   },
 
   /**

packages/core/test/tracing/timetodisplay.test.tsx

@@ -1,6 +1,14 @@
+import { logger } from '@sentry/utils';
+jest.spyOn(logger, 'warn');
+
 import * as mockedtimetodisplaynative from './mockedtimetodisplaynative';
 jest.mock('../../src/js/tracing/timetodisplaynative', () => mockedtimetodisplaynative);
 
+import { isTurboModuleEnabled } from '../../src/js/utils/environment';
+jest.mock('../../src/js/utils/environment', () => ({
+  isTurboModuleEnabled: jest.fn().mockReturnValue(false),
+}));
+
 import { getActiveSpan, getCurrentScope, getGlobalScope, getIsolationScope, getSpanDescendants, setCurrentClient, spanToJSON, startSpanManual} from '@sentry/core';
 import type { Event, Measurements, Span, SpanJSON} from '@sentry/types';
 import * as React from "react";
@@ -371,6 +379,26 @@ describe('TimeToDisplay', () => {
     expect(spanToJSON(initialDisplaySpan!).timestamp).toEqual(initialDisplayEndTimestampMs / 1_000);
     expect(spanToJSON(fullDisplaySpan!).timestamp).toEqual(fullDisplayEndTimestampMs / 1_000);
   });
+
+  test('should not log a warning if native component exists and not in new architecture', async () => {
+
+    (isTurboModuleEnabled as jest.Mock).mockReturnValue(false);
+
+    TestRenderer.create(<TimeToInitialDisplay record={true} />);
+    await jest.runOnlyPendingTimersAsync(); // Flush setTimeout.
+
+    expect(logger.warn).not.toHaveBeenCalled();
+  });
+
+  test('should log a warning if in new architecture', async () => {
+
+    (isTurboModuleEnabled as jest.Mock).mockReturnValue(true);
+    TestRenderer.create(<TimeToInitialDisplay record={true} />);
+    await jest.runOnlyPendingTimersAsync(); // Flush setTimeout.
+
+    expect(logger.warn).toHaveBeenCalledWith(
+      'TimeToInitialDisplay and TimeToFullDisplay are not supported on the web, Expo Go and New Architecture. Run native build or report an issue at https://github.com/getsentry/sentry-react-native');
+  });
 });
 
 function getInitialDisplaySpan(span?: Span) {

packages/core/test/wrapper.test.ts

@@ -629,4 +629,122 @@ describe('Tests Native Wrapper', () => {
       expect(NATIVE.stopProfiling()).toBe(null);
     });
   });
+
+  describe('setExtra', () => {
+    test('passes string value to native method', () => {
+      NATIVE.setExtra('key', 'string value');
+      expect(RNSentry.setExtra).toHaveBeenCalledWith('key', 'string value');
+      expect(RNSentry.setExtra).toHaveBeenCalledOnce();
+    });
+
+    test('stringifies number value before passing to native method', () => {
+      NATIVE.setExtra('key', 42);
+      expect(RNSentry.setExtra).toHaveBeenCalledWith('key', '42');
+      expect(RNSentry.setExtra).toHaveBeenCalledOnce();
+    });
+
+    test('stringifies boolean value before passing to native method', () => {
+      NATIVE.setExtra('key', true);
+      expect(RNSentry.setExtra).toHaveBeenCalledWith('key', 'true');
+      expect(RNSentry.setExtra).toHaveBeenCalledOnce();
+    });
+
+    test('stringifies object value before passing to native method', () => {
+      const obj = { foo: 'bar', baz: 123 };
+      NATIVE.setExtra('key', obj);
+      expect(RNSentry.setExtra).toHaveBeenCalledWith('key', JSON.stringify(obj));
+      expect(RNSentry.setExtra).toHaveBeenCalledOnce();
+    });
+
+    test('stringifies array value before passing to native method', () => {
+      const arr = [1, 'two', { three: 3 }];
+      NATIVE.setExtra('key', arr);
+      expect(RNSentry.setExtra).toHaveBeenCalledWith('key', JSON.stringify(arr));
+      expect(RNSentry.setExtra).toHaveBeenCalledOnce();
+    });
+
+    test('handles null value by stringifying', () => {
+      NATIVE.setExtra('key', null);
+      expect(RNSentry.setExtra).toHaveBeenCalledWith('key', 'null');
+      expect(RNSentry.setExtra).toHaveBeenCalledOnce();
+    });
+
+    test('handles undefined value by stringifying', () => {
+      NATIVE.setExtra('key', undefined);
+      expect(RNSentry.setExtra).toHaveBeenCalledWith('key', 'undefined');
+      expect(RNSentry.setExtra).toHaveBeenCalledOnce();
+    });
+
+    test('handles non-serializable value by stringifying', () => {
+      const circular: { self?: unknown } = {};
+      circular.self = circular;
+      NATIVE.setExtra('key', circular);
+      expect(RNSentry.setExtra).toHaveBeenCalledWith('key', '{"self":"[Circular ~]"}');
+      expect(RNSentry.setExtra).toHaveBeenCalledOnce();
+    });
+  });
+
+  describe('setContext', () => {
+    test('passes plain JS object to native method', () => {
+      const context = { foo: 'bar', baz: 123 };
+      NATIVE.setContext('key', context);
+      expect(RNSentry.setContext).toHaveBeenCalledWith('key', context);
+      expect(RNSentry.setContext).toHaveBeenCalledOnce();
+    });
+
+    test('converts non-plain JS object to plain object before passing to native method', () => {
+      class TestClass {
+        prop = 'value';
+      }
+      const context = new TestClass();
+      NATIVE.setContext('key', context);
+      expect(RNSentry.setContext).toHaveBeenCalledWith('key', { prop: 'value' });
+      expect(RNSentry.setContext).toHaveBeenCalledOnce();
+    });
+
+    test('converts array to object with "value" key before passing to native method', () => {
+      const context = [1, 'two', { three: 3 }];
+      NATIVE.setContext('key', context);
+      expect(RNSentry.setContext).toHaveBeenCalledWith('key', { value: [1, 'two', { three: 3 }] });
+      expect(RNSentry.setContext).toHaveBeenCalledOnce();
+    });
+
+    test('converts string primitive to object with "value" key before passing to native method', () => {
+      NATIVE.setContext('key', 'string value' as unknown as object);
+      expect(RNSentry.setContext).toHaveBeenCalledWith('key', { value: 'string value' });
+      expect(RNSentry.setContext).toHaveBeenCalledOnce();
+    });
+
+    test('converts number primitive to object with "value" key before passing to native method', () => {
+      NATIVE.setContext('key', 42 as unknown as object);
+      expect(RNSentry.setContext).toHaveBeenCalledWith('key', { value: 42 });
+      expect(RNSentry.setContext).toHaveBeenCalledOnce();
+    });
+
+    test('converts boolean primitive to object with "value" key before passing to native method', () => {
+      NATIVE.setContext('key', true as unknown as object);
+      expect(RNSentry.setContext).toHaveBeenCalledWith('key', { value: true });
+      expect(RNSentry.setContext).toHaveBeenCalledOnce();
+    });
+
+    test('handles null value by passing null to native method', () => {
+      NATIVE.setContext('key', null);
+      expect(RNSentry.setContext).toHaveBeenCalledWith('key', null);
+      expect(RNSentry.setContext).toHaveBeenCalledOnce();
+    });
+
+    test('handles undefined value by converting to object with "value" key', () => {
+      NATIVE.setContext('key', undefined as unknown as object);
+      expect(RNSentry.setContext).toHaveBeenCalledWith('key', { value: undefined });
+      expect(RNSentry.setContext).toHaveBeenCalledOnce();
+    });
+
+    test('handles non-serializable value by converting to normalized object', () => {
+      const circular: { self?: unknown } = {};
+      circular.self = circular;
+      NATIVE.setContext('key', circular);
+      expect(RNSentry.setContext).toHaveBeenCalledWith('key', { self: '[Circular ~]' });
+      expect(RNSentry.setContext).toHaveBeenCalledOnce();
+    });
+  });
 });

samples/expo/app.json

@@ -58,4 +58,4 @@
       ]
     ]
   }
-}
+}

samples/react-native/src/App.tsx

@@ -33,6 +33,7 @@ import Ionicons from 'react-native-vector-icons/Ionicons';
 import PlaygroundScreen from './Screens/PlaygroundScreen';
 import { logWithoutTracing } from './utils';
 import { ErrorEvent } from '@sentry/types';
+import HeavyNavigationScreen from './Screens/HeavyNavigationScreen';
 
 LogBox.ignoreAllLogs();
 const isMobileOs = Platform.OS === 'android' || Platform.OS === 'ios';
@@ -161,6 +162,10 @@ const TabTwoStack = Sentry.withProfiler(
               name="ManualTracker"
               component={ManualTrackerScreen}
             />
+            <Stack.Screen
+              name="HeavyNavigation"
+              component={HeavyNavigationScreen}
+            />
             <Stack.Screen
               name="PerformanceTiming"
               component={PerformanceTimingScreen}