diff --git a/CHANGELOG.md b/CHANGELOG.md index 031a90f771..2f2e7779d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,18 @@ - Automatically detect Release name and version for Expo Web ([#4967](https://github.com/getsentry/sentry-react-native/pull/4967)) +### Breaking changes + +- Tags formatting logic updated ([#4965](https://github.com/getsentry/sentry-react-native/pull/4965)) +Here are the altered/unaltered types, make sure to update your UI filters and alerts. + + Unaltered: string, null, number, and undefined values remain unchanged. + + Altered: Boolean values are now capitalized: true -> True, false -> False. + ### Fixes +- tags with symbol are now logged ([#4965](https://github.com/getsentry/sentry-react-native/pull/4965)) - ignoreError now filters Native errors ([#4948](https://github.com/getsentry/sentry-react-native/pull/4948)) You can use strings to filter errors or RegEx for filtering with a pattern. diff --git a/packages/core/src/js/integrations/default.ts b/packages/core/src/js/integrations/default.ts index b5b69d3f21..2967440e0b 100644 --- a/packages/core/src/js/integrations/default.ts +++ b/packages/core/src/js/integrations/default.ts @@ -27,6 +27,7 @@ import { modulesLoaderIntegration, nativeLinkedErrorsIntegration, nativeReleaseIntegration, + primitiveTagIntegration, reactNativeErrorHandlersIntegration, reactNativeInfoIntegration, screenshotIntegration, @@ -153,5 +154,7 @@ export function getDefaultIntegrations(options: ReactNativeClientOptions): Integ integrations.push(debugSymbolicatorIntegration()); } + integrations.push(primitiveTagIntegration()); + return integrations; } diff --git a/packages/core/src/js/integrations/exports.ts b/packages/core/src/js/integrations/exports.ts index a1b54de361..6a7ad0feef 100644 --- a/packages/core/src/js/integrations/exports.ts +++ b/packages/core/src/js/integrations/exports.ts @@ -23,6 +23,7 @@ export { createReactNativeRewriteFrames } from './rewriteframes'; export { appRegistryIntegration } from './appRegistry'; export { timeToDisplayIntegration } from '../tracing/integrations/timeToDisplayIntegration'; export { breadcrumbsIntegration } from './breadcrumbs'; +export { primitiveTagIntegration } from './primitiveTagIntegration'; export { browserApiErrorsIntegration, diff --git a/packages/core/src/js/integrations/primitiveTagIntegration.ts b/packages/core/src/js/integrations/primitiveTagIntegration.ts new file mode 100644 index 0000000000..7a26d461de --- /dev/null +++ b/packages/core/src/js/integrations/primitiveTagIntegration.ts @@ -0,0 +1,37 @@ +import type { Integration, Primitive } from '@sentry/core'; +import { PrimitiveToString } from '../utils/primitiveConverter'; +import { NATIVE } from '../wrapper'; + +export const INTEGRATION_NAME = 'PrimitiveTagIntegration'; + +/** + * Format tags set with Primitive values with a standard string format. + * + * When this Integration is enable, the following types will have the following behaviour: + * + * Unaltered: string, null, number, and undefined values remain unchanged. + * + * Altered: + * Boolean values are now capitalized: true -> True, false -> False. + * Symbols are stringified. + * + */ +export const primitiveTagIntegration = (): Integration => { + return { + name: INTEGRATION_NAME, + setup(client) { + client.on('beforeSendEvent', event => { + if (event.tags) { + Object.keys(event.tags).forEach(key => { + event.tags![key] = PrimitiveToString(event.tags![key]); + }); + } + }); + }, + afterAllSetup() { + if (NATIVE.enableNative) { + NATIVE._setPrimitiveProcessor((value: Primitive) => PrimitiveToString(value)); + } + }, + }; +}; diff --git a/packages/core/src/js/integrations/reactnativeinfo.ts b/packages/core/src/js/integrations/reactnativeinfo.ts index a66ba9da2f..f45139f232 100644 --- a/packages/core/src/js/integrations/reactnativeinfo.ts +++ b/packages/core/src/js/integrations/reactnativeinfo.ts @@ -60,7 +60,7 @@ function processEvent(event: Event, hint: EventHint): Event { if (reactNativeContext.js_engine === 'hermes') { event.tags = { - hermes: 'true', + hermes: true, ...event.tags, }; } diff --git a/packages/core/src/js/scopeSync.ts b/packages/core/src/js/scopeSync.ts index 78ff027c20..5c6537d89e 100644 --- a/packages/core/src/js/scopeSync.ts +++ b/packages/core/src/js/scopeSync.ts @@ -26,14 +26,14 @@ export function enableSyncToNative(scope: Scope): void { }); fillTyped(scope, 'setTag', original => (key, value): Scope => { - NATIVE.setTag(key, value as string); + NATIVE.setTag(key, NATIVE.primitiveProcessor(value)); return original.call(scope, key, value); }); fillTyped(scope, 'setTags', original => (tags): Scope => { // As native only has setTag, we just loop through each tag key. Object.keys(tags).forEach(key => { - NATIVE.setTag(key, tags[key] as string); + NATIVE.setTag(key, NATIVE.primitiveProcessor(tags[key])); }); return original.call(scope, tags); }); diff --git a/packages/core/src/js/utils/primitiveConverter.ts b/packages/core/src/js/utils/primitiveConverter.ts new file mode 100644 index 0000000000..c57073c73f --- /dev/null +++ b/packages/core/src/js/utils/primitiveConverter.ts @@ -0,0 +1,26 @@ +import type { Primitive } from '@sentry/core'; + +/** + * Converts primitive to string. + */ +export function PrimitiveToString(primitive: Primitive): string | undefined { + if (primitive === null) { + return ''; + } + + switch (typeof primitive) { + case 'string': + return primitive; + case 'boolean': + return primitive == true ? 'True' : 'False'; + case 'number': + case 'bigint': + return `${primitive}`; + case 'undefined': + return undefined; + case 'symbol': + return primitive.toString(); + default: + return primitive as string; + } +} diff --git a/packages/core/src/js/wrapper.ts b/packages/core/src/js/wrapper.ts index f4cc5c9c15..77c8bdeb2c 100644 --- a/packages/core/src/js/wrapper.ts +++ b/packages/core/src/js/wrapper.ts @@ -6,6 +6,7 @@ import type { EnvelopeItem, Event, Package, + Primitive, SeverityLevel, User, } from '@sentry/core'; @@ -66,6 +67,7 @@ interface SentryNativeWrapper { _NativeClientError: Error; _DisabledNativeError: Error; + _setPrimitiveProcessor: (processor: (value: Primitive) => void) => void; _processItem(envelopeItem: EnvelopeItem): EnvelopeItem; _processLevels(event: Event): Event; _processLevel(level: SeverityLevel): SeverityLevel; @@ -95,7 +97,7 @@ interface SentryNativeWrapper { clearBreadcrumbs(): void; setExtra(key: string, extra: unknown): void; setUser(user: User | null): void; - setTag(key: string, value: string): void; + setTag(key: string, value?: string): void; nativeCrash(): void; @@ -129,6 +131,8 @@ interface SentryNativeWrapper { setActiveSpanId(spanId: string): void; encodeToBase64(data: Uint8Array): Promise; + + primitiveProcessor(value: Primitive): string; } const EOL = encodeUTF8('\n'); @@ -396,7 +400,7 @@ export const NATIVE: SentryNativeWrapper = { * @param key string * @param value string */ - setTag(key: string, value: string): void { + setTag(key: string, value?: string): void { if (!this.enableNative) { return; } @@ -777,6 +781,10 @@ export const NATIVE: SentryNativeWrapper = { } }, + primitiveProcessor: function (value: Primitive): string { + return value as string; + }, + /** * Gets the event from envelopeItem and applies the level filter to the selected event. * @param data An envelope item containing the event. @@ -822,7 +830,6 @@ export const NATIVE: SentryNativeWrapper = { * @param event * @returns Event with more widely supported Severity level strings */ - _processLevels(event: Event): Event { const processed: Event = { ...event, @@ -841,7 +848,6 @@ export const NATIVE: SentryNativeWrapper = { * @param level * @returns More widely supported Severity level strings */ - _processLevel(level: SeverityLevel): SeverityLevel { if (level == ('log' as SeverityLevel)) { return 'debug' as SeverityLevel; @@ -856,6 +862,10 @@ export const NATIVE: SentryNativeWrapper = { return !!module; }, + _setPrimitiveProcessor: function (processor: (value: Primitive) => any): void { + this.primitiveProcessor = processor; + }, + _DisabledNativeError: new SentryError('Native is disabled'), _NativeClientError: new SentryError("Native Client is not available, can't start on native."), diff --git a/packages/core/test/integrations/primitiveTagIntegration.test.ts b/packages/core/test/integrations/primitiveTagIntegration.test.ts new file mode 100644 index 0000000000..e77a9aacd8 --- /dev/null +++ b/packages/core/test/integrations/primitiveTagIntegration.test.ts @@ -0,0 +1,95 @@ +import type { Client } from '@sentry/core'; +import { primitiveTagIntegration } from '../../src/js/integrations/primitiveTagIntegration'; +import { NATIVE } from '../../src/js/wrapper'; +import { setupTestClient } from '../mocks/client'; + +describe('primitiveTagIntegration', () => { + beforeEach(() => { + jest.clearAllMocks(); + setupTestClient(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('integration setup', () => { + it('sets up beforeSendEvent handler', () => { + const integration = primitiveTagIntegration(); + const mockClient = { + on: jest.fn(), + } as any; + + integration.setup!(mockClient); + + expect(mockClient.on).toHaveBeenCalledWith('beforeSendEvent', expect.any(Function)); + }); + }); + + describe('beforeSendEvent processing', () => { + let beforeSendEventHandler: (event: any) => void; + + beforeEach(() => { + const integration = primitiveTagIntegration(); + const mockClient = { + on: jest.fn((eventName, handler) => { + if (eventName === 'beforeSendEvent') { + beforeSendEventHandler = handler; + } + }), + } as any; + + integration.setup!(mockClient); + }); + + it('handles events without tags', () => { + const event = { message: 'test' }; + + expect(() => beforeSendEventHandler(event)).not.toThrow(); + expect(event).toEqual({ message: 'test' }); + }); + + it('handles events with empty tags object', () => { + const event = { tags: {} }; + + expect(() => beforeSendEventHandler(event)).not.toThrow(); + expect(event.tags).toEqual({}); + }); + + it('handles events with null tags', () => { + const event = { tags: null }; + + expect(() => beforeSendEventHandler(event)).not.toThrow(); + expect(event.tags).toBeNull(); + }); + }); + + describe('integration with native processor', () => { + it('sets primitiveProcessor to PrimitiveToString function', () => { + const integration = primitiveTagIntegration(); + NATIVE.enableNative = true; + jest.spyOn(NATIVE, '_setPrimitiveProcessor'); + + integration.afterAllSetup!({ getOptions: () => ({}) } as Client); + + expect(NATIVE._setPrimitiveProcessor).toHaveBeenCalledWith(expect.any(Function)); + + // Verify the function passed is PrimitiveToString + const passedFunction = (NATIVE._setPrimitiveProcessor as jest.Mock).mock.calls[0][0]; + expect(passedFunction(true)).toBe('True'); + expect(passedFunction(false)).toBe('False'); + expect(passedFunction(null)).toBe(''); + expect(passedFunction(42)).toBe('42'); + }); + + it('does not set processor when native is disabled', () => { + const integration = primitiveTagIntegration(); + NATIVE.enableNative = false; + jest.spyOn(NATIVE, '_setPrimitiveProcessor'); + + integration.afterAllSetup!({ getOptions: () => ({}) } as Client); + + expect(NATIVE._setPrimitiveProcessor).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/core/test/integrations/reactnativeinfo.test.ts b/packages/core/test/integrations/reactnativeinfo.test.ts index 9c6e9b8fc0..2a6ab284be 100644 --- a/packages/core/test/integrations/reactnativeinfo.test.ts +++ b/packages/core/test/integrations/reactnativeinfo.test.ts @@ -60,7 +60,7 @@ describe('React Native Info', () => { }, }, tags: { - hermes: 'true', + hermes: true, }, }); }); @@ -71,7 +71,7 @@ describe('React Native Info', () => { const actualEvent = await executeIntegrationFor({}, {}); expectMocksToBeCalledOnce(); - expect(actualEvent?.tags?.hermes).toEqual('true'); + expect(actualEvent?.tags?.hermes).toBeTrue(); expect(actualEvent?.contexts?.react_native_context).toEqual( expect.objectContaining({ js_engine: 'hermes', diff --git a/packages/core/test/mockWrapper.ts b/packages/core/test/mockWrapper.ts index c26c384e26..83d15681b5 100644 --- a/packages/core/test/mockWrapper.ts +++ b/packages/core/test/mockWrapper.ts @@ -16,6 +16,7 @@ const NATIVE: MockInterface = { _processLevel: jest.fn(), _serializeObject: jest.fn(), _isModuleLoaded: jest.fn(), + _setPrimitiveProcessor: jest.fn(), isNativeAvailable: jest.fn(), @@ -63,6 +64,7 @@ const NATIVE: MockInterface = { popTimeToDisplayFor: jest.fn(), setActiveSpanId: jest.fn(), encodeToBase64: jest.fn(), + primitiveProcessor: jest.fn(), }; NATIVE.isNativeAvailable.mockReturnValue(true); @@ -89,7 +91,7 @@ NATIVE.getCurrentReplayId.mockReturnValue(null); NATIVE.crashedLastRun.mockResolvedValue(false); NATIVE.popTimeToDisplayFor.mockResolvedValue(null); NATIVE.getNewScreenTimeToDisplay.mockResolvedValue(null); - +NATIVE.primitiveProcessor.mockReturnValue(''); export const getRNSentryModule = jest.fn(); export { NATIVE }; diff --git a/packages/core/test/scopeSync.test.ts b/packages/core/test/scopeSync.test.ts index 7026bc901c..29c6efae29 100644 --- a/packages/core/test/scopeSync.test.ts +++ b/packages/core/test/scopeSync.test.ts @@ -135,7 +135,6 @@ describe('ScopeSync', () => { it('setUser', () => { expect(SentryCore.getIsolationScope().setUser).not.toBe(setUserScopeSpy); - const user = { id: '123' }; SentryCore.setUser(user); expect(NATIVE.setUser).toHaveBeenCalledExactlyOnceWith({ id: '123' }); @@ -143,6 +142,7 @@ describe('ScopeSync', () => { }); it('setTag', () => { + jest.spyOn(NATIVE, 'primitiveProcessor').mockImplementation((value: SentryCore.Primitive) => value as string); expect(SentryCore.getIsolationScope().setTag).not.toBe(setTagScopeSpy); SentryCore.setTag('key', 'value'); @@ -151,6 +151,7 @@ describe('ScopeSync', () => { }); it('setTags', () => { + jest.spyOn(NATIVE, 'primitiveProcessor').mockImplementation((value: SentryCore.Primitive) => value as string); expect(SentryCore.getIsolationScope().setTags).not.toBe(setTagsScopeSpy); SentryCore.setTags({ key: 'value', second: 'bar' }); diff --git a/packages/core/test/utils/PrimitiveConverter.test.ts b/packages/core/test/utils/PrimitiveConverter.test.ts new file mode 100644 index 0000000000..0583eccd73 --- /dev/null +++ b/packages/core/test/utils/PrimitiveConverter.test.ts @@ -0,0 +1,49 @@ +import { PrimitiveToString } from '../../src/js/utils/primitiveConverter'; + +describe('Primitive to String', () => { + it('Doesnt change strings', () => { + expect(PrimitiveToString('1234')).toBe('1234'); + expect(PrimitiveToString('1234,1')).toBe('1234,1'); + expect(PrimitiveToString('abc')).toBe('abc'); + }); + + it('Converts boolean to uppercase', () => { + expect(PrimitiveToString(false)).toBe('False'); + expect(PrimitiveToString(true)).toBe('True'); + }); + + it('Keeps undefined', () => { + expect(PrimitiveToString(undefined)).toBeUndefined(); + }); + + it('Converts null to empty', () => { + expect(PrimitiveToString(null)).toBe(''); + }); + + test.each([ + [0, '0'], + [1, '1'], + [12345, '12345'], + [Number.MIN_VALUE, `${Number.MIN_VALUE}`], + [Number.MAX_VALUE, `${Number.MAX_VALUE}`], + [Number.MIN_SAFE_INTEGER, `${Number.MIN_SAFE_INTEGER}`], + [Number.MAX_SAFE_INTEGER, `${Number.MAX_SAFE_INTEGER}`], + ])('Converts %p to "%s"', (input, expected) => { + expect(PrimitiveToString(input)).toBe(expected); + }); + + test.each([ + [BigInt('0'), '0'], + [BigInt('1'), '1'], + [BigInt('-1'), '-1'], + [BigInt('123456789012345678901234567890'), '123456789012345678901234567890'], + [BigInt('-98765432109876543210987654321'), '-98765432109876543210987654321'], + ])('converts bigint %p to "%s"', (input, expected) => { + expect(PrimitiveToString(input)).toBe(expected); + }); + + it('Symbol to String', () => { + const symbol = Symbol('a symbol'); + expect(PrimitiveToString(symbol)).toBe('Symbol(a symbol)'); + }); +}); diff --git a/packages/core/test/wrapper.test.ts b/packages/core/test/wrapper.test.ts index 72e29385fc..c5d071228a 100644 --- a/packages/core/test/wrapper.test.ts +++ b/packages/core/test/wrapper.test.ts @@ -838,4 +838,101 @@ describe('Tests Native Wrapper', () => { expect(result).toBeNull(); }); }); + + describe('primitiveProcessor and _setPrimitiveProcessor', () => { + describe('primitiveProcessor', () => { + it('default primitiveProcessor returns value as string', () => { + expect(NATIVE.primitiveProcessor('test')).toBe('test'); + expect(NATIVE.primitiveProcessor(123)).toBe(123); + expect(NATIVE.primitiveProcessor(true)).toBe(true); + expect(NATIVE.primitiveProcessor(null)).toBe(null); + expect(NATIVE.primitiveProcessor(undefined)).toBe(undefined); + }); + + it('handles all primitive types correctly', () => { + const testCases = [ + { input: 'string', expected: 'string' }, + { input: 42, expected: 42 }, + { input: true, expected: true }, + { input: false, expected: false }, + { input: null, expected: null }, + { input: undefined, expected: undefined }, + { input: BigInt(123), expected: BigInt(123) }, + ]; + + testCases.forEach(({ input, expected }) => { + expect(NATIVE.primitiveProcessor(input)).toBe(expected); + }); + }); + }); + + describe('_setPrimitiveProcessor', () => { + it('sets primitiveProcessor to the provided function', () => { + const mockProcessor = jest.fn(value => `processed_${value}`); + + NATIVE._setPrimitiveProcessor(mockProcessor); + + expect(NATIVE.primitiveProcessor).toBe(mockProcessor); + }); + + it('allows custom processing of primitive values', () => { + const customProcessor = (value: any) => { + if (typeof value === 'boolean') { + return value ? 'YES' : 'NO'; + } + if (value === null) { + return 'NULL'; + } + return String(value); + }; + + NATIVE._setPrimitiveProcessor(customProcessor); + + expect(NATIVE.primitiveProcessor(true)).toBe('YES'); + expect(NATIVE.primitiveProcessor(false)).toBe('NO'); + expect(NATIVE.primitiveProcessor(null)).toBe('NULL'); + expect(NATIVE.primitiveProcessor(42)).toBe('42'); + expect(NATIVE.primitiveProcessor('test')).toBe('test'); + }); + + it('can be chained with PrimitiveToString for consistent formatting', () => { + const { PrimitiveToString } = require('../src/js/utils/primitiveConverter'); + + NATIVE._setPrimitiveProcessor(PrimitiveToString); + + expect(NATIVE.primitiveProcessor(true)).toBe('True'); + expect(NATIVE.primitiveProcessor(false)).toBe('False'); + expect(NATIVE.primitiveProcessor(null)).toBe(''); + expect(NATIVE.primitiveProcessor(42)).toBe('42'); + expect(NATIVE.primitiveProcessor('test')).toBe('test'); + expect(NATIVE.primitiveProcessor(undefined)).toBeUndefined(); + }); + + it('can be reset to default behavior', () => { + const customProcessor = jest.fn(); + NATIVE._setPrimitiveProcessor(customProcessor); + expect(NATIVE.primitiveProcessor).toBe(customProcessor); + + const defaultProcessor = (value: any) => value; + NATIVE._setPrimitiveProcessor(defaultProcessor); + expect(NATIVE.primitiveProcessor).toBe(defaultProcessor); + }); + + it('works with primitiveTagIntegration', () => { + const { primitiveTagIntegration } = require('../src/js/integrations/primitiveTagIntegration'); + + const client = { + on: jest.fn(), + }; + + const integration = primitiveTagIntegration(); + integration.setup(client); + integration.afterAllSetup(); + + expect(NATIVE.primitiveProcessor(true)).toBe('True'); + expect(NATIVE.primitiveProcessor(false)).toBe('False'); + expect(NATIVE.primitiveProcessor(null)).toBe(''); + }); + }); + }); }); diff --git a/samples/react-native/src/Screens/ErrorsScreen.tsx b/samples/react-native/src/Screens/ErrorsScreen.tsx index 26823aa3a0..cfce6f9c00 100644 --- a/samples/react-native/src/Screens/ErrorsScreen.tsx +++ b/samples/react-native/src/Screens/ErrorsScreen.tsx @@ -20,6 +20,7 @@ import { FallbackRender } from '@sentry/react'; import NativeSampleModule from '../../tm/NativeSampleModule'; import NativePlatformSampleModule from '../../tm/NativePlatformSampleModule'; import { TimeToFullDisplay } from '../utils'; +import type { Event as SentryEvent } from '@sentry/core'; const { AssetsModule, CppModule, CrashModule } = NativeModules; @@ -226,6 +227,48 @@ const ErrorsScreen = (_props: Props) => { } }} /> +