From 20a698481e48801c36c56d2e0075d458c9d2dba4 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 13 Dec 2022 14:31:23 +0100 Subject: [PATCH 01/16] 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 02/16] 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 b859da13c172c920b6ac5432306fcfaffebb5533 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 13 Dec 2022 15:32:37 +0100 Subject: [PATCH 03/16] ref(client): Simplify client constructor --- src/js/client.ts | 32 ++++++------------------- src/js/transports/factory.ts | 18 ++++++++++++++ src/js/utils/disablerequirecyclelogs.ts | 15 ++++++++++++ 3 files changed, 40 insertions(+), 25 deletions(-) create mode 100644 src/js/transports/factory.ts create mode 100644 src/js/utils/disablerequirecyclelogs.ts diff --git a/src/js/client.ts b/src/js/client.ts index 2ee1782f46..6f5b1a23d6 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -1,5 +1,4 @@ -import { makeFetchTransport } from '@sentry/browser'; -import { FetchImpl } from '@sentry/browser/types/transports/utils'; +import { defaultStackParser } from '@sentry/browser'; import { BaseClient } from '@sentry/core'; import { ClientReportEnvelope, @@ -9,7 +8,6 @@ import { EventHint, Outcome, SeverityLevel, - Transport, UserFeedback, } from '@sentry/types'; import { dateTimestampInSeconds, logger, SentryError } from '@sentry/utils'; @@ -19,11 +17,12 @@ import { eventFromException, eventFromMessage } from './eventbuilder'; import { Screenshot } from './integrations/screenshot'; import { defaultSdkInfo } from './integrations/sdkinfo'; -import { ReactNativeClientOptions, ReactNativeTransportOptions } from './options'; -import { makeReactNativeTransport } from './transports/native'; +import { ReactNativeClientOptions } from './options'; import { createUserFeedbackEnvelope, items } from './utils/envelope'; import { mergeOutcomes } from './utils/outcome'; import { NATIVE } from './wrapper'; +import { makeTransport } from './transports/factory'; +import { disableRequireCycleLogs } from './utils/disablerequirecyclelogs'; /** * The Sentry React Native SDK Client. @@ -40,34 +39,17 @@ export class ReactNativeClient extends BaseClient { * @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); - }; - } + disableRequireCycleLogs(); + options.transport = options.transport || makeTransport; options._metadata = options._metadata || {}; options._metadata.sdk = options._metadata.sdk || defaultSdkInfo; + options.stackParser = options.stackParser || defaultStackParser; 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 - if (LogBox) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - LogBox.ignoreLogs(['Require cycle:']); - } else { - // eslint-disable-next-line deprecation/deprecation - YellowBox.ignoreWarnings(['Require cycle:']); - } - void this._initNativeSdk(); } - /** * @inheritDoc */ diff --git a/src/js/transports/factory.ts b/src/js/transports/factory.ts new file mode 100644 index 0000000000..c02fdaa84b --- /dev/null +++ b/src/js/transports/factory.ts @@ -0,0 +1,18 @@ +import { makeFetchTransport } from '@sentry/browser'; +import { FetchImpl } from '@sentry/browser/types/transports/utils'; +import { Transport } from '@sentry/types'; + +import { NATIVE } from '../wrapper'; +import { makeReactNativeTransport } from './native'; +import { ReactNativeTransportOptions } from '../options'; + +/** + * Creates native transport if available. + * Fallbacks to fetch transport otherwise. + */ +export function makeTransport(options: ReactNativeTransportOptions, nativeFetch?: FetchImpl): Transport { + if (NATIVE.isNativeTransportAvailable()) { + return makeReactNativeTransport(options); + } + return makeFetchTransport(options, nativeFetch); +} diff --git a/src/js/utils/disablerequirecyclelogs.ts b/src/js/utils/disablerequirecyclelogs.ts new file mode 100644 index 0000000000..f008e0562e --- /dev/null +++ b/src/js/utils/disablerequirecyclelogs.ts @@ -0,0 +1,15 @@ +import { LogBox, YellowBox } from 'react-native'; + +/** + * 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 + */ +export function disableRequireCycleLogs() { + if (LogBox) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + LogBox.ignoreLogs(['Require cycle:']); + } else { + // eslint-disable-next-line deprecation/deprecation + YellowBox.ignoreWarnings(['Require cycle:']); + } +} From 24c9cddeb35f827eec636ffbda80fe8147a4306a Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 13 Dec 2022 16:06:14 +0100 Subject: [PATCH 04/16] 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 05/16] 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 eafdd0e0bd80666707cffe588b75bf869118890b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 13 Dec 2022 21:31:04 +0100 Subject: [PATCH 06/16] Remove unnecessary transport factory, rename ignore logs --- src/js/client.ts | 13 ++++++------- src/js/sdk.tsx | 4 ++-- src/js/transports/factory.ts | 18 ------------------ src/js/transports/native.ts | 7 +++++-- ...ecyclelogs.ts => ignorerequirecyclelogs.ts} | 5 ++--- 5 files changed, 15 insertions(+), 32 deletions(-) delete mode 100644 src/js/transports/factory.ts rename src/js/utils/{disablerequirecyclelogs.ts => ignorerequirecyclelogs.ts} (67%) diff --git a/src/js/client.ts b/src/js/client.ts index 33de21482a..2e2c7735e9 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -1,4 +1,4 @@ -import { defaultStackParser, eventFromException, eventFromMessage } from '@sentry/browser'; +import { defaultStackParser, eventFromException, eventFromMessage, makeFetchTransport } from '@sentry/browser'; import { BaseClient } from '@sentry/core'; import { ClientReportEnvelope, @@ -13,15 +13,14 @@ import { 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 { Alert } from 'react-native'; import { Screenshot } from './integrations/screenshot'; import { defaultSdkInfo } from './integrations/sdkinfo'; import { ReactNativeClientOptions } from './options'; -import { makeTransport } from './transports/factory'; -import { disableRequireCycleLogs } from './utils/disablerequirecyclelogs'; +import { makeNativeTransport } from './transports/native'; import { createUserFeedbackEnvelope, items } from './utils/envelope'; +import { ignoreRequireCycleLogs } from './utils/ignorerequirecyclelogs'; import { mergeOutcomes } from './utils/outcome'; import { NATIVE } from './wrapper'; @@ -40,8 +39,8 @@ export class ReactNativeClient extends BaseClient { * @param options Configuration options for this SDK. */ public constructor(options: ReactNativeClientOptions) { - disableRequireCycleLogs(); - options.transport = options.transport || makeTransport; + ignoreRequireCycleLogs(); + options.transport = options.transport || makeNativeTransport || makeFetchTransport; options._metadata = options._metadata || {}; options._metadata.sdk = options._metadata.sdk || defaultSdkInfo; options.stackParser = options.stackParser || defaultStackParser; diff --git a/src/js/sdk.tsx b/src/js/sdk.tsx index 45ffba150e..2cff993a04 100644 --- a/src/js/sdk.tsx +++ b/src/js/sdk.tsx @@ -24,7 +24,7 @@ import { ReactNativeClientOptions, ReactNativeOptions, ReactNativeWrapperOptions import { ReactNativeScope } from './scope'; import { TouchEventBoundary } from './touchevents'; import { ReactNativeProfiler, ReactNativeTracing } from './tracing'; -import { DEFAULT_BUFFER_SIZE, makeReactNativeTransport } from './transports/native'; +import { DEFAULT_BUFFER_SIZE, makeNativeTransport } from './transports/native'; import { makeUtf8TextEncoder } from './transports/TextEncoder'; import { safeFactory, safeTracesSampler } from './utils/safe'; import { RN_GLOBAL_OBJ } from './utils/worldwide'; @@ -63,7 +63,7 @@ export function init(passedOptions: ReactNativeOptions): void { ...DEFAULT_OPTIONS, ...passedOptions, // If custom transport factory fails the SDK won't initialize - transport: passedOptions.transport || makeReactNativeTransport, + transport: passedOptions.transport || makeNativeTransport, transportOptions: { ...DEFAULT_OPTIONS.transportOptions, ...(passedOptions.transportOptions ?? {}), diff --git a/src/js/transports/factory.ts b/src/js/transports/factory.ts deleted file mode 100644 index c02fdaa84b..0000000000 --- a/src/js/transports/factory.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { makeFetchTransport } from '@sentry/browser'; -import { FetchImpl } from '@sentry/browser/types/transports/utils'; -import { Transport } from '@sentry/types'; - -import { NATIVE } from '../wrapper'; -import { makeReactNativeTransport } from './native'; -import { ReactNativeTransportOptions } from '../options'; - -/** - * Creates native transport if available. - * Fallbacks to fetch transport otherwise. - */ -export function makeTransport(options: ReactNativeTransportOptions, nativeFetch?: FetchImpl): Transport { - if (NATIVE.isNativeTransportAvailable()) { - return makeReactNativeTransport(options); - } - return makeFetchTransport(options, nativeFetch); -} diff --git a/src/js/transports/native.ts b/src/js/transports/native.ts index 9eee06764f..376d9aecd2 100644 --- a/src/js/transports/native.ts +++ b/src/js/transports/native.ts @@ -45,6 +45,9 @@ export class NativeTransport implements Transport { /** * Creates a Native Transport. */ -export function makeReactNativeTransport(options: BaseNativeTransportOptions = {}): NativeTransport { - return new NativeTransport(options); +export function makeNativeTransport(options: BaseNativeTransportOptions = {}): NativeTransport | null { + if (NATIVE.isNativeTransportAvailable()) { + return new NativeTransport(options); + } + return null; } diff --git a/src/js/utils/disablerequirecyclelogs.ts b/src/js/utils/ignorerequirecyclelogs.ts similarity index 67% rename from src/js/utils/disablerequirecyclelogs.ts rename to src/js/utils/ignorerequirecyclelogs.ts index f008e0562e..5fdc9120be 100644 --- a/src/js/utils/disablerequirecyclelogs.ts +++ b/src/js/utils/ignorerequirecyclelogs.ts @@ -1,15 +1,14 @@ +/* eslint-disable deprecation/deprecation */ import { LogBox, YellowBox } from 'react-native'; /** * 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 */ -export function disableRequireCycleLogs() { +export function ignoreRequireCycleLogs(): void { if (LogBox) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access LogBox.ignoreLogs(['Require cycle:']); } else { - // eslint-disable-next-line deprecation/deprecation YellowBox.ignoreWarnings(['Require cycle:']); } } From a89878b890b9dd125a29a11cf061146592ee2449 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Dec 2022 10:49:15 +0100 Subject: [PATCH 07/16] 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 08/16] 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 61fda35bb6a394d351aeb04b5419d3ef49b67485 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Dec 2022 11:25:49 +0100 Subject: [PATCH 09/16] Fix fetch transport can be reached --- src/js/client.ts | 4 +--- src/js/sdk.tsx | 5 +++-- src/js/transports/native.ts | 11 +++++++++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/js/client.ts b/src/js/client.ts index 2e2c7735e9..a61580c73a 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -1,4 +1,4 @@ -import { defaultStackParser, eventFromException, eventFromMessage, makeFetchTransport } from '@sentry/browser'; +import { defaultStackParser, eventFromException, eventFromMessage } from '@sentry/browser'; import { BaseClient } from '@sentry/core'; import { ClientReportEnvelope, @@ -18,7 +18,6 @@ import { Alert } from 'react-native'; import { Screenshot } from './integrations/screenshot'; import { defaultSdkInfo } from './integrations/sdkinfo'; import { ReactNativeClientOptions } from './options'; -import { makeNativeTransport } from './transports/native'; import { createUserFeedbackEnvelope, items } from './utils/envelope'; import { ignoreRequireCycleLogs } from './utils/ignorerequirecyclelogs'; import { mergeOutcomes } from './utils/outcome'; @@ -40,7 +39,6 @@ export class ReactNativeClient extends BaseClient { */ public constructor(options: ReactNativeClientOptions) { ignoreRequireCycleLogs(); - options.transport = options.transport || makeNativeTransport || makeFetchTransport; options._metadata = options._metadata || {}; options._metadata.sdk = options._metadata.sdk || defaultSdkInfo; options.stackParser = options.stackParser || defaultStackParser; diff --git a/src/js/sdk.tsx b/src/js/sdk.tsx index a86c9755b2..8a3af37a7a 100644 --- a/src/js/sdk.tsx +++ b/src/js/sdk.tsx @@ -4,6 +4,7 @@ import { defaultIntegrations as reactDefaultIntegrations, defaultStackParser, getCurrentHub, + makeFetchTransport, } from '@sentry/react'; import { Integration, StackFrame, UserFeedback } from '@sentry/types'; import { logger, stackParserFromStackParserOptions } from '@sentry/utils'; @@ -25,7 +26,7 @@ import { ReactNativeClientOptions, ReactNativeOptions, ReactNativeWrapperOptions import { ReactNativeScope } from './scope'; import { TouchEventBoundary } from './touchevents'; import { ReactNativeProfiler, ReactNativeTracing } from './tracing'; -import { DEFAULT_BUFFER_SIZE, makeNativeTransport } from './transports/native'; +import { DEFAULT_BUFFER_SIZE, makeNativeTransportFactory } from './transports/native'; import { makeUtf8TextEncoder } from './transports/TextEncoder'; import { safeFactory, safeTracesSampler } from './utils/safe'; @@ -63,7 +64,7 @@ export function init(passedOptions: ReactNativeOptions): void { ...DEFAULT_OPTIONS, ...passedOptions, // If custom transport factory fails the SDK won't initialize - transport: passedOptions.transport || makeNativeTransport, + transport: passedOptions.transport || makeNativeTransportFactory() || makeFetchTransport, transportOptions: { ...DEFAULT_OPTIONS.transportOptions, ...(passedOptions.transportOptions ?? {}), diff --git a/src/js/transports/native.ts b/src/js/transports/native.ts index 376d9aecd2..9bc2e26e88 100644 --- a/src/js/transports/native.ts +++ b/src/js/transports/native.ts @@ -45,9 +45,16 @@ export class NativeTransport implements Transport { /** * Creates a Native Transport. */ -export function makeNativeTransport(options: BaseNativeTransportOptions = {}): NativeTransport | null { +export function makeNativeTransport(options: BaseNativeTransportOptions = {}): NativeTransport { + return new NativeTransport(options); +} + +/** + * Creates a Native Transport factory if the native transport is available. + */ +export function makeNativeTransportFactory(): typeof makeNativeTransport | null { if (NATIVE.isNativeTransportAvailable()) { - return new NativeTransport(options); + return makeNativeTransport; } return null; } From bffe4162c3c233ac5a89bc89f75ad3490906e0c5 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Dec 2022 11:30:17 +0100 Subject: [PATCH 10/16] 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 11/16] 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 12/16] 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, }; /** From a6922789a6b074b9eeaec8fce9df555dfa28305e Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Dec 2022 14:03:40 +0100 Subject: [PATCH 13/16] Add tests and changelog --- CHANGELOG.md | 4 ++++ test/sdk.test.ts | 36 +++++++++++++++++++++++++++++++----- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe363c398f..ff84786871 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ - Message event current stack trace moved from exception to threads ([#2694](https://github.com/getsentry/sentry-react-native/pull/2694)) +### Fixes + +- Unreachable fallback to fetch transport if native is not available + ### Dependencies - Bump Cocoa SDK from v7.31.2 to v7.31.3 ([#2647](https://github.com/getsentry/sentry-react-native/pull/2647)) diff --git a/test/sdk.test.ts b/test/sdk.test.ts index cb4fc3d3cd..e3cd17f2a1 100644 --- a/test/sdk.test.ts +++ b/test/sdk.test.ts @@ -3,6 +3,8 @@ */ import { logger } from '@sentry/utils'; +import { NATIVE } from '../src/js/wrapper'; + interface MockedClient { flush: jest.Mock; } @@ -61,18 +63,24 @@ jest.mock('../src/js/client', () => { }; }); +jest.mock('../src/js/wrapper'); + jest.spyOn(logger, 'error'); import { initAndBind } from '@sentry/core'; -import { getCurrentHub } from '@sentry/react'; +import { getCurrentHub, makeFetchTransport } from '@sentry/react'; import { BaseTransportOptions,ClientOptions, Integration, Scope } from '@sentry/types'; import { ReactNativeClientOptions } from '../src/js/options'; import { configureScope,flush, init, withScope } from '../src/js/sdk'; import { ReactNativeTracing, ReactNavigationInstrumentation } from '../src/js/tracing'; +import { makeNativeTransport } from '../src/js/transports/native'; import { firstArg, secondArg } from './testutils'; const mockedInitAndBind = initAndBind as jest.MockedFunction; +const usedOptions = (): ClientOptions | undefined => { + return mockedInitAndBind.mock.calls[0]?.[1]; +} afterEach(() => { jest.clearAllMocks(); @@ -184,10 +192,6 @@ describe('Tests the SDK functionality', () => { }); describe('transport options buffer size', () => { - const usedOptions = (): ClientOptions | undefined => { - return mockedInitAndBind.mock.calls[0]?.[1]; - } - it('uses default transport options buffer size', () => { init({ tracesSampleRate: 0.5, @@ -214,6 +218,28 @@ describe('Tests the SDK functionality', () => { }); }); + describe('transport initialization', () => { + it('uses transport from the options', () => { + const mockTransport = jest.fn(); + init({ + transport: mockTransport, + }); + expect(usedOptions()?.transport).toEqual(mockTransport); + }); + + it('uses native transport', () => { + (NATIVE.isNativeTransportAvailable as jest.Mock).mockImplementation(() => true); + init({}); + expect(usedOptions()?.transport).toEqual(makeNativeTransport); + }); + + it('uses fallback fetch transport', () => { + (NATIVE.isNativeTransportAvailable as jest.Mock).mockImplementation(() => false); + init({}); + expect(usedOptions()?.transport).toEqual(makeFetchTransport); + }); + }); + describe('initIsSafe', () => { test('initialScope callback is safe after init', () => { const mockInitialScope = jest.fn(() => { throw 'Test error' }); From 6364cdc7ef22ff0c6f9f7917d82c52e92a7320c2 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Dec 2022 14:06:15 +0100 Subject: [PATCH 14/16] Add pr id --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff84786871..43c98bbb2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ ### Fixes -- Unreachable fallback to fetch transport if native is not available +- Unreachable fallback to fetch transport if native is not available ([#2695](https://github.com/getsentry/sentry-react-native/pull/2695)) ### Dependencies From e08e61c103958a8b51bdbcdd787b47ef135eb8ca Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 20 Dec 2022 15:08:03 +0100 Subject: [PATCH 15/16] Remove unnecessary stack parser fallback This is implemented in the init function in sdk.tsx --- src/js/client.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/js/client.ts b/src/js/client.ts index 361b34db9d..88e8cc3b37 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -41,7 +41,6 @@ export class ReactNativeClient extends BaseClient { ignoreRequireCycleLogs(); options._metadata = options._metadata || {}; options._metadata.sdk = options._metadata.sdk || defaultSdkInfo; - options.stackParser = options.stackParser || defaultStackParser; super(options); this._outcomesBuffer = []; From fccee60123fbc3c0373d74b2a88ba5dbbe8bdc7b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 20 Dec 2022 15:13:39 +0100 Subject: [PATCH 16/16] Fix unused defaultStackParser import --- 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 88e8cc3b37..5a3c47c0bd 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -1,4 +1,4 @@ -import { defaultStackParser, eventFromException, eventFromMessage } from '@sentry/browser'; +import { eventFromException, eventFromMessage } from '@sentry/browser'; import { BaseClient } from '@sentry/core'; import { ClientReportEnvelope,