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:/getsentry/sentry-javascript/blob/b46674c6382be48bd6cafafa44b6e1a6b4661610/packages/core/src/baseclient.ts#L377-L379
[3] https:/getsentry/sentry-javascript/blob/b46674c6382be48bd6cafafa44b6e1a6b4661610/packages/core/src/baseclient.ts#L349

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:/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);
  }

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:/getsentry/sentry-javascript/issues/2809 and
- // https:/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:/getsentry/sentry-javascript/issues/2809,
+ // https:/getsentry/sentry-javascript/pull/4425, and
+ // https:/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

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:/getsentry/sentry-javascript/pull/4425 and
+ // https:/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', () => {

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:/getsentry/sentry-javascript/pull/4425 and
+ // https:/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();