ref(core): Renormalize event only after stringification errors

packages/core/src/baseclient.ts

@@ -428,10 +428,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
       normalized.contexts.trace = event.contexts.trace;
     }
 
-    const { _experiments = {} } = this.getOptions();
-    if (_experiments.ensureNoCircularStructures) {
-      return normalize(normalized);
-    }
+    event.sdkProcessingMetadata = { ...event.sdkProcessingMetadata, baseClientNormalized: true };
 
     return normalized;
   }

packages/core/src/request.ts

@@ -1,5 +1,5 @@
 import { Event, SdkInfo, SentryRequest, SentryRequestType, Session, SessionAggregates } from '@sentry/types';
-import { dsnToString } from '@sentry/utils';
+import { dsnToString, normalize } from '@sentry/utils';
 
 import { APIDetails, getEnvelopeEndpointWithUrlEncodedAuth, getStoreEndpointWithUrlEncodedAuth } from './api';
 
@@ -58,11 +58,58 @@ export function eventToSentryRequest(event: Event, api: APIDetails): SentryReque
   const { transactionSampling } = event.sdkProcessingMetadata || {};
   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.
+  //
+  // 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
+  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
+  if (!(event.sdkProcessingMetadata && event.sdkProcessingMetadata.baseClientNormalized)) {
+    event.tags.skippedNormalization = true;
+  }
+
   // prevent this data from being sent to sentry
+  // TODO: This is NOT part of the hack - DO NOT DELETE
   delete event.sdkProcessingMetadata;
 
+  let body;
+  try {
+    // 99.9% of events should get through just fine - no change in behavior for them
+    body = JSON.stringify(event);
+  } catch (err) {
+    // Record data about the error without replacing original event data, then force renormalization
+    event.tags.JSONStringifyError = true;
+    event.extra.JSONStringifyError = err;
+    try {
+      body = JSON.stringify(normalize(event));
+    } catch (newErr) {
+      // At this point even renormalization hasn't worked, meaning something about the event data has gone very wrong.
+      // Time to cut our losses and record only the new error. With luck, even in the problematic cases we're trying to
+      // debug with this hack, we won't ever land here.
+      const innerErr = newErr as Error;
+      body = JSON.stringify({
+        message: 'JSON.stringify error after renormalization',
+        // setting `extra: { innerErr }` here for some reason results in an empty object, so unpack manually
+        extra: { message: innerErr.message, stack: innerErr.stack },
+      });
+    }
+  }
+
   const req: SentryRequest = {
-    body: JSON.stringify(sdkInfo ? enhanceEventWithSdkInfo(event, api.metadata.sdk) : event),
+    // this is the relevant line of code before the hack was added, to make it easy to undo said hack once we've solved
+    // the mystery
+    // body: JSON.stringify(sdkInfo ? enhanceEventWithSdkInfo(event, api.metadata.sdk) : event),
+    body,
     type: eventType,
     url: useEnvelope
       ? getEnvelopeEndpointWithUrlEncodedAuth(api.dsn, api.tunnel)

packages/integration-tests/utils/helpers.ts

@@ -87,7 +87,24 @@ async function getMultipleRequests(
       if (urlRgx.test(request.url())) {
         try {
           reqCount -= 1;
-          requestData.push(requestParser(request));
+
+          // TODO: This is to compensate for a temporary debugging hack which adds data the tests aren't anticipating to
+          // the request. 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.
+          const parsedRequest = requestParser(request);
+          if (parsedRequest.tags) {
+            if (parsedRequest.tags.skippedNormalization && Object.keys(parsedRequest.tags).length === 1) {
+              delete parsedRequest.tags;
+            } else {
+              delete parsedRequest.tags.skippedNormalization;
+            }
+          }
+          if (parsedRequest.extra && Object.keys(parsedRequest.extra).length === 0) {
+            delete parsedRequest.extra;
+          }
+          requestData.push(parsedRequest);
+          // requestData.push(requestParser(request));
+
           if (reqCount === 0) {
             resolve(requestData);
           }