fix unintended chagne in merging SDK info + add tests for it

Author: Lms24

packages/core/src/envelope.ts

@@ -32,22 +32,27 @@ import { showSpanDropWarning, spanToJSON } from './utils/spanUtils';
 /**
  * Apply SdkInfo (name, version, packages, integrations) to the corresponding event key.
  * Merge with existing data if any.
+ *
+ * @internal, exported only for testing
  **/
-function enhanceEventWithSdkInfo(event: Event, sdkInfo?: SdkInfo): Event {
-  if (!sdkInfo) {
+export function _enhanceEventWithSdkInfo(event: Event, newSdkInfo?: SdkInfo): Event {
+  if (!newSdkInfo) {
     return event;
   }
 
+  const eventSdkInfo = event.sdk || {};
+
   event.sdk = {
-    ...event.sdk,
-    ...sdkInfo,
-    integrations: [...(event.sdk?.integrations || []), ...(sdkInfo.integrations || [])],
-    packages: [...(event.sdk?.packages || []), ...(sdkInfo.packages || [])],
+    ...eventSdkInfo,
+    name: eventSdkInfo.name || newSdkInfo.name,
+    version: eventSdkInfo.version || newSdkInfo.version,
+    integrations: [...(event.sdk?.integrations || []), ...(newSdkInfo.integrations || [])],
+    packages: [...(event.sdk?.packages || []), ...(newSdkInfo.packages || [])],
     settings:
-      event.sdk?.settings || sdkInfo.settings
+      event.sdk?.settings || newSdkInfo.settings
         ? {
             ...event.sdk?.settings,
-            ...sdkInfo.settings,
+            ...newSdkInfo.settings,
           }
         : undefined,
   };
@@ -95,7 +100,7 @@ export function createEventEnvelope(
   */
   const eventType = event.type && event.type !== 'replay_event' ? event.type : 'event';
 
-  enhanceEventWithSdkInfo(event, metadata?.sdk);
+  _enhanceEventWithSdkInfo(event, metadata?.sdk);
 
   const envelopeHeaders = createEventEnvelopeHeaders(event, sdkInfo, tunnel, dsn);
 

packages/core/test/lib/envelope.test.ts

@@ -1,7 +1,7 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import type { DsnComponents } from '../../build/types/types-hoist/dsn';
 import type { DynamicSamplingContext } from '../../build/types/types-hoist/envelope';
-import type { Client } from '../../src';
+import type { Client, SdkInfo } from '../../src';
 import {
   getCurrentScope,
   getIsolationScope,
@@ -10,7 +10,7 @@ import {
   setAsyncContextStrategy,
   setCurrentClient,
 } from '../../src';
-import { createEventEnvelope, createSpanEnvelope } from '../../src/envelope';
+import { _enhanceEventWithSdkInfo, createEventEnvelope, createSpanEnvelope } from '../../src/envelope';
 import type { Event } from '../../src/types-hoist/event';
 import { getDefaultTestClientOptions, TestClient } from '../mocks/client';
 
@@ -261,3 +261,139 @@ describe('createSpanEnvelope', () => {
     });
   });
 });
+
+describe('_enhanceEventWithSdkInfo', () => {
+  it('does nothing if no new sdk info is provided', () => {
+    const event: Event = {
+      sdk: { name: 'original', version: '1.0.0' },
+    };
+    const enhancedEvent = _enhanceEventWithSdkInfo(event, undefined);
+    expect(enhancedEvent.sdk).toEqual({ name: 'original', version: '1.0.0' });
+  });
+
+  /**
+   * Note LS: I'm not sure if this is intended behaviour, but this is how it was before
+   * I made implementation changes for the `settings` object. Documenting behaviour for now,
+   * we can revisit it if it turns out this is not intended.
+   */
+  it('prefers original version and name over newSdkInfo', () => {
+    const event: Event = {
+      sdk: {
+        name: 'original',
+        version: '1.0.0',
+        integrations: ['integration1', 'integration2'],
+        packages: [{ name: '@sentry/browser', version: '10.0.0' }],
+      },
+    };
+    const newSdkInfo: SdkInfo = { name: 'newName', version: '2.0.0' };
+
+    const enhancedEvent = _enhanceEventWithSdkInfo(event, newSdkInfo);
+
+    expect(enhancedEvent.sdk).toEqual({
+      name: 'original',
+      version: '1.0.0',
+      integrations: ['integration1', 'integration2'],
+      packages: [{ name: '@sentry/browser', version: '10.0.0' }],
+    });
+  });
+
+  describe('integrations and packages', () => {
+    it('merges integrations and packages of original and newSdkInfo', () => {
+      const event: Event = {
+        sdk: {
+          name: 'original',
+          version: '1.0.0',
+          integrations: ['integration1', 'integration2'],
+          packages: [{ name: '@sentry/browser', version: '10.0.0' }],
+        },
+      };
+
+      const newSdkInfo: SdkInfo = {
+        name: 'newName',
+        version: '2.0.0',
+        integrations: ['integration3', 'integration4'],
+        packages: [{ name: '@sentry/node', version: '11.0.0' }],
+      };
+
+      const enhancedEvent = _enhanceEventWithSdkInfo(event, newSdkInfo);
+
+      expect(enhancedEvent.sdk).toEqual({
+        name: 'original',
+        version: '1.0.0',
+        integrations: ['integration1', 'integration2', 'integration3', 'integration4'],
+        packages: [
+          { name: '@sentry/browser', version: '10.0.0' },
+          { name: '@sentry/node', version: '11.0.0' },
+        ],
+      });
+    });
+
+    it('creates empty integrations and packages arrays if no original or newSdkInfo are provided', () => {
+      const event: Event = {
+        sdk: {
+          name: 'original',
+          version: '1.0.0',
+        },
+      };
+
+      const newSdkInfo: SdkInfo = {};
+
+      const enhancedEvent = _enhanceEventWithSdkInfo(event, newSdkInfo);
+      expect(enhancedEvent.sdk).toEqual({
+        name: 'original',
+        version: '1.0.0',
+        integrations: [],
+        packages: [],
+      });
+    });
+  });
+
+  describe('settings', () => {
+    it('prefers newSdkInfo settings over original settings', () => {
+      const event: Event = {
+        sdk: {
+          name: 'original',
+          version: '1.0.0',
+          integrations: ['integration1', 'integration2'],
+          packages: [{ name: '@sentry/browser', version: '10.0.0' }],
+          settings: { infer_ip: 'auto' },
+        },
+      };
+      const newSdkInfo: SdkInfo = {
+        settings: { infer_ip: 'never' },
+      };
+
+      const enhancedEvent = _enhanceEventWithSdkInfo(event, newSdkInfo);
+
+      expect(enhancedEvent.sdk).toEqual({
+        name: 'original',
+        version: '1.0.0',
+        integrations: ['integration1', 'integration2'],
+        packages: [{ name: '@sentry/browser', version: '10.0.0' }],
+        settings: { infer_ip: 'never' },
+      });
+    });
+
+    it("doesn't create a `settings` object if no settings are provided", () => {
+      const event: Event = {
+        sdk: {
+          name: 'original',
+          version: '1.0.0',
+        },
+      };
+
+      const newSdkInfo: SdkInfo = {
+        packages: [{ name: '@sentry/browser', version: '10.0.0' }],
+      };
+
+      const enhancedEvent = _enhanceEventWithSdkInfo(event, newSdkInfo);
+      expect(enhancedEvent.sdk).toEqual({
+        name: 'original',
+        version: '1.0.0',
+        packages: [{ name: '@sentry/browser', version: '10.0.0' }],
+        integrations: [],
+        settings: undefined, // undefined is fine because JSON.stringify omits undefined values anyways
+      });
+    });
+  });
+});