ref(core): Log normalizeDepth when normalization is skipped (#4574)

Continuing debugging of #2809. In the most recent example event there[1], it's clear from the stringifying error's stacktrace that the event is hitting `BaseClient._processEvent`, and we can tell from the line number in that stack trace that it's getting past `_prepareEvent`, which is what calls `normalize`. In `_prepareEvent`, the only way for normalization not to run is if the `normalizeDepth` option either isn't a number or is set to a value <= 0.[2] (It's got a default value[3], so it not being set directly by the user isn't the issue.) In order to better diagnose what the problem might be, this PR adds logging of the `normalizeDepth` value in cases where an event fails to be normalized when it should. [1] #2809 (comment) [2] https://github.com/getsentry/sentry-javascript/blob/b46674c6382be48bd6cafafa44b6e1a6b4661610/packages/core/src/baseclient.ts#L377-L379 [3] https://github.com/getsentry/sentry-javascript/blob/b46674c6382be48bd6cafafa44b6e1a6b4661610/packages/core/src/baseclient.ts#L349
getsentry · Feb 15, 2022 · 00d359d · 00d359d
1 parent b46674c
commit 00d359d
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 6 deletions.
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -374,6 +374,11 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     }
 
     return result.then(evt => {
+      if (evt) {
+        // TODO this is more of the hack trying to solve https://github.com/getsentry/sentry-javascript/issues/2809
+        // it is only attached as extra data to the event if the event somehow skips being normalized
+        evt.sdkProcessingMetadata = { ...evt.sdkProcessingMetadata, normalizeDepth: normalize(normalizeDepth) };
+      }
       if (typeof normalizeDepth === 'number' && normalizeDepth > 0) {
         return this._normalizeEvent(evt, normalizeDepth);
       }

diff --git a/packages/core/src/request.ts b/packages/core/src/request.ts
@@ -59,23 +59,28 @@ export function eventToSentryRequest(event: Event, api: APIDetails): SentryReque
   const { method: samplingMethod, rate: sampleRate } = transactionSampling || {};
 
   // TODO: Below is a temporary hack in order to debug a serialization error - see
-  // https://github.com/getsentry/sentry-javascript/issues/2809 and
-  // https://github.com/getsentry/sentry-javascript/pull/4425. TL;DR: even though we normalize all events (which should
-  // prevent this), something is causing `JSON.stringify` to throw a circular reference error.
+  // https://github.com/getsentry/sentry-javascript/issues/2809,
+  // https://github.com/getsentry/sentry-javascript/pull/4425, and
+  // https://github.com/getsentry/sentry-javascript/pull/4574.
+  //
+  // TL; DR: even though we normalize all events (which should prevent this), something is causing `JSON.stringify` to
+  // throw a circular reference error.
   //
   // When it's time to remove it:
   // 1. Delete everything between here and where the request object `req` is created, EXCEPT the line deleting
   //    `sdkProcessingMetadata`
   // 2. Restore the original version of the request body, which is commented out
-  // 3. Search for `skippedNormalization` and pull out the companion hack in the browser playwright tests
+  // 3. Search for either of the PR URLs above and pull out the companion hacks in the browser playwright tests and the
+  //    baseClient tests in this package
   enhanceEventWithSdkInfo(event, api.metadata.sdk);
   event.tags = event.tags || {};
   event.extra = event.extra || {};
 
   // In theory, all events should be marked as having gone through normalization and so
-  // we should never set this tag
+  // we should never set this tag/extra data
   if (!(event.sdkProcessingMetadata && event.sdkProcessingMetadata.baseClientNormalized)) {
     event.tags.skippedNormalization = true;
+    event.extra.normalizeDepth = event.sdkProcessingMetadata ? event.sdkProcessingMetadata.normalizeDepth : 'unset';
   }
 
   // prevent this data from being sent to sentry

diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts
@@ -867,7 +867,21 @@ describe('BaseClient', () => {
 
       client.captureEvent(transaction);
 
-      expect(TestBackend.instance!.event!).toEqual(normalizedTransaction);
+      // TODO: This is to compensate for a temporary debugging hack which adds data the tests aren't anticipating to the
+      // event. The code can be restored to its original form (the commented-out line below) once that hack is
+      // removed. See https://github.com/getsentry/sentry-javascript/pull/4425 and
+      // https://github.com/getsentry/sentry-javascript/pull/4574
+      const capturedEvent = TestBackend.instance!.event!;
+      if (capturedEvent.sdkProcessingMetadata?.normalizeDepth) {
+        if (Object.keys(capturedEvent.sdkProcessingMetadata).length === 1) {
+          delete capturedEvent.sdkProcessingMetadata;
+        } else {
+          delete capturedEvent.sdkProcessingMetadata.normalizeDepth;
+        }
+      }
+
+      expect(capturedEvent).toEqual(normalizedTransaction);
+      // expect(TestBackend.instance!.event!).toEqual(normalizedTransaction);
     });
 
     test('calls beforeSend and uses original event without any changes', () => {

diff --git a/packages/integration-tests/suites/public-api/configureScope/clear_scope/test.ts b/packages/integration-tests/suites/public-api/configureScope/clear_scope/test.ts
@@ -8,6 +8,18 @@ sentryTest('should clear previously set properties of a scope', async ({ getLoca
 
   const eventData = await getSentryRequest(page, url);
 
+  // TODO: This is to compensate for a temporary debugging hack which adds data the tests aren't anticipating to the
+  // event. The code can be restored to its original form (the commented-out line below) once that hack is
+  // removed. See https://github.com/getsentry/sentry-javascript/pull/4425 and
+  // https://github.com/getsentry/sentry-javascript/pull/4574
+  if (eventData.extra) {
+    if (Object.keys(eventData.extra).length === 1) {
+      delete eventData.extra;
+    } else {
+      delete eventData.extra.normalizeDepth;
+    }
+  }
+
   expect(eventData.message).toBe('cleared_scope');
   expect(eventData.user).toBeUndefined();
   expect(eventData.tags).toBeUndefined();