From d72b4fa65a427ffe6a4598e8817efa0d3cd6270a Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 13 Nov 2024 14:44:56 +0100 Subject: [PATCH 1/3] fix(replay): `browserReplayIntegration` should not be included by default --- CHANGELOG.md | 1 + packages/core/src/js/integrations/default.ts | 10 ++- packages/core/test/sdk.test.ts | 80 +++++++++++++++++++- 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4439c0806a..867a44dfc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ - Remove duplicate HTTP Client Errors on iOS ([#4250](https://github.com/getsentry/sentry-react-native/pull/4250)) - Replay `maskAll*` set to `false` on iOS kept all masked ([#4257](https://github.com/getsentry/sentry-react-native/pull/4257)) - Add missing `getRootSpan`, `withActiveSpan` and `suppressTracing` exports from `@sentry/core`, and `SeverityLevel` export from `@sentry/types` ([#4254](https://github.com/getsentry/sentry-react-native/pull/4254), [#4260](https://github.com/getsentry/sentry-react-native/pull/4260)) +- `browserReplayIntegration` should not be included by default ([#4270](https://github.com/getsentry/sentry-react-native/pull/4270)) ### Dependencies diff --git a/packages/core/src/js/integrations/default.ts b/packages/core/src/js/integrations/default.ts index d60d8fe3be..868e5bcc34 100644 --- a/packages/core/src/js/integrations/default.ts +++ b/packages/core/src/js/integrations/default.ts @@ -4,14 +4,13 @@ import type { Integration } from '@sentry/types'; import type { ReactNativeClientOptions } from '../options'; import { reactNativeTracingIntegration } from '../tracing'; -import { isExpoGo, notWeb } from '../utils/environment'; +import { isExpoGo, isWeb, notWeb } from '../utils/environment'; import { appStartIntegration, breadcrumbsIntegration, browserApiErrorsIntegration, browserGlobalHandlersIntegration, browserLinkedErrorsIntegration, - browserReplayIntegration, createNativeFramesIntegrations, createReactNativeRewriteFrames, debugSymbolicatorIntegration, @@ -136,10 +135,13 @@ export function getDefaultIntegrations(options: ReactNativeClientOptions): Integ (options._experiments && typeof options._experiments.replaysOnErrorSampleRate === 'number') || (options._experiments && typeof options._experiments.replaysSessionSampleRate === 'number') ) { - integrations.push(notWeb() ? mobileReplayIntegration() : browserReplayIntegration()); - if (!notWeb()) { + if (isWeb()) { + // We can't create and add browserReplayIntegration as it overrides the users supplied one + // The browser replay integration works differently than the rest of default integrations (options as BrowserOptions).replaysOnErrorSampleRate = options._experiments.replaysOnErrorSampleRate; (options as BrowserOptions).replaysSessionSampleRate = options._experiments.replaysSessionSampleRate; + } else { + integrations.push(mobileReplayIntegration()); } } diff --git a/packages/core/test/sdk.test.ts b/packages/core/test/sdk.test.ts index b5487340cd..61ff9da964 100644 --- a/packages/core/test/sdk.test.ts +++ b/packages/core/test/sdk.test.ts @@ -7,7 +7,7 @@ import { init, withScope } from '../src/js/sdk'; import type { ReactNativeTracingIntegration } from '../src/js/tracing'; import { REACT_NATIVE_TRACING_INTEGRATION_NAME, reactNativeTracingIntegration } from '../src/js/tracing'; import { makeNativeTransport } from '../src/js/transports/native'; -import { getDefaultEnvironment, isExpoGo, notWeb } from '../src/js/utils/environment'; +import { getDefaultEnvironment, isExpoGo, isWeb, notWeb } from '../src/js/utils/environment'; import { NATIVE } from './mockWrapper'; import { firstArg, secondArg } from './testutils'; @@ -663,6 +663,84 @@ describe('Tests the SDK functionality', () => { expectIntegration('ExpoContext'); }); + + it('adds mobile replay integration when _experiments.replaysOnErrorSampleRate is set', () => { + init({ + _experiments: { + replaysOnErrorSampleRate: 1.0, + }, + }); + + expectIntegration('MobileReplay'); + }); + + it('adds mobile replay integration when _experiments.replaysSessionSampleRate is set', () => { + init({ + _experiments: { + replaysSessionSampleRate: 1.0, + }, + }); + + expectIntegration('MobileReplay'); + }); + + it('does not add mobile replay integration when no replay sample rates are set', () => { + init({ + _experiments: {}, + }); + + expectNotIntegration('MobileReplay'); + }); + + it('does not add any replay integration when on web even with on error sample rate', () => { + (isWeb as jest.Mock).mockImplementation(() => true); + init({ + _experiments: { + replaysOnErrorSampleRate: 1.0, + }, + }); + + expectNotIntegration('Replay'); + expectNotIntegration('MobileReplay'); + }); + + it('does not add any replay integration when on web even with session sample rate', () => { + (isWeb as jest.Mock).mockImplementation(() => true); + init({ + _experiments: { + replaysSessionSampleRate: 1.0, + }, + }); + + expectNotIntegration('Replay'); + expectNotIntegration('MobileReplay'); + }); + + it('does not add any replay integration when on web', () => { + (isWeb as jest.Mock).mockImplementation(() => true); + init({}); + + expectNotIntegration('Replay'); + expectNotIntegration('MobileReplay'); + }); + + it('converts experimental replay options to standard web options when on web', () => { + (isWeb as jest.Mock).mockImplementation(() => true); + init({ + _experiments: { + replaysOnErrorSampleRate: 0.5, + replaysSessionSampleRate: 0.1, + }, + }); + + const actualOptions = usedOptions(); + expect(actualOptions).toEqual( + expect.objectContaining({ + replaysOnErrorSampleRate: 0.5, + replaysSessionSampleRate: 0.1, + }), + ); + }); }); function expectIntegration(name: string): void { From 4f215c1d8c8f50c6af6629c7bf0cc139424c35c5 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:46:37 +0100 Subject: [PATCH 2/3] Update CHANGELOG.md Co-authored-by: LucasZF --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 867a44dfc8..c5a2e68e65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ - Remove duplicate HTTP Client Errors on iOS ([#4250](https://github.com/getsentry/sentry-react-native/pull/4250)) - Replay `maskAll*` set to `false` on iOS kept all masked ([#4257](https://github.com/getsentry/sentry-react-native/pull/4257)) - Add missing `getRootSpan`, `withActiveSpan` and `suppressTracing` exports from `@sentry/core`, and `SeverityLevel` export from `@sentry/types` ([#4254](https://github.com/getsentry/sentry-react-native/pull/4254), [#4260](https://github.com/getsentry/sentry-react-native/pull/4260)) -- `browserReplayIntegration` should not be included by default ([#4270](https://github.com/getsentry/sentry-react-native/pull/4270)) +- `browserReplayIntegration` is no longer included by default on React Native Web ([#4270](https://github.com/getsentry/sentry-react-native/pull/4270)) ### Dependencies From bf09ed40a1d3e2b6299acfc694483deb194c7b7e Mon Sep 17 00:00:00 2001 From: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:48:47 +0100 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe7b70405d..99e6b4f337 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ - Skips development server spans ([#4271](https://github.com/getsentry/sentry-react-native/pull/4271)) - Execute `DebugSymbolicator` after `RewriteFrames` to avoid overwrites by default ([#4285](https://github.com/getsentry/sentry-react-native/pull/4285)) - If custom `RewriteFrames` is provided the order changes +- `browserReplayIntegration` is no longer included by default on React Native Web ([#4270](https://github.com/getsentry/sentry-react-native/pull/4270)) ### Dependencies @@ -86,7 +87,6 @@ - Remove duplicate HTTP Client Errors on iOS ([#4250](https://github.com/getsentry/sentry-react-native/pull/4250)) - Replay `maskAll*` set to `false` on iOS kept all masked ([#4257](https://github.com/getsentry/sentry-react-native/pull/4257)) - Add missing `getRootSpan`, `withActiveSpan` and `suppressTracing` exports from `@sentry/core`, and `SeverityLevel` export from `@sentry/types` ([#4254](https://github.com/getsentry/sentry-react-native/pull/4254), [#4260](https://github.com/getsentry/sentry-react-native/pull/4260)) -- `browserReplayIntegration` is no longer included by default on React Native Web ([#4270](https://github.com/getsentry/sentry-react-native/pull/4270)) ### Dependencies