fix(core): Ensure normalizedRequest on sdkProcessingMetadata is merged

Author: mydea

packages/core/src/scope.ts

@@ -24,6 +24,7 @@ import type {
 import { dateTimestampInSeconds, generatePropagationContext, isPlainObject, logger, uuid4 } from '@sentry/utils';
 
 import { updateSession } from './session';
+import { mergeSdkProcessingMetadata } from './utils/applyScopeDataToEvent';
 import { _getSpanForScope, _setSpanForScope } from './utils/spanOnScope';
 
 /**
@@ -479,8 +480,7 @@ class ScopeClass implements ScopeInterface {
    * @inheritDoc
    */
   public setSDKProcessingMetadata(newData: { [key: string]: unknown }): this {
-    this._sdkProcessingMetadata = { ...this._sdkProcessingMetadata, ...newData };
-
+    this._sdkProcessingMetadata = mergeSdkProcessingMetadata(this._sdkProcessingMetadata, newData);
     return this;
   }
 

packages/core/src/utils/applyScopeDataToEvent.ts

@@ -46,7 +46,8 @@ export function mergeScopeData(data: ScopeData, mergeData: ScopeData): void {
   mergeAndOverwriteScopeData(data, 'tags', tags);
   mergeAndOverwriteScopeData(data, 'user', user);
   mergeAndOverwriteScopeData(data, 'contexts', contexts);
-  mergeAndOverwriteScopeData(data, 'sdkProcessingMetadata', sdkProcessingMetadata);
+
+  data.sdkProcessingMetadata = mergeSdkProcessingMetadata(data.sdkProcessingMetadata, sdkProcessingMetadata);
 
   if (level) {
     data.level = level;
@@ -115,6 +116,35 @@ export function mergeArray<Prop extends 'breadcrumbs' | 'fingerprint'>(
   event[prop] = merged.length ? merged : undefined;
 }
 
+/**
+ * Merge new SDK processing metadata into existing data.
+ * New data will overwrite existing data.
+ * `normalizedRequest` is special handled and will also be merged.
+ */
+export function mergeSdkProcessingMetadata(
+  sdkProcessingMetadata: ScopeData['sdkProcessingMetadata'],
+  newSdkProcessingMetadata: ScopeData['sdkProcessingMetadata'],
+): ScopeData['sdkProcessingMetadata'] {
+  // We want to merge `normalizedRequest` to avoid some partial entry on the scope
+  // overwriting potentially more complete data on the isolation scope
+  const normalizedRequestBefore = sdkProcessingMetadata['normalizedRequest'];
+  const normalizedRequest = newSdkProcessingMetadata['normalizedRequest'];
+
+  const newData = {
+    ...sdkProcessingMetadata,
+    ...newSdkProcessingMetadata,
+  };
+
+  if (normalizedRequestBefore || normalizedRequest) {
+    newData['normalizedRequest'] = {
+      ...(normalizedRequestBefore || {}),
+      ...(normalizedRequest || {}),
+    };
+  }
+
+  return newData;
+}
+
 function applyDataToEvent(event: Event, data: ScopeData): void {
   const { extra, tags, user, contexts, level, transactionName } = data;
 

packages/core/test/lib/scope.test.ts

@@ -204,10 +204,47 @@ describe('Scope', () => {
       expect(scope['_user']).toEqual({});
     });
 
-    test('setProcessingMetadata', () => {
-      const scope = new Scope();
-      scope.setSDKProcessingMetadata({ dogs: 'are great!' });
-      expect(scope['_sdkProcessingMetadata'].dogs).toEqual('are great!');
+    describe('setProcessingMetadata', () => {
+      test('it works with no initial data', () => {
+        const scope = new Scope();
+        scope.setSDKProcessingMetadata({ dogs: 'are great!' });
+        expect(scope['_sdkProcessingMetadata'].dogs).toEqual('are great!');
+      });
+
+      test('it overwrites arbitrary data', () => {
+        const scope = new Scope();
+        scope.setSDKProcessingMetadata({ dogs: 'are great!' });
+        scope.setSDKProcessingMetadata({ dogs: 'are really great!' });
+        scope.setSDKProcessingMetadata({ cats: 'are also great!' });
+        scope.setSDKProcessingMetadata({ obj: { nested: 'value1' } });
+        scope.setSDKProcessingMetadata({ obj: { nested2: 'value2' } });
+
+        expect(scope['_sdkProcessingMetadata']).toEqual({
+          dogs: 'are really great!',
+          cats: 'are also great!',
+          obj: { nested2: 'value2' },
+        });
+      });
+
+      test('it merges normalizedRequest data', () => {
+        const scope = new Scope();
+        scope.setSDKProcessingMetadata({
+          normalizedRequest: {
+            url: 'value1',
+            method: 'value1',
+          },
+        });
+        scope.setSDKProcessingMetadata({
+          normalizedRequest: {
+            url: 'value2',
+            headers: {},
+          },
+        });
+
+        expect(scope['_sdkProcessingMetadata']).toEqual({
+          normalizedRequest: { url: 'value2', method: 'value1', headers: {} },
+        });
+      });
     });
 
     test('set and get propagation context', () => {

packages/core/test/lib/utils/applyScopeDataToEvent.test.ts

@@ -5,6 +5,7 @@ import {
   mergeAndOverwriteScopeData,
   mergeArray,
   mergeScopeData,
+  mergeSdkProcessingMetadata,
 } from '../../../src/utils/applyScopeDataToEvent';
 
 describe('mergeArray', () => {
@@ -134,7 +135,15 @@ describe('mergeScopeData', () => {
       contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } },
       attachments: [attachment1],
       propagationContext: { spanId: '1', traceId: '1' },
-      sdkProcessingMetadata: { aa: 'aa', bb: 'aa' },
+      sdkProcessingMetadata: {
+        aa: 'aa',
+        bb: 'aa',
+        obj: { key: 'value' },
+        normalizedRequest: {
+          url: 'oldUrl',
+          method: 'oldMethod',
+        },
+      },
       fingerprint: ['aa', 'bb'],
     };
     const data2: ScopeData = {
@@ -146,7 +155,17 @@ describe('mergeScopeData', () => {
       contexts: { os: { name: 'os2' } },
       attachments: [attachment2, attachment3],
       propagationContext: { spanId: '2', traceId: '2' },
-      sdkProcessingMetadata: { bb: 'bb', cc: 'bb' },
+      sdkProcessingMetadata: {
+        bb: 'bb',
+        cc: 'bb',
+        // Regular objects are not deep merged
+        obj: { key2: 'value2' },
+        // Only normalizedRequest is deep merged
+        normalizedRequest: {
+          url: 'newUrl',
+          headers: {},
+        },
+      },
       fingerprint: ['cc'],
     };
     mergeScopeData(data1, data2);
@@ -159,12 +178,136 @@ describe('mergeScopeData', () => {
       contexts: { os: { name: 'os2' }, culture: { display_name: 'name1' } },
       attachments: [attachment1, attachment2, attachment3],
       propagationContext: { spanId: '2', traceId: '2' },
-      sdkProcessingMetadata: { aa: 'aa', bb: 'bb', cc: 'bb' },
+      sdkProcessingMetadata: {
+        aa: 'aa',
+        bb: 'bb',
+        cc: 'bb',
+        obj: { key2: 'value2' },
+        normalizedRequest: {
+          url: 'newUrl',
+          method: 'oldMethod',
+          headers: {},
+        },
+      },
       fingerprint: ['aa', 'bb', 'cc'],
     });
   });
 });
 
+describe('mergeSdkProcessingMetadata', () => {
+  it('works with empty objects', () => {
+    const oldData = {};
+    const newData = {};
+
+    const actual = mergeSdkProcessingMetadata(oldData, newData);
+
+    expect(actual).toEqual({});
+    expect(actual).not.toBe(oldData);
+    expect(actual).not.toBe(newData);
+  });
+
+  it('works with arbitrary data', () => {
+    const oldData = {
+      old1: 'old1',
+      old2: 'old2',
+      obj: { key: 'value' },
+    };
+    const newData = {
+      new1: 'new1',
+      old2: 'new2',
+      obj: { key2: 'value2' },
+    };
+
+    const actual = mergeSdkProcessingMetadata(oldData, newData);
+
+    expect(actual).toEqual({
+      old1: 'old1',
+      old2: 'new2',
+      new1: 'new1',
+      obj: { key2: 'value2' },
+    });
+    expect(actual).not.toBe(oldData);
+    expect(actual).not.toBe(newData);
+  });
+
+  it('merges normalizedRequest', () => {
+    const oldData = {
+      old1: 'old1',
+      normalizedRequest: {
+        url: 'oldUrl',
+        method: 'oldMethod',
+      },
+    };
+    const newData = {
+      new1: 'new1',
+      normalizedRequest: {
+        url: 'newUrl',
+        headers: {},
+      },
+    };
+
+    const actual = mergeSdkProcessingMetadata(oldData, newData);
+
+    expect(actual).toEqual({
+      old1: 'old1',
+      new1: 'new1',
+      normalizedRequest: {
+        url: 'newUrl',
+        method: 'oldMethod',
+        headers: {},
+      },
+    });
+  });
+
+  it('keeps existing normalizedRequest', () => {
+    const oldData = {
+      old1: 'old1',
+      normalizedRequest: {
+        url: 'oldUrl',
+        method: 'oldMethod',
+      },
+    };
+    const newData = {
+      new1: 'new1',
+    };
+
+    const actual = mergeSdkProcessingMetadata(oldData, newData);
+
+    expect(actual).toEqual({
+      old1: 'old1',
+      new1: 'new1',
+      normalizedRequest: {
+        url: 'oldUrl',
+        method: 'oldMethod',
+      },
+    });
+  });
+
+  it('adds new normalizedRequest', () => {
+    const oldData = {
+      old1: 'old1',
+    };
+    const newData = {
+      new1: 'new1',
+      normalizedRequest: {
+        url: 'newUrl',
+        method: 'newMethod',
+      },
+    };
+
+    const actual = mergeSdkProcessingMetadata(oldData, newData);
+
+    expect(actual).toEqual({
+      old1: 'old1',
+      new1: 'new1',
+      normalizedRequest: {
+        url: 'newUrl',
+        method: 'newMethod',
+      },
+    });
+  });
+});
+
 describe('applyScopeDataToEvent', () => {
   it("doesn't apply scope.transactionName to transaction events", () => {
     const data: ScopeData = {

packages/types/src/scope.ts

@@ -217,7 +217,9 @@ export interface Scope {
   clearAttachments(): this;
 
   /**
-   * Add data which will be accessible during event processing but won't get sent to Sentry
+   * Add data which will be accessible during event processing but won't get sent to Sentry.
+   *
+   * TODO(v9): We should type this stricter, so that e.g. `normalizedRequest` is strictly typed.
    */
   setSDKProcessingMetadata(newData: { [key: string]: unknown }): this;