Merge pull request #33 from contentpass/chore-add-more-details-in-sentry

chore: add more details in sentry

Author: slawomirzaba

src/Contentpass.test.ts

@@ -58,6 +58,7 @@ describe('Contentpass', () => {
   let sendStatsSpy: jest.SpyInstance;
   let sendPageViewEventSpy: jest.SpyInstance;
   let enableLoggerSpy: jest.SpyInstance;
+  let initSentrySpy: jest.SpyInstance;
 
   beforeEach(() => {
     jest.useFakeTimers({ now: NOW });
@@ -71,6 +72,10 @@ describe('Contentpass', () => {
       .spyOn(SentryIntegrationModule, 'reportError')
       .mockReturnValue(undefined);
 
+    initSentrySpy = jest
+      .spyOn(SentryIntegrationModule, 'initSentry')
+      .mockReturnValue(undefined);
+
     oidcAuthStorageMock = {
       storeOidcAuthState: jest.fn(),
       getOidcAuthState: jest.fn(),
@@ -125,6 +130,12 @@ describe('Contentpass', () => {
       ).toThrow('Sampling rate must be between 0 and 1');
     });
 
+    it('should initialise sentry', () => {
+      expect(initSentrySpy).toHaveBeenCalledWith({
+        propertyId: config.propertyId,
+      });
+    });
+
     it('should initialise contentpass state', () => {
       const contentpassStates: ContentpassState[] = [];
       contentpass.registerObserver((state) => {

src/Contentpass.ts

@@ -16,7 +16,7 @@ import { RefreshTokenStrategy } from './types/RefreshTokenStrategy';
 import fetchContentpassToken from './contentpassTokenUtils/fetchContentpassToken';
 import validateSubscription from './contentpassTokenUtils/validateSubscription';
 import type { ContentpassConfig } from './types/ContentpassConfig';
-import { reportError, setSentryExtraAttribute } from './sentryIntegration';
+import { initSentry, reportError } from './sentryIntegration';
 import sendStats from './countImpressionUtils/sendStats';
 import sendPageViewEvent from './countImpressionUtils/sendPageViewEvent';
 import logger, { enableLogger } from './logger';
@@ -62,7 +62,7 @@ export default class Contentpass implements ContentpassInterface {
     this.samplingRate = config.samplingRate || DEFAULT_SAMPLING_RATE;
     this.authStateStorage = new OidcAuthStateStorage(config.propertyId);
     this.config = config;
-    setSentryExtraAttribute('propertyId', config.propertyId);
+    initSentry({ propertyId: config.propertyId });
     this.initialiseAuthState();
   }
 

src/sentryIntegration.test.ts

@@ -1,48 +1,72 @@
-import * as Sentry from '@sentry/react-native';
 import { defaultStackParser, makeFetchTransport } from '@sentry/react';
 import * as SentryReactNativeModule from '@sentry/react-native';
-import { reportError, setSentryExtraAttribute } from './sentryIntegration';
-
-jest.mock('@sentry/react-native', () => {
-  const scope = {
-    setClient: jest.fn(),
-    addBreadcrumb: jest.fn(),
-    captureException: jest.fn(),
-    setExtra: jest.fn(),
-  };
-  return {
-    ReactNativeClient: jest.fn().mockImplementation(() => ({
-      init: jest.fn(),
-      captureException: jest.fn(),
-    })),
-    Scope: jest.fn().mockImplementation(() => scope),
+import {
+  __internal_reset_sentry_scope,
+  initSentry,
+  reportError,
+} from './sentryIntegration';
+import logger from './logger';
 
-    // Only for internal testing
-    __test_getScope: () => scope,
+jest.mock('react-native', () => {
+  return {
+    Platform: {
+      OS: 'android',
+      Version: '10.1.2',
+    },
   };
 });
 
-describe('sentryScope', () => {
+describe('sentryIntegration', () => {
   let addBreadcrumbMock: jest.Mock;
   let captureExceptionMock: jest.Mock;
+  let setTagsMock: jest.Mock;
+  let ReactNativeClientSpy: jest.SpyInstance;
+  let ScopeSpy: jest.SpyInstance;
 
   beforeEach(() => {
     addBreadcrumbMock = jest.fn();
     captureExceptionMock = jest.fn();
-    jest.spyOn(SentryReactNativeModule, 'Scope').mockReturnValue({
+    setTagsMock = jest.fn();
+    ScopeSpy = jest.spyOn(SentryReactNativeModule, 'Scope').mockReturnValue({
       setClient: jest.fn(),
+      init: jest.fn(),
       addBreadcrumb: addBreadcrumbMock,
       captureException: captureExceptionMock,
+      setTags: setTagsMock,
     } as any);
+
+    ReactNativeClientSpy = jest
+      .spyOn(SentryReactNativeModule, 'ReactNativeClient')
+      .mockImplementation(
+        () =>
+          ({
+            init: jest.fn(),
+          }) as any
+      );
+
+    jest.spyOn(logger, 'error').mockImplementation(() => {});
+    jest.spyOn(logger, 'warn').mockImplementation(() => {});
   });
 
   afterEach(() => {
     jest.restoreAllMocks();
     jest.resetAllMocks();
+    __internal_reset_sentry_scope();
+  });
+
+  it('should not initialise sentry scope if already initialised', () => {
+    initSentry({ propertyId: 'test-id' });
+    initSentry({ propertyId: 'test-id' });
+
+    expect(logger.warn).toHaveBeenCalledWith('Sentry already initialized');
+    expect(ReactNativeClientSpy).toHaveBeenCalledTimes(1);
+    expect(ScopeSpy).toHaveBeenCalledTimes(1);
   });
 
   it('should initialise sentry scope with correct options', () => {
-    expect(Sentry.ReactNativeClient).toHaveBeenCalledWith({
+    initSentry({ propertyId: 'test-id' });
+
+    expect(ReactNativeClientSpy).toHaveBeenCalledWith({
       attachStacktrace: true,
       autoInitializeNativeSdk: false,
       dsn: 'https://[email protected]/8',
@@ -57,52 +81,58 @@ describe('sentryScope', () => {
       enableStallTracking: true,
       enableUserInteractionTracing: false,
       enableWatchdogTerminationTracking: false,
+      environment: 'development',
       maxQueueSize: 30,
       parentSpanIsAlwaysRootSpan: true,
       patchGlobalPromise: true,
       sendClientReports: true,
-      integrations: [],
+      integrations: undefined,
       stackParser: defaultStackParser,
       transport: makeFetchTransport,
     });
+
+    expect(setTagsMock).toHaveBeenCalledWith({
+      OS: 'android',
+      platformVersion: '10.1.2',
+      propertyId: 'test-id',
+    });
   });
 
   describe('reportError', () => {
     it('should add breadcrumb with message if provided', () => {
+      initSentry({ propertyId: 'test-id' });
       const err = new Error('test');
       const msg = 'test message';
-      // @ts-ignore
-      const { addBreadcrumb, captureException } = Sentry.__test_getScope();
+
       reportError(err, { msg });
 
-      expect(addBreadcrumb).toHaveBeenCalledWith({
+      expect(addBreadcrumbMock).toHaveBeenCalledWith({
         category: 'Error',
         message: msg,
         level: 'log',
       });
-      expect(captureException).toHaveBeenCalledWith(err);
+      expect(captureExceptionMock).toHaveBeenCalledWith(err);
     });
 
     it('should not add breadcrumb if message is not provided', () => {
+      initSentry({ propertyId: 'test-id' });
       const err = new Error('test');
-      // @ts-ignore
-      const { addBreadcrumb, captureException } = Sentry.__test_getScope();
+
       reportError(err);
 
-      expect(addBreadcrumb).not.toHaveBeenCalled();
-      expect(captureException).toHaveBeenCalledWith(err);
+      expect(addBreadcrumbMock).not.toHaveBeenCalled();
+      expect(captureExceptionMock).toHaveBeenCalledWith(err);
     });
-  });
 
-  describe('setSentryExtraAttribute', () => {
-    it('should set extra attribute', () => {
-      const key = 'testKey';
-      const value = 'testValue';
-      // @ts-ignore
-      const { setExtra } = Sentry.__test_getScope();
-      setSentryExtraAttribute(key, value);
+    it('should not throw error when reportError is called before initSentry', () => {
+      const err = new Error('test');
+      const msg = 'test message';
+
+      reportError(err, { msg });
 
-      expect(setExtra).toHaveBeenCalledWith(key, value);
+      expect(logger.error).toHaveBeenCalledWith({ err }, msg);
+      expect(addBreadcrumbMock).not.toHaveBeenCalled();
+      expect(captureExceptionMock).not.toHaveBeenCalledWith(err);
     });
   });
 });

src/sentryIntegration.ts

@@ -2,6 +2,7 @@ import * as Sentry from '@sentry/react-native';
 import { defaultStackParser, makeFetchTransport } from '@sentry/react';
 import { getDefaultIntegrations } from '@sentry/react-native/dist/js/integrations/default';
 import logger from './logger';
+import { Platform } from 'react-native';
 
 // as it's only the open source package, we want to have minimal sentry configuration here to not override sentry instance,
 // which can be used in the application
@@ -27,24 +28,48 @@ const options = {
   stackParser: defaultStackParser,
   transport: makeFetchTransport,
   integrations: [],
+  environment: __DEV__ ? 'development' : 'production',
 };
 
-const sentryClient = new Sentry.ReactNativeClient({
-  ...options,
-  integrations: getDefaultIntegrations(options),
-});
+type InitSentryArgs = {
+  propertyId: string;
+};
+
+let sentryScope: Sentry.Scope | null = null;
+
+export const initSentry = (args: InitSentryArgs) => {
+  if (sentryScope) {
+    logger.warn('Sentry already initialized');
+    return;
+  }
+
+  const sentryClient = new Sentry.ReactNativeClient({
+    ...options,
+    integrations: getDefaultIntegrations(options),
+  });
 
-const sentryScope = new Sentry.Scope();
-sentryScope.setClient(sentryClient);
+  sentryScope = new Sentry.Scope();
+  sentryScope.setClient(sentryClient);
 
-sentryClient.init();
+  sentryClient.init();
+
+  sentryScope.setTags({
+    propertyId: args.propertyId,
+    OS: Platform.OS,
+    platformVersion: Platform.Version,
+  });
+};
 
 type ReportErrorOptions = {
   msg?: string;
 };
 
 export const reportError = (err: Error, { msg }: ReportErrorOptions = {}) => {
   logger.error({ err }, msg || 'Unexpected error');
+  if (!sentryScope) {
+    return;
+  }
+
   if (msg) {
     sentryScope.addBreadcrumb({
       category: 'Error',
@@ -56,6 +81,6 @@ export const reportError = (err: Error, { msg }: ReportErrorOptions = {}) => {
   sentryScope.captureException(err);
 };
 
-export const setSentryExtraAttribute = (key: string, value: string) => {
-  sentryScope.setExtra(key, value);
+export const __internal_reset_sentry_scope = () => {
+  sentryScope = null;
 };