Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Report as app context what's the current screen during the error #3339

Merged
merged 12 commits into from
Nov 13, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Features

- Current screen is now repoted on the error's app context ([#3339](https://github.com/getsentry/sentry-react-native/pull/3339))
- Export New JS Performance API ([#3371](https://github.com/getsentry/sentry-react-native/pull/3371))

```js
Expand Down
3 changes: 3 additions & 0 deletions src/js/integrations/devicecontext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ export class DeviceContext implements Integration {
}
if (nativeContexts) {
event.contexts = { ...nativeContexts, ...event.contexts };
if (nativeContexts.app) {
event.contexts.app = { ...nativeContexts.app, ...event.contexts.app };
}
}

const nativeTags = native.tags;
Expand Down
27 changes: 26 additions & 1 deletion src/js/tracing/reactnativetracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@ import type { RequestInstrumentationOptions } from '@sentry/browser';
import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from '@sentry/browser';
import type { Hub, IdleTransaction, Transaction } from '@sentry/core';
import { getActiveTransaction, getCurrentHub, startIdleTransaction } from '@sentry/core';
import type { EventProcessor, Integration, Transaction as TransactionType, TransactionContext } from '@sentry/types';
import type {
Event,
EventProcessor,
Integration,
Transaction as TransactionType,
TransactionContext,
} from '@sentry/types';
import { logger } from '@sentry/utils';

import { APP_START_COLD, APP_START_WARM } from '../measurements';
Expand Down Expand Up @@ -140,6 +146,7 @@ export class ReactNativeTracing implements Integration {
private _currentRoute?: string;
private _hasSetTracePropagationTargets: boolean;
private _hasSetTracingOrigins: boolean;
private _currentViewName: string | undefined;

public constructor(options: Partial<ReactNativeTracingOptions> = {}) {
this._hasSetTracePropagationTargets = !!(
Expand Down Expand Up @@ -255,6 +262,9 @@ export class ReactNativeTracing implements Integration {
logger.log('[ReactNativeTracing] Not instrumenting route changes as routingInstrumentation has not been set.');
}

this._getCurrentViewEventProcessor = this._getCurrentViewEventProcessor.bind(this);
addGlobalEventProcessor(async (event: Event) => this._getCurrentViewEventProcessor(event));
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved

instrumentOutgoingRequests({
traceFetch,
traceXHR,
Expand Down Expand Up @@ -350,6 +360,17 @@ export class ReactNativeTracing implements Integration {
return this._inflightInteractionTransaction;
}

/**
* Sets the current view name into the app context.
* @param event Le event.
*/
private _getCurrentViewEventProcessor(event: Event): Event {
if (event.contexts && this._currentViewName) {
event.contexts.app = { ...{ view_names: [this._currentViewName] }, ...event.contexts.app };
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved
}
return event;
}

/**
* Returns the App Start Duration in Milliseconds. Also returns undefined if not able do
* define the duration.
Expand Down Expand Up @@ -455,6 +476,10 @@ export class ReactNativeTracing implements Integration {
});
}

this._currentViewName = context.name;
/**
* @deprecated tag routing.route.name will be removed in the future.
*/
scope.setTag('routing.route.name', context.name);
});
}
Expand Down
41 changes: 41 additions & 0 deletions test/integrations/devicecontext.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,47 @@ describe('Device Context Integration', () => {
).expectEvent.toStrictEqualMockEvent();
});

it('do not overwrite event app context', async () => {
(
await executeIntegrationWith({
nativeContexts: { app: { view_names: ['native view'] } },
mockEvent: { contexts: { app: { view_names: ['Home'] } } },
})
).expectEvent.toStrictEqualMockEvent();
});

it('merge event context app', async () => {
const { processedEvent } = await executeIntegrationWith({
nativeContexts: { contexts: { app: { native: 'value' } } },
mockEvent: { contexts: { app: { event_app: 'value' } } },
});
expect(processedEvent).toStrictEqual({
contexts: {
app: {
event_app: 'value',
native: 'value',
},
},
});
});

it('merge event context app even when event app doesnt exist', async () => {
const { processedEvent } = await executeIntegrationWith({
nativeContexts: { contexts: { app: { native: 'value' } } },
mockEvent: { contexts: { keyContext: { key: 'value' } } },
});
expect(processedEvent).toStrictEqual({
contexts: {
keyContext: {
key: 'value',
},
app: {
native: 'value',
},
},
});
});

it('merge event and native contexts', async () => {
const { processedEvent } = await executeIntegrationWith({
nativeContexts: { contexts: { duplicate: { context: 'native-value' }, native: { context: 'value' } } },
Expand Down
3 changes: 3 additions & 0 deletions test/tracing/gesturetracing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ const getMockScope = () => {
setTag(_tag: unknown) {
// Placeholder
},
setContext(_context: unknown) {
// Placeholder
},
addBreadcrumb(_breadcrumb: unknown) {
// Placeholder
},
Expand Down
74 changes: 71 additions & 3 deletions test/tracing/reactnativetracing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ const getMockScope = () => {
setTag(_tag: any) {
// Placeholder
},
setContext(_context: any) {
// Placeholder
},
addBreadcrumb(_breadcrumb: any) {
// Placeholder
},
Expand Down Expand Up @@ -95,7 +98,7 @@ const getMockHub = () => {
return mockHub;
};

import type { Scope } from '@sentry/types';
import type { Event, Scope } from '@sentry/types';
import type { AppState, AppStateStatus } from 'react-native';

import { APP_START_COLD, APP_START_WARM } from '../../src/js/measurements';
Expand Down Expand Up @@ -678,15 +681,16 @@ describe('ReactNativeTracing', () => {

describe('Routing Instrumentation', () => {
describe('_onConfirmRoute', () => {
it('Sets tag and adds breadcrumb', () => {
it('Sets app context, tag and adds breadcrumb', () => {
const routing = new RoutingInstrumentation();
const integration = new ReactNativeTracing({
routingInstrumentation: routing,
});

let mockEvent: Event | null = { contexts: {} };
const mockScope = {
addBreadcrumb: jest.fn(),
setTag: jest.fn(),
setContext: jest.fn(),

// Not relevant to test
setSpan: () => {},
Expand Down Expand Up @@ -724,6 +728,20 @@ describe('ReactNativeTracing', () => {
};
routing.onRouteWillChange(routeContext);

mockEvent = integration['_getCurrentViewEventProcessor'](mockEvent);

if (!mockEvent) {
throw new Error('mockEvent was not defined');
}
expect(mockEvent.contexts?.app).toBeDefined();
// Only required to mark app as defined.
if (mockEvent.contexts?.app) {
expect(mockEvent.contexts.app['view_names']).toEqual([routeContext.name]);
}

/**
* @deprecated tag routing.route.name will be removed in the future.
*/
expect(mockScope.setTag).toBeCalledWith('routing.route.name', routeContext.name);
expect(mockScope.addBreadcrumb).toBeCalledWith({
type: 'navigation',
Expand All @@ -735,6 +753,56 @@ describe('ReactNativeTracing', () => {
},
});
});

describe('View Names event processor', () => {
it('Do not overwrite event app context', () => {
const routing = new RoutingInstrumentation();
const integration = new ReactNativeTracing({
routingInstrumentation: routing,
});

const expectedRouteName = 'Route';
const event: Event = { contexts: { app: { appKey: 'value' } } };
const expectedEvent: Event = { contexts: { app: { appKey: 'value', view_names: [expectedRouteName] } } };

// @ts-expect-error only for testing.
integration._currentViewName = expectedRouteName;
const processedEvent = integration['_getCurrentViewEventProcessor'](event);

expect(processedEvent).toEqual(expectedEvent);
});

it('Do not add view_names if context is undefined', () => {
const routing = new RoutingInstrumentation();
const integration = new ReactNativeTracing({
routingInstrumentation: routing,
});

const expectedRouteName = 'Route';
const event: Event = { release: 'value' };
const expectedEvent: Event = { release: 'value' };

// @ts-expect-error only for testing.
integration._currentViewName = expectedRouteName;
const processedEvent = integration['_getCurrentViewEventProcessor'](event);

expect(processedEvent).toEqual(expectedEvent);
});

it('ignore view_names if undefined', () => {
const routing = new RoutingInstrumentation();
const integration = new ReactNativeTracing({
routingInstrumentation: routing,
});

const event: Event = { contexts: { app: { key: 'value ' } } };
const expectedEvent: Event = { contexts: { app: { key: 'value ' } } };

const processedEvent = integration['_getCurrentViewEventProcessor'](event);

expect(processedEvent).toEqual(expectedEvent);
});
});
});
});
describe('Handling deprecated options', () => {
Expand Down
Loading