diff --git a/CHANGELOG.md b/CHANGELOG.md index ac4cbaffd5..28ff1e36ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ - This flag equals `true` when Hermes Bundle contains Debug Info (Hermes Source Map was not emitted) - Add `enableNdk` property to ReactNativeOptions for Android. ([#3304](https://github.com/getsentry/sentry-react-native/pull/3304)) +### Fixes + +- Cancel auto instrumentation transaction when app goes to background ([#3307])(https://github.com/getsentry/sentry-react-native/pull/3307) + ## 5.9.2 ### Fixes diff --git a/sample-new-architecture/metro.config.js b/sample-new-architecture/metro.config.js index 9ed92b08db..4ea8ab6b3e 100644 --- a/sample-new-architecture/metro.config.js +++ b/sample-new-architecture/metro.config.js @@ -20,7 +20,7 @@ const config = { resolver: { blacklistRE: blacklist([ new RegExp(`${parentDir}/node_modules/react-native/.*`), - new RegExp(`.*\\android\\.*`), // Required for Windows in order to run the Sample. + new RegExp('.*\\android\\.*'), // Required for Windows in order to run the Sample. ]), extraNodeModules: new Proxy( { diff --git a/sample-new-architecture/src/App.tsx b/sample-new-architecture/src/App.tsx index ab74583719..de8aeec6b0 100644 --- a/sample-new-architecture/src/App.tsx +++ b/sample-new-architecture/src/App.tsx @@ -35,6 +35,10 @@ Sentry.init({ console.log('Event beforeSend:', event.event_id); return event; }, + beforeSendTransaction(event) { + console.log('Transaction beforeSend:', event.event_id); + return event; + }, // This will be called with a boolean `didCallNativeInit` when the native SDK has been contacted. onReady: ({ didCallNativeInit }) => { console.log('onReady called with didCallNativeInit:', didCallNativeInit); diff --git a/src/js/tracing/reactnativetracing.ts b/src/js/tracing/reactnativetracing.ts index 5664578937..e05efe5dbe 100644 --- a/src/js/tracing/reactnativetracing.ts +++ b/src/js/tracing/reactnativetracing.ts @@ -13,7 +13,7 @@ import { NATIVE } from '../wrapper'; import { NativeFramesInstrumentation } from './nativeframes'; import { APP_START_COLD as APP_START_COLD_OP, APP_START_WARM as APP_START_WARM_OP, UI_LOAD } from './ops'; import { StallTrackingInstrumentation } from './stalltracking'; -import { onlySampleIfChildSpans } from './transaction'; +import { cancelInBackground, onlySampleIfChildSpans } from './transaction'; import type { BeforeNavigate, RouteChangeContextData } from './types'; import { adjustTransactionDuration, getTimeOriginMilliseconds, isNearToNow } from './utils'; @@ -306,8 +306,6 @@ export class ReactNativeTracing implements Integration { return; } - const { idleTimeoutMs, finalTimeoutMs } = this.options; - if (this._inflightInteractionTransaction) { this._inflightInteractionTransaction.cancelIdleTimeout(undefined, { restartOnChildSpanChange: false }); this._inflightInteractionTransaction = undefined; @@ -319,7 +317,7 @@ export class ReactNativeTracing implements Integration { op, trimEnd: true, }; - this._inflightInteractionTransaction = startIdleTransaction(hub, context, idleTimeoutMs, finalTimeoutMs, true); + this._inflightInteractionTransaction = this._startIdleTransaction(context); this._inflightInteractionTransaction.registerBeforeFinishCallback((transaction: IdleTransaction) => { this._inflightInteractionTransaction = undefined; this.onTransactionFinish(transaction); @@ -445,16 +443,14 @@ export class ReactNativeTracing implements Integration { this._inflightInteractionTransaction.finish(); } - // eslint-disable-next-line @typescript-eslint/unbound-method - const { idleTimeoutMs, finalTimeoutMs } = this.options; + const { finalTimeoutMs } = this.options; const expandedContext = { ...context, trimEnd: true, }; - const hub = this._getCurrentHub(); - const idleTransaction = startIdleTransaction(hub as Hub, expandedContext, idleTimeoutMs, finalTimeoutMs, true); + const idleTransaction = this._startIdleTransaction(expandedContext); this.onTransactionStart(idleTransaction); @@ -498,4 +494,15 @@ export class ReactNativeTracing implements Integration { return idleTransaction; } + + /** + * Start app state aware idle transaction on the scope. + */ + private _startIdleTransaction(context: TransactionContext): IdleTransaction { + const { idleTimeoutMs, finalTimeoutMs } = this.options; + const hub = this._getCurrentHub?.() || getCurrentHub(); + const tx = startIdleTransaction(hub, context, idleTimeoutMs, finalTimeoutMs, true); + cancelInBackground(tx); + return tx; + } } diff --git a/src/js/tracing/transaction.ts b/src/js/tracing/transaction.ts index ea149805ac..2686fe9603 100644 --- a/src/js/tracing/transaction.ts +++ b/src/js/tracing/transaction.ts @@ -1,5 +1,7 @@ -import type { BeforeFinishCallback, IdleTransaction } from '@sentry/core'; +import { type BeforeFinishCallback, type IdleTransaction } from '@sentry/core'; import { logger } from '@sentry/utils'; +import type { AppStateStatus } from 'react-native'; +import { AppState } from 'react-native'; /** * Idle Transaction callback to only sample transactions with child spans. @@ -15,3 +17,20 @@ export const onlySampleIfChildSpans: BeforeFinishCallback = (transaction: IdleTr transaction.sampled = false; } }; + +/** + * Hooks on AppState change to cancel the transaction if the app goes background. + */ +export const cancelInBackground = (transaction: IdleTransaction): void => { + const subscription = AppState.addEventListener('change', (newState: AppStateStatus) => { + if (newState === 'background') { + logger.debug(`Setting ${transaction.op} transaction to cancelled because the app is in the background.`); + transaction.setStatus('cancelled'); + transaction.finish(); + } + }); + transaction.registerBeforeFinishCallback(() => { + logger.debug(`Removing AppState listener for ${transaction.op} transaction.`); + subscription.remove(); + }); +}; diff --git a/test/tracing/reactnativetracing.test.ts b/test/tracing/reactnativetracing.test.ts index 346380096b..47fa0e8e6f 100644 --- a/test/tracing/reactnativetracing.test.ts +++ b/test/tracing/reactnativetracing.test.ts @@ -28,6 +28,29 @@ jest.mock('../../src/js/tracing/utils', () => { }; }); +type MockAppState = { + setState: (state: AppStateStatus) => void; + listener: (newState: AppStateStatus) => void; + removeSubscription: jest.Func; +}; +const mockedAppState: AppState & MockAppState = { + removeSubscription: jest.fn(), + listener: jest.fn(), + isAvailable: true, + currentState: 'active', + addEventListener: (_, listener) => { + mockedAppState.listener = listener; + return { + remove: mockedAppState.removeSubscription, + }; + }, + setState: (state: AppStateStatus) => { + mockedAppState.currentState = state; + mockedAppState.listener(state); + }, +}; +jest.mock('react-native/Libraries/AppState/AppState', () => mockedAppState); + const getMockScope = () => { let scopeTransaction: Transaction | undefined; let scopeUser: User | undefined; @@ -62,6 +85,7 @@ const getMockHub = () => { import type { BrowserClientOptions } from '@sentry/browser/types/client'; import type { Scope } from '@sentry/types'; +import type { AppState, AppStateStatus } from 'react-native'; import { APP_START_COLD, APP_START_WARM } from '../../src/js/measurements'; import { @@ -100,16 +124,7 @@ describe('ReactNativeTracing', () => { enableNativeFramesTracking: false, }); - const timeOriginMilliseconds = Date.now(); - const appStartTimeMilliseconds = timeOriginMilliseconds - 100; - const mockAppStartResponse: NativeAppStartResponse = { - isColdStart: true, - appStartTime: appStartTimeMilliseconds, - didFetchAppStart: false, - }; - - mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds); - mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse); + const [timeOriginMilliseconds, appStartTimeMilliseconds] = mockAppStartResponse({ cold: true }); const mockHub = getMockHub(); integration.setupOnce(addGlobalEventProcessor, () => mockHub); @@ -139,16 +154,7 @@ describe('ReactNativeTracing', () => { it('Starts route transaction (warm)', async () => { const integration = new ReactNativeTracing(); - const timeOriginMilliseconds = Date.now(); - const appStartTimeMilliseconds = timeOriginMilliseconds - 100; - const mockAppStartResponse: NativeAppStartResponse = { - isColdStart: false, - appStartTime: appStartTimeMilliseconds, - didFetchAppStart: false, - }; - - mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds); - mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse); + const [timeOriginMilliseconds, appStartTimeMilliseconds] = mockAppStartResponse({ cold: false }); const mockHub = getMockHub(); integration.setupOnce(addGlobalEventProcessor, () => mockHub); @@ -173,6 +179,24 @@ describe('ReactNativeTracing', () => { } }); + it('Cancels route transaction when app goes to background', async () => { + const integration = new ReactNativeTracing(); + + mockAppStartResponse({ cold: false }); + + const mockHub = getMockHub(); + integration.setupOnce(addGlobalEventProcessor, () => mockHub); + + await jest.advanceTimersByTimeAsync(500); + const transaction = mockHub.getScope()?.getTransaction(); + + mockedAppState.setState('background'); + jest.runAllTimers(); + + expect(transaction?.status).toBe('cancelled'); + expect(mockedAppState.removeSubscription).toBeCalledTimes(1); + }); + it('Does not add app start measurement if more than 60s', async () => { const integration = new ReactNativeTracing(); @@ -212,16 +236,7 @@ describe('ReactNativeTracing', () => { it('Does not create app start transaction if didFetchAppStart == true', async () => { const integration = new ReactNativeTracing(); - const timeOriginMilliseconds = Date.now(); - const appStartTimeMilliseconds = timeOriginMilliseconds - 100; - const mockAppStartResponse: NativeAppStartResponse = { - isColdStart: true, - appStartTime: appStartTimeMilliseconds, - didFetchAppStart: true, - }; - - mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds); - mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse); + mockAppStartResponse({ cold: false, didFetchAppStart: true }); const mockHub = getMockHub(); integration.setupOnce(addGlobalEventProcessor, () => mockHub); @@ -235,22 +250,38 @@ describe('ReactNativeTracing', () => { }); describe('With routing instrumentation', () => { - it('Adds measurements and child span onto existing routing transaction and sets the op (cold)', async () => { + it('Cancels route transaction when app goes to background', async () => { const routingInstrumentation = new RoutingInstrumentation(); const integration = new ReactNativeTracing({ routingInstrumentation, }); - const timeOriginMilliseconds = Date.now(); - const appStartTimeMilliseconds = timeOriginMilliseconds - 100; - const mockAppStartResponse: NativeAppStartResponse = { - isColdStart: true, - appStartTime: appStartTimeMilliseconds, - didFetchAppStart: false, - }; + mockAppStartResponse({ cold: true }); - mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds); - mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse); + const mockHub = getMockHub(); + integration.setupOnce(addGlobalEventProcessor, () => mockHub); + // wait for internal promises to resolve, fetch app start data from mocked native + await Promise.resolve(); + + const routeTransaction = routingInstrumentation.onRouteWillChange({ + name: 'test', + }) as IdleTransaction; + + mockedAppState.setState('background'); + + jest.runAllTimers(); + + expect(routeTransaction.status).toBe('cancelled'); + expect(mockedAppState.removeSubscription).toBeCalledTimes(1); + }); + + it('Adds measurements and child span onto existing routing transaction and sets the op (cold)', async () => { + const routingInstrumentation = new RoutingInstrumentation(); + const integration = new ReactNativeTracing({ + routingInstrumentation, + }); + + const [timeOriginMilliseconds, appStartTimeMilliseconds] = mockAppStartResponse({ cold: true }); const mockHub = getMockHub(); integration.setupOnce(addGlobalEventProcessor, () => mockHub); @@ -297,16 +328,7 @@ describe('ReactNativeTracing', () => { routingInstrumentation, }); - const timeOriginMilliseconds = Date.now(); - const appStartTimeMilliseconds = timeOriginMilliseconds - 100; - const mockAppStartResponse: NativeAppStartResponse = { - isColdStart: false, - appStartTime: appStartTimeMilliseconds, - didFetchAppStart: false, - }; - - mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds); - mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse); + const [timeOriginMilliseconds, appStartTimeMilliseconds] = mockAppStartResponse({ cold: false }); const mockHub = getMockHub(); integration.setupOnce(addGlobalEventProcessor, () => mockHub); @@ -353,16 +375,7 @@ describe('ReactNativeTracing', () => { routingInstrumentation, }); - const timeOriginMilliseconds = Date.now(); - const appStartTimeMilliseconds = timeOriginMilliseconds - 100; - const mockAppStartResponse: NativeAppStartResponse = { - isColdStart: false, - appStartTime: appStartTimeMilliseconds, - didFetchAppStart: true, - }; - - mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds); - mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse); + const [, appStartTimeMilliseconds] = mockAppStartResponse({ cold: false, didFetchAppStart: true }); const mockHub = getMockHub(); integration.setupOnce(addGlobalEventProcessor, () => mockHub); @@ -641,6 +654,24 @@ describe('ReactNativeTracing', () => { expect(actualTransactionContext?.sampled).toEqual(false); }); + test('does cancel UI event transaction when app goes to background', () => { + tracing.startUserInteractionTransaction(mockedUserInteractionId); + + const actualTransaction = mockedScope.getTransaction() as Transaction | undefined; + + mockedAppState.setState('background'); + jest.runAllTimers(); + + const actualTransactionContext = actualTransaction?.toContext(); + expect(actualTransactionContext).toEqual( + expect.objectContaining({ + endTimestamp: expect.any(Number), + status: 'cancelled', + }), + ); + expect(mockedAppState.removeSubscription).toBeCalledTimes(1); + }); + test('do not overwrite existing status of UI event transactions', () => { tracing.startUserInteractionTransaction(mockedUserInteractionId); @@ -792,3 +823,18 @@ describe('ReactNativeTracing', () => { }); }); }); + +function mockAppStartResponse({ cold, didFetchAppStart }: { cold: boolean; didFetchAppStart?: boolean }) { + const timeOriginMilliseconds = Date.now(); + const appStartTimeMilliseconds = timeOriginMilliseconds - 100; + const mockAppStartResponse: NativeAppStartResponse = { + isColdStart: cold, + appStartTime: appStartTimeMilliseconds, + didFetchAppStart: didFetchAppStart ?? false, + }; + + mockFunction(getTimeOriginMilliseconds).mockReturnValue(timeOriginMilliseconds); + mockFunction(NATIVE.fetchNativeAppStart).mockResolvedValue(mockAppStartResponse); + + return [timeOriginMilliseconds, appStartTimeMilliseconds]; +}