fix: setContext correctly processes non-plain JS object

CHANGELOG.md

@@ -5,6 +5,9 @@
 ### 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
 

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);
  });
  }

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];
+ }
  }];
 }
 

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;
@@ -395,10 +397,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**');
  },
 
  /**
@@ -435,19 +452,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**' });
+ }
  },
 
  /**

test/wrapper.test.ts

@@ -623,4 +623,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();
+ });
+ });
 });