From 20a698481e48801c36c56d2e0075d458c9d2dba4 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 13 Dec 2022 14:31:23 +0100 Subject: [PATCH 1/9] ref(client): Remove dep on BrowserClient --- src/js/client.ts | 62 +++++++++++++++++++++--------------------- src/js/eventbuilder.ts | 61 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 31 deletions(-) create mode 100644 src/js/eventbuilder.ts diff --git a/src/js/client.ts b/src/js/client.ts index ebf79ed0ed..d83e496645 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -1,4 +1,4 @@ -import { BrowserClient, defaultStackParser, makeFetchTransport } from '@sentry/browser'; +import { makeFetchTransport } from '@sentry/browser'; import { FetchImpl } from '@sentry/browser/types/transports/utils'; import { BaseClient } from '@sentry/core'; import { @@ -15,6 +15,7 @@ import { import { dateTimestampInSeconds, logger, SentryError } from '@sentry/utils'; // @ts-ignore LogBox introduced in RN 0.63 import { Alert, LogBox, YellowBox } from 'react-native'; +import { eventFromException, eventFromMessage } from './eventbuilder'; import { Screenshot } from './integrations/screenshot'; import { defaultSdkInfo } from './integrations/sdkinfo'; @@ -34,26 +35,24 @@ export class ReactNativeClient extends BaseClient { private _outcomesBuffer: Outcome[]; - private readonly _browserClient: BrowserClient; - /** * Creates a new React Native SDK instance. * @param options Configuration options for this SDK. */ - public constructor(options: ReactNativeClientOptions) { - if (!options.transport) { - options.transport = (options: ReactNativeTransportOptions, nativeFetch?: FetchImpl): Transport => { - if (NATIVE.isNativeTransportAvailable()) { - return makeReactNativeTransport(options); - } - return makeFetchTransport(options, nativeFetch); - }; - } - options._metadata = options._metadata || {}; - options._metadata.sdk = options._metadata.sdk || defaultSdkInfo; - super(options); - - this._outcomesBuffer = []; + public constructor(options: ReactNativeClientOptions) { + if (!options.transport) { + options.transport = (options: ReactNativeTransportOptions, nativeFetch?: FetchImpl): Transport => { + if (NATIVE.isNativeTransportAvailable()) { + return makeReactNativeTransport(options); + } + return makeFetchTransport(options, nativeFetch); + }; + } + options._metadata = options._metadata || {}; + options._metadata.sdk = options._metadata.sdk || defaultSdkInfo; + super(options); + + this._outcomesBuffer = []; // This is a workaround for now using fetch on RN, this is a known issue in react-native and only generates a warning // YellowBox deprecated and replaced with with LogBox in RN 0.63 @@ -65,17 +64,7 @@ export class ReactNativeClient extends BaseClient { YellowBox.ignoreWarnings(['Require cycle:']); } - this._browserClient = new BrowserClient({ - dsn: options.dsn, - transport: options.transport, - transportOptions: options.transportOptions, - stackParser: options.stackParser || defaultStackParser, - integrations: [], - _metadata: options._metadata, - attachStacktrace: options.attachStacktrace, - }); - - void this._initNativeSdk(); + void this._initNativeSdk(); } @@ -84,14 +73,25 @@ export class ReactNativeClient extends BaseClient { */ public eventFromException(exception: unknown, hint: EventHint = {}): PromiseLike { return Screenshot.attachScreenshotToEventHint(hint, this._options) - .then(enrichedHint => this._browserClient.eventFromException(exception, enrichedHint)); + .then(hintWithScreenshot => eventFromException( + this._options.stackParser, + exception, + hintWithScreenshot, + this._options.attachStacktrace, + )); } /** * @inheritDoc */ - public eventFromMessage(_message: string, _level?: SeverityLevel, _hint?: EventHint): PromiseLike { - return this._browserClient.eventFromMessage(_message, _level, _hint); + public eventFromMessage(message: string, level?: SeverityLevel, hint?: EventHint): PromiseLike { + return eventFromMessage( + this._options.stackParser, + message, + level, + hint, + this._options.attachStacktrace, + ); } /** diff --git a/src/js/eventbuilder.ts b/src/js/eventbuilder.ts new file mode 100644 index 0000000000..ea468446a8 --- /dev/null +++ b/src/js/eventbuilder.ts @@ -0,0 +1,61 @@ +import { resolvedSyncPromise } from '@sentry/utils'; +import { StackParser, Severity, SeverityLevel, EventHint, Event, Thread } from '@sentry/types'; +import { parseStackFrames } from '@sentry/browser/cjs/eventbuilder'; + +export { eventFromException } from '@sentry/browser/cjs/eventbuilder'; + +/** + * @hidden + * To be removed after update to JS SDK v8 + */ +interface EventWithThreads extends Event { + threads?: { + values: Thread[]; + }; +} + +/** + * Builds and Event from a Message + * @hidden + */ +export function eventFromMessage( + stackParser: StackParser, + message: string, + // eslint-disable-next-line deprecation/deprecation + level: Severity | SeverityLevel = 'info', + hint?: EventHint, + attachStacktrace?: boolean, +): PromiseLike { + const syntheticException = (hint && hint.syntheticException) || undefined; + const event = messageEventFromString(stackParser, message, syntheticException, attachStacktrace); + event.level = level; + if (hint && hint.event_id) { + event.event_id = hint.event_id; + } + return resolvedSyncPromise(event); +} + +/** + * @hidden + */ +export function messageEventFromString( + stackParser: StackParser, + input: string, + syntheticException?: Error, + attachStacktrace?: boolean, +): Event { + const event: EventWithThreads = { + message: input, + }; + + if (attachStacktrace && syntheticException) { + const frames = parseStackFrames(stackParser, syntheticException); + if (frames.length) { + event.threads = { + values: [{ stacktrace: { frames } }], + }; + } + } + + return event; +} From 317785355a8d9c9538e7b40abe6d4d22500020d9 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 13 Dec 2022 14:34:37 +0100 Subject: [PATCH 2/9] fix formatting --- src/js/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/client.ts b/src/js/client.ts index d83e496645..2ee1782f46 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -65,7 +65,7 @@ export class ReactNativeClient extends BaseClient { } void this._initNativeSdk(); - } + } /** From 24c9cddeb35f827eec636ffbda80fe8147a4306a Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 13 Dec 2022 16:06:14 +0100 Subject: [PATCH 3/9] ref: move exception to threads for messages --- src/js/client.ts | 20 +++++++++++--- src/js/eventbuilder.ts | 61 ------------------------------------------ 2 files changed, 17 insertions(+), 64 deletions(-) delete mode 100644 src/js/eventbuilder.ts diff --git a/src/js/client.ts b/src/js/client.ts index 2ee1782f46..c2f8f753e1 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -1,4 +1,4 @@ -import { makeFetchTransport } from '@sentry/browser'; +import { makeFetchTransport, eventFromException, eventFromMessage } from '@sentry/browser'; import { FetchImpl } from '@sentry/browser/types/transports/utils'; import { BaseClient } from '@sentry/core'; import { @@ -7,15 +7,16 @@ import { Envelope, Event, EventHint, + Exception, Outcome, SeverityLevel, + Thread, Transport, UserFeedback, } from '@sentry/types'; import { dateTimestampInSeconds, logger, SentryError } from '@sentry/utils'; // @ts-ignore LogBox introduced in RN 0.63 import { Alert, LogBox, YellowBox } from 'react-native'; -import { eventFromException, eventFromMessage } from './eventbuilder'; import { Screenshot } from './integrations/screenshot'; import { defaultSdkInfo } from './integrations/sdkinfo'; @@ -91,7 +92,20 @@ export class ReactNativeClient extends BaseClient { level, hint, this._options.attachStacktrace, - ); + ).then((event: Event) => { + // TMP! Remove this function once JS SDK uses threads for messages + if (!event.exception?.values || event.exception.values.length <= 0) { + return event; + } + + const values = event.exception.values.map((exception: Exception): Thread => ({ + id: exception.thread_id, + stacktrace: exception.stacktrace, + })); + (event as { threads?: { values: Thread[] } }).threads = { values }; + delete event.exception; + return event; + }); } /** diff --git a/src/js/eventbuilder.ts b/src/js/eventbuilder.ts deleted file mode 100644 index ea468446a8..0000000000 --- a/src/js/eventbuilder.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { resolvedSyncPromise } from '@sentry/utils'; -import { StackParser, Severity, SeverityLevel, EventHint, Event, Thread } from '@sentry/types'; -import { parseStackFrames } from '@sentry/browser/cjs/eventbuilder'; - -export { eventFromException } from '@sentry/browser/cjs/eventbuilder'; - -/** - * @hidden - * To be removed after update to JS SDK v8 - */ -interface EventWithThreads extends Event { - threads?: { - values: Thread[]; - }; -} - -/** - * Builds and Event from a Message - * @hidden - */ -export function eventFromMessage( - stackParser: StackParser, - message: string, - // eslint-disable-next-line deprecation/deprecation - level: Severity | SeverityLevel = 'info', - hint?: EventHint, - attachStacktrace?: boolean, -): PromiseLike { - const syntheticException = (hint && hint.syntheticException) || undefined; - const event = messageEventFromString(stackParser, message, syntheticException, attachStacktrace); - event.level = level; - if (hint && hint.event_id) { - event.event_id = hint.event_id; - } - return resolvedSyncPromise(event); -} - -/** - * @hidden - */ -export function messageEventFromString( - stackParser: StackParser, - input: string, - syntheticException?: Error, - attachStacktrace?: boolean, -): Event { - const event: EventWithThreads = { - message: input, - }; - - if (attachStacktrace && syntheticException) { - const frames = parseStackFrames(stackParser, syntheticException); - if (frames.length) { - event.threads = { - values: [{ stacktrace: { frames } }], - }; - } - } - - return event; -} From e4c64f652d823888a916f51b930dff79fe6f3d8e Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 13 Dec 2022 16:08:06 +0100 Subject: [PATCH 4/9] only map stacktrace --- src/js/client.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/js/client.ts b/src/js/client.ts index c2f8f753e1..4ce75893cc 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -99,7 +99,6 @@ export class ReactNativeClient extends BaseClient { } const values = event.exception.values.map((exception: Exception): Thread => ({ - id: exception.thread_id, stacktrace: exception.stacktrace, })); (event as { threads?: { values: Thread[] } }).threads = { values }; From a89878b890b9dd125a29a11cf061146592ee2449 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Dec 2022 10:49:15 +0100 Subject: [PATCH 5/9] Fix imports order --- src/js/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/client.ts b/src/js/client.ts index 4ce75893cc..f9dd741eec 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -1,4 +1,4 @@ -import { makeFetchTransport, eventFromException, eventFromMessage } from '@sentry/browser'; +import { eventFromException, eventFromMessage,makeFetchTransport } from '@sentry/browser'; import { FetchImpl } from '@sentry/browser/types/transports/utils'; import { BaseClient } from '@sentry/core'; import { From 898da273cd75bb54082e9b7cad515347c78f86ab Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Dec 2022 10:53:16 +0100 Subject: [PATCH 6/9] Add changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6acbd3b103..1482c75b8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Add `lastEventId` method to the API ([#2675](https://github.com/getsentry/sentry-react-native/pull/2675)) +### Breaking changes + +- Message event current stack trace moved from exception to threads ([#2694](https://github.com/getsentry/sentry-react-native/pull/2694)) + ### Dependencies - Bump Cocoa SDK from v7.31.2 to v7.31.3 ([#2647](https://github.com/getsentry/sentry-react-native/pull/2647)) From bffe4162c3c233ac5a89bc89f75ad3490906e0c5 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Dec 2022 11:30:17 +0100 Subject: [PATCH 7/9] Formatting --- src/js/client.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/js/client.ts b/src/js/client.ts index f9dd741eec..f124b7aa05 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -97,7 +97,6 @@ export class ReactNativeClient extends BaseClient { if (!event.exception?.values || event.exception.values.length <= 0) { return event; } - const values = event.exception.values.map((exception: Exception): Thread => ({ stacktrace: exception.stacktrace, })); From 3c2a41bc0997d807efa8c92132f62ad9faaab534 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Dec 2022 11:59:21 +0100 Subject: [PATCH 8/9] Add test to check message event threads --- test/client.test.ts | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/test/client.test.ts b/test/client.test.ts index ced3a2755f..172450cb3f 100644 --- a/test/client.test.ts +++ b/test/client.test.ts @@ -1,4 +1,5 @@ -import { Envelope, Event,Outcome, Transport } from '@sentry/types'; +import { defaultStackParser } from '@sentry/browser'; +import { Envelope, Event, Outcome, Transport } from '@sentry/types'; import { rejectedSyncPromise, SentryError } from '@sentry/utils'; import * as RN from 'react-native'; @@ -234,6 +235,39 @@ describe('Tests ReactNativeClient', () => { }); }); + describe('attachStacktrace', () => { + let mockTransportSend: jest.Mock; + let client: ReactNativeClient; + + beforeEach(() => { + mockTransportSend = jest.fn(() => Promise.resolve()); + client = new ReactNativeClient({ + ...DEFAULT_OPTIONS, + attachStacktrace: true, + stackParser: defaultStackParser, + dsn: EXAMPLE_DSN, + transport: () => ({ + send: mockTransportSend, + flush: jest.fn(), + }), + } as ReactNativeClientOptions); + }); + + afterEach(() => { + mockTransportSend.mockClear(); + }); + + const getMessageEventFrom = (func: jest.Mock) => + func.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload]; + + test('captureMessage contains stack trace in threads', async () => { + const mockSyntheticExceptionFromHub = new Error(); + client.captureMessage('test message', 'error', { syntheticException: mockSyntheticExceptionFromHub }); + expect(getMessageEventFrom(mockTransportSend).threads.values.length).toBeGreaterThan(0); + expect(getMessageEventFrom(mockTransportSend).exception).toBeUndefined(); + }); + }); + describe('envelopeHeader SdkInfo', () => { let mockTransportSend: jest.Mock; let client: ReactNativeClient; From 3cb00a9e17f89fcda813a978b2fa5084a6fed4ff Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Dec 2022 12:07:00 +0100 Subject: [PATCH 9/9] Add attach stack trace default true --- src/js/sdk.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/js/sdk.tsx b/src/js/sdk.tsx index 11e5998c74..a69d305e41 100644 --- a/src/js/sdk.tsx +++ b/src/js/sdk.tsx @@ -46,6 +46,7 @@ const DEFAULT_OPTIONS: ReactNativeOptions = { }, sendClientReports: true, maxQueueSize: DEFAULT_BUFFER_SIZE, + attachStacktrace: true, }; /**