fix(envelope): Remove duplicate sdk package record from envelope (#2570)

Author: krystofwoldrich

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## 4.7.0
 
+### Fixes
+
+- Remove duplicate sdk package record from envelope ([#2570](https://github.com/getsentry/sentry-react-native/pull/2570))
+
 ### Dependencies
 
 - Bump Android SDK from v6.4.3 to v6.5.0 ([#2535](https://github.com/getsentry/sentry-react-native/pull/2535))

src/js/integrations/sdkinfo.ts

@@ -57,7 +57,6 @@ export class SdkInfo implements Integration {
         packages: [
           ...((event.sdk && event.sdk.packages) || []),
           ...((this._nativeSdkInfo && [this._nativeSdkInfo]) || []),
-          ...defaultSdkInfo.packages,
         ],
       };
 

test/client.test.ts

@@ -1,11 +1,11 @@
-import { Envelope, Outcome, Transport } from '@sentry/types';
+import { Envelope, Event,Outcome, Transport } from '@sentry/types';
 import { rejectedSyncPromise, SentryError } from '@sentry/utils';
 import * as RN from 'react-native';
 
 import { ReactNativeClient } from '../src/js/client';
 import { ReactNativeClientOptions, ReactNativeOptions } from '../src/js/options';
 import { NativeTransport } from '../src/js/transports/native';
-import { SDK_NAME, SDK_VERSION } from '../src/js/version';
+import { SDK_NAME, SDK_PACKAGE_NAME, SDK_VERSION } from '../src/js/version';
 import { NATIVE } from '../src/js/wrapper';
 import {
   envelopeHeader,
@@ -134,8 +134,7 @@ describe('Tests ReactNativeClient', () => {
     });
 
     test('use custom transport function', async () => {
-      // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      const mySend = (request: Envelope) => Promise.resolve();
+      const mySend = (_request: Envelope) => Promise.resolve();
       const myFlush = (timeout?: number) => Promise.resolve(Boolean(timeout));
       const myCustomTransportFn = (): Transport => ({
         send: mySend,
@@ -298,6 +297,32 @@ describe('Tests ReactNativeClient', () => {
     });
   });
 
+  describe('event data enhancement', () => {
+    test('event contains sdk default information', async () => {
+      const mockedSend = jest.fn<PromiseLike<void>, [Envelope]>();
+      const mockedTransport = (): Transport => ({
+        send: mockedSend,
+        flush: jest.fn().mockResolvedValue(true),
+      });
+      const client = new ReactNativeClient(<ReactNativeClientOptions> {
+        ...DEFAULT_OPTIONS,
+        dsn: EXAMPLE_DSN,
+        transport: mockedTransport,
+      });
+
+      client.captureEvent({ message: 'test event' });
+
+      expect(mockedSend).toBeCalled();
+      const actualEvent: Event | undefined = <Event>mockedSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload];
+      expect(actualEvent?.sdk?.packages).toEqual([
+        {
+          name: SDK_PACKAGE_NAME,
+          version: SDK_VERSION,
+        },
+      ]);
+    });
+  });
+
   describe('clientReports', () => {
     test('does not send client reports if disabled', () => {
       const mockTransportSend = jest.fn((_envelope: Envelope) => Promise.resolve());

test/integrations/sdkinfo.test.ts

@@ -1,8 +1,10 @@
-import { Event } from '@sentry/types';
+import { Event, EventHint, Package } from '@sentry/types';
 
 import { SdkInfo } from '../../src/js/integrations';
 import { NATIVE } from '../../src/js/wrapper';
 
+let mockedFetchNativeSdkInfo: jest.Mock<PromiseLike<Package | null>, []>;
+
 const mockPackage = {
   name: 'sentry-cocoa',
   version: '0.0.1',
@@ -15,7 +17,7 @@ jest.mock('../../src/js/wrapper', () => {
     NATIVE: {
       ...actual.NATIVE,
       platform: 'ios',
-      fetchNativeSdkInfo: jest.fn(() => Promise.resolve(mockPackage)),
+      fetchNativeSdkInfo: () => mockedFetchNativeSdkInfo(),
     },
   };
 });
@@ -25,72 +27,48 @@ afterEach(() => {
 });
 
 describe('Sdk Info', () => {
-  it('Adds native package and javascript platform to event on iOS', (done) => {
-    const integration = new SdkInfo();
-
+  it('Adds native package and javascript platform to event on iOS', async () => {
+    mockedFetchNativeSdkInfo = jest.fn().mockResolvedValue(mockPackage);
     const mockEvent: Event = {};
+    const processedEvent = await executeIntegrationFor(mockEvent);
 
-    integration.setupOnce(async (eventProcessor) => {
-      try {
-        const processedEvent = await eventProcessor(mockEvent);
-
-        expect(processedEvent).toBeDefined();
-        if (processedEvent) {
-          expect(processedEvent.sdk).toBeDefined();
-          if (processedEvent.sdk) {
-            expect(processedEvent.sdk.packages).toBeDefined();
-            if (processedEvent.sdk.packages) {
-              expect(
-                processedEvent.sdk.packages.some(
-                  (pkg) =>
-                    pkg.name === mockPackage.name &&
-                    pkg.version === mockPackage.version
-                )
-              ).toBe(true);
-            }
-          }
-          expect(processedEvent.platform === 'javascript');
-        }
-
-        done();
-      } catch (e) {
-        done(e);
-      }
-    });
+    expect(processedEvent?.sdk?.packages).toEqual(expect.arrayContaining([mockPackage]));
+    expect(processedEvent?.platform === 'javascript');
+    expect(mockedFetchNativeSdkInfo).toBeCalledTimes(1);
   });
 
-  it('Adds javascript platform but not native package on Android', (done) => {
+  it('Adds javascript platform but not native package on Android', async () => {
     NATIVE.platform = 'android';
-    const integration = new SdkInfo();
+    mockedFetchNativeSdkInfo = jest.fn().mockResolvedValue(mockPackage);
+    const mockEvent: Event = {};
+    const processedEvent = await executeIntegrationFor(mockEvent);
+
+    expect(processedEvent?.sdk?.packages).toEqual(expect.not.arrayContaining([mockPackage]));
+    expect(processedEvent?.platform === 'javascript');
+    expect(mockedFetchNativeSdkInfo).not.toBeCalled();
+  });
 
+  it('Does not add any default non native packages', async () => {
+    mockedFetchNativeSdkInfo = jest.fn().mockResolvedValue(null);
     const mockEvent: Event = {};
+    const processedEvent = await executeIntegrationFor(mockEvent);
 
+    expect(processedEvent?.sdk?.packages).toEqual([]);
+    expect(processedEvent?.platform === 'javascript');
+    expect(mockedFetchNativeSdkInfo).toBeCalledTimes(1);
+  });
+});
+
+function executeIntegrationFor(mockedEvent: Event, mockedHint: EventHint = {}): Promise<Event | null> {
+  const integration = new SdkInfo();
+  return new Promise((resolve, reject) => {
     integration.setupOnce(async (eventProcessor) => {
       try {
-        const processedEvent = await eventProcessor(mockEvent);
-
-        expect(processedEvent).toBeDefined();
-        if (processedEvent) {
-          expect(processedEvent.sdk).toBeDefined();
-          if (processedEvent.sdk) {
-            expect(processedEvent.sdk.packages).toBeDefined();
-            if (processedEvent.sdk.packages) {
-              expect(
-                processedEvent.sdk.packages.some(
-                  (pkg) =>
-                    pkg.name === mockPackage.name &&
-                    pkg.version === mockPackage.version
-                )
-              ).toBe(false);
-            }
-          }
-          expect(processedEvent.platform === 'javascript');
-        }
-
-        done();
+        const processedEvent = await eventProcessor(mockedEvent, mockedHint);
+        resolve(processedEvent);
       } catch (e) {
-        done(e);
+        reject(e);
       }
     });
   });
-});
+}