diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index ef5635f98541..f7834e8bf996 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -446,14 +446,6 @@ export abstract class BaseClient implements Client { } 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)} (${typeof normalizeDepth})`, - }; - } if (typeof normalizeDepth === 'number' && normalizeDepth > 0) { return this._normalizeEvent(evt, normalizeDepth, normalizeMaxBreadth); } @@ -525,8 +517,6 @@ export abstract class BaseClient implements Client { }); } - normalized.sdkProcessingMetadata = { ...normalized.sdkProcessingMetadata, baseClientNormalized: true }; - return normalized; } diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 0fb974d66bc7..49077eca7042 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -73,33 +73,12 @@ export function createEventEnvelope( 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, - // 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 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, metadata && 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/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 - // TODO: This is NOT part of the hack - DO NOT DELETE + // Prevent this data (which, if it exists, was used in earlier steps in the processing pipeline) from being sent to + // sentry. (Note: Our use of this property comes and goes with whatever we might be debugging, whatever hacks we may + // have temporarily added, etc. Even if we don't happen to be using it at some point in the future, let's not get rid + // of this `delete`, lest we miss putting it back in the next time the property is in use.) delete event.sdkProcessingMetadata; const envelopeHeaders = createEventEnvelopeHeaders(event, sdkInfo, tunnel, dsn); diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 7a5431d1f9cb..742a9f0a7ae3 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -884,29 +884,9 @@ describe('BaseClient', () => { const normalizedTransaction = JSON.parse(JSON.stringify(transaction)); // deep-copy client.captureEvent(transaction); - - // 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 = TestClient.instance!.event!; - if (capturedEvent.sdkProcessingMetadata?.normalizeDepth) { - if (Object.keys(capturedEvent.sdkProcessingMetadata).length === 1) { - delete capturedEvent.sdkProcessingMetadata; - } else { - delete capturedEvent.sdkProcessingMetadata.normalizeDepth; - } - } - if (capturedEvent.sdkProcessingMetadata?.baseClientNormalized) { - if (Object.keys(capturedEvent.sdkProcessingMetadata).length === 1) { - delete capturedEvent.sdkProcessingMetadata; - } else { - delete capturedEvent.sdkProcessingMetadata.baseClientNormalized; - } - } expect(capturedEvent).toEqual(normalizedTransaction); - // expect(TestClient.instance!.event!).toEqual(normalizedTransaction); }); test('calls beforeSend and uses original event without any changes', () => { diff --git a/packages/core/test/mocks/client.ts b/packages/core/test/mocks/client.ts index 62dfa10219da..aec42264ff58 100644 --- a/packages/core/test/mocks/client.ts +++ b/packages/core/test/mocks/client.ts @@ -69,6 +69,10 @@ export class TestClient extends BaseClient { public sendEvent(event: Event, hint?: EventHint): void { this.event = event; + + // In real life, this will get deleted as part of envelope creation. + delete event.sdkProcessingMetadata; + if (this._options.enableSend) { super.sendEvent(event, hint); return; 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 index 8626cf1adf66..e86d5e11aef6 100644 --- a/packages/integration-tests/suites/public-api/configureScope/clear_scope/test.ts +++ b/packages/integration-tests/suites/public-api/configureScope/clear_scope/test.ts @@ -9,18 +9,6 @@ sentryTest('should clear previously set properties of a scope', async ({ getLoca const eventData = await getFirstSentryEnvelopeRequest(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(); diff --git a/packages/integration-tests/utils/helpers.ts b/packages/integration-tests/utils/helpers.ts index 34b512ec9e01..d2a806b81bd4 100644 --- a/packages/integration-tests/utils/helpers.ts +++ b/packages/integration-tests/utils/helpers.ts @@ -70,26 +70,7 @@ async function getMultipleRequests( if (urlRgx.test(request.url())) { try { reqCount -= 1; - - // 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 ( - Object.keys(parsedRequest.tags).length === 0 || - (Object.keys(parsedRequest.tags).length === 1 && 'skippedNormalization' in parsedRequest.tags) - ) { - 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)); + requestData.push(requestParser(request)); if (reqCount === 0) { if (timeoutId) { diff --git a/packages/node-integration-tests/suites/public-api/configureScope/clear_scope/test.ts b/packages/node-integration-tests/suites/public-api/configureScope/clear_scope/test.ts index e1cf213e2e4c..c510bc22489c 100644 --- a/packages/node-integration-tests/suites/public-api/configureScope/clear_scope/test.ts +++ b/packages/node-integration-tests/suites/public-api/configureScope/clear_scope/test.ts @@ -8,8 +8,6 @@ test('should clear previously set properties of a scope', async () => { assertSentryEvent(envelope[2], { message: 'cleared_scope', - tags: {}, - extra: {}, }); expect((envelope[2] as Event).user).not.toBeDefined(); diff --git a/packages/node-integration-tests/suites/public-api/withScope/nested-scopes/test.ts b/packages/node-integration-tests/suites/public-api/withScope/nested-scopes/test.ts index 57047696cf0c..b6399b8dd78e 100644 --- a/packages/node-integration-tests/suites/public-api/withScope/nested-scopes/test.ts +++ b/packages/node-integration-tests/suites/public-api/withScope/nested-scopes/test.ts @@ -11,7 +11,6 @@ test('should allow nested scoping', async () => { user: { id: 'qux', }, - tags: {}, }); assertSentryEvent(envelopes[3][2], { @@ -49,6 +48,5 @@ test('should allow nested scoping', async () => { user: { id: 'qux', }, - tags: {}, }); });