From 34ce5a2d5b5d01923693e37325fc40c696325a3f Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Wed, 14 Jan 2026 10:52:40 +0100 Subject: [PATCH 1/4] fix(core): Don't record outcomes for failed client reports --- packages/core/src/transports/base.ts | 5 ++ .../core/test/lib/transports/base.test.ts | 89 ++++++++++++++++++- 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index d85d9305bbe9..372c27c6b275 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -10,6 +10,7 @@ import type { import { debug } from '../utils/debug-logger'; import { createEnvelope, + envelopeContainsItemType, envelopeItemTypeToDataCategory, forEachEnvelopeItem, serializeEnvelope, @@ -57,6 +58,10 @@ export function createTransport( // Creates client report for each item in an envelope const recordEnvelopeLoss = (reason: EventDropReason): void => { + // Don't record outcomes for client reports - we don't want to create a feedback loop if client reports themselves fail to send + if (envelopeContainsItemType(filteredEnvelope, ['client_report'])) { + return; + } forEachEnvelopeItem(filteredEnvelope, (item, type) => { options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type)); }); diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts index ef2220ac1f8b..df11d0fafc29 100644 --- a/packages/core/test/lib/transports/base.test.ts +++ b/packages/core/test/lib/transports/base.test.ts @@ -1,9 +1,11 @@ import { describe, expect, it, vi } from 'vitest'; import { createTransport } from '../../../src/transports/base'; +import type { ClientReport } from '../../../src/types-hoist/clientreport'; import type { AttachmentItem, EventEnvelope, EventItem } from '../../../src/types-hoist/envelope'; import type { TransportMakeRequestResponse } from '../../../src/types-hoist/transport'; +import { createClientReportEnvelope } from '../../../src/utils/clientreport'; import { createEnvelope, serializeEnvelope } from '../../../src/utils/envelope'; -import type { PromiseBuffer } from '../../../src/utils/promisebuffer'; +import { type PromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from '../../../src/utils/promisebuffer'; import { resolvedSyncPromise } from '../../../src/utils/syncpromise'; const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ @@ -31,6 +33,25 @@ const ATTACHMENT_ENVELOPE = createEnvelope( ], ); +const defaultDiscardedEvents: ClientReport['discarded_events'] = [ + { + reason: 'before_send', + category: 'error', + quantity: 30, + }, + { + reason: 'network_error', + category: 'transaction', + quantity: 23, + }, +]; + +const CLIENT_REPORT_ENVELOPE = createClientReportEnvelope( + defaultDiscardedEvents, + 'https://public@dsn.ingest.sentry.io/1337', + 123456, +); + const transportOptions = { recordDroppedEvent: () => undefined, // noop }; @@ -304,5 +325,71 @@ describe('createTransport', () => { expect(recordDroppedEventCallback).not.toHaveBeenCalled(); }); }); + + describe('Client Reports', () => { + it('should not record outcomes when client reports fail to send', async () => { + expect.assertions(2); + + const mockRecordDroppedEventCallback = vi.fn(); + + const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, req => { + expect(req.body).toEqual(serializeEnvelope(CLIENT_REPORT_ENVELOPE)); + return Promise.reject(new Error('Network error')); + }); + + try { + await transport.send(CLIENT_REPORT_ENVELOPE); + } catch (e) { + // Expected to throw + } + + // recordDroppedEvent should NOT be called when a client report fails + expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled(); + }); + + it('should not record outcomes when client reports fail due to buffer overflow', async () => { + expect.assertions(2); + + const mockRecordDroppedEventCallback = vi.fn(); + const mockBuffer: PromiseBuffer = { + $: [], + add: vi.fn(() => Promise.reject(SENTRY_BUFFER_FULL_ERROR)), + drain: vi.fn(), + }; + + const transport = createTransport( + { recordDroppedEvent: mockRecordDroppedEventCallback }, + _ => resolvedSyncPromise({}), + mockBuffer, + ); + + const result = await transport.send(CLIENT_REPORT_ENVELOPE); + + // Should resolve without throwing + expect(result).toEqual({}); + // recordDroppedEvent should NOT be called when a client report fails + expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled(); + }); + + it('should record outcomes when regular events fail to send', async () => { + expect.assertions(2); + + const mockRecordDroppedEventCallback = vi.fn(); + + const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, req => { + expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE)); + return Promise.reject(new Error('Network error')); + }); + + try { + await transport.send(ERROR_ENVELOPE); + } catch (e) { + // Expected to throw + } + + // recordDroppedEvent SHOULD be called for regular events + expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('network_error', 'error'); + }); + }); }); }); From 1cd7ba182ec59e3733433b9c6de7dd74ba6a58bd Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Wed, 14 Jan 2026 10:53:24 +0100 Subject: [PATCH 2/4] Revert "fix(core): Don't record outcomes for failed client reports" This reverts commit 9c1d063c72e4de98123a11d44778bb2e7e3ae847. --- packages/core/src/transports/base.ts | 5 -- .../core/test/lib/transports/base.test.ts | 89 +------------------ 2 files changed, 1 insertion(+), 93 deletions(-) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index 372c27c6b275..d85d9305bbe9 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -10,7 +10,6 @@ import type { import { debug } from '../utils/debug-logger'; import { createEnvelope, - envelopeContainsItemType, envelopeItemTypeToDataCategory, forEachEnvelopeItem, serializeEnvelope, @@ -58,10 +57,6 @@ export function createTransport( // Creates client report for each item in an envelope const recordEnvelopeLoss = (reason: EventDropReason): void => { - // Don't record outcomes for client reports - we don't want to create a feedback loop if client reports themselves fail to send - if (envelopeContainsItemType(filteredEnvelope, ['client_report'])) { - return; - } forEachEnvelopeItem(filteredEnvelope, (item, type) => { options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type)); }); diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts index df11d0fafc29..ef2220ac1f8b 100644 --- a/packages/core/test/lib/transports/base.test.ts +++ b/packages/core/test/lib/transports/base.test.ts @@ -1,11 +1,9 @@ import { describe, expect, it, vi } from 'vitest'; import { createTransport } from '../../../src/transports/base'; -import type { ClientReport } from '../../../src/types-hoist/clientreport'; import type { AttachmentItem, EventEnvelope, EventItem } from '../../../src/types-hoist/envelope'; import type { TransportMakeRequestResponse } from '../../../src/types-hoist/transport'; -import { createClientReportEnvelope } from '../../../src/utils/clientreport'; import { createEnvelope, serializeEnvelope } from '../../../src/utils/envelope'; -import { type PromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from '../../../src/utils/promisebuffer'; +import type { PromiseBuffer } from '../../../src/utils/promisebuffer'; import { resolvedSyncPromise } from '../../../src/utils/syncpromise'; const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ @@ -33,25 +31,6 @@ const ATTACHMENT_ENVELOPE = createEnvelope( ], ); -const defaultDiscardedEvents: ClientReport['discarded_events'] = [ - { - reason: 'before_send', - category: 'error', - quantity: 30, - }, - { - reason: 'network_error', - category: 'transaction', - quantity: 23, - }, -]; - -const CLIENT_REPORT_ENVELOPE = createClientReportEnvelope( - defaultDiscardedEvents, - 'https://public@dsn.ingest.sentry.io/1337', - 123456, -); - const transportOptions = { recordDroppedEvent: () => undefined, // noop }; @@ -325,71 +304,5 @@ describe('createTransport', () => { expect(recordDroppedEventCallback).not.toHaveBeenCalled(); }); }); - - describe('Client Reports', () => { - it('should not record outcomes when client reports fail to send', async () => { - expect.assertions(2); - - const mockRecordDroppedEventCallback = vi.fn(); - - const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, req => { - expect(req.body).toEqual(serializeEnvelope(CLIENT_REPORT_ENVELOPE)); - return Promise.reject(new Error('Network error')); - }); - - try { - await transport.send(CLIENT_REPORT_ENVELOPE); - } catch (e) { - // Expected to throw - } - - // recordDroppedEvent should NOT be called when a client report fails - expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled(); - }); - - it('should not record outcomes when client reports fail due to buffer overflow', async () => { - expect.assertions(2); - - const mockRecordDroppedEventCallback = vi.fn(); - const mockBuffer: PromiseBuffer = { - $: [], - add: vi.fn(() => Promise.reject(SENTRY_BUFFER_FULL_ERROR)), - drain: vi.fn(), - }; - - const transport = createTransport( - { recordDroppedEvent: mockRecordDroppedEventCallback }, - _ => resolvedSyncPromise({}), - mockBuffer, - ); - - const result = await transport.send(CLIENT_REPORT_ENVELOPE); - - // Should resolve without throwing - expect(result).toEqual({}); - // recordDroppedEvent should NOT be called when a client report fails - expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled(); - }); - - it('should record outcomes when regular events fail to send', async () => { - expect.assertions(2); - - const mockRecordDroppedEventCallback = vi.fn(); - - const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, req => { - expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE)); - return Promise.reject(new Error('Network error')); - }); - - try { - await transport.send(ERROR_ENVELOPE); - } catch (e) { - // Expected to throw - } - - // recordDroppedEvent SHOULD be called for regular events - expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('network_error', 'error'); - }); - }); }); }); From bd6b952bbbb4b9b22d80a28db0082104afc9587e Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Wed, 14 Jan 2026 10:52:40 +0100 Subject: [PATCH 3/4] fix(core): Don't record outcomes for failed client reports --- packages/core/src/transports/base.ts | 5 ++ .../core/test/lib/transports/base.test.ts | 89 ++++++++++++++++++- 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index d85d9305bbe9..372c27c6b275 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -10,6 +10,7 @@ import type { import { debug } from '../utils/debug-logger'; import { createEnvelope, + envelopeContainsItemType, envelopeItemTypeToDataCategory, forEachEnvelopeItem, serializeEnvelope, @@ -57,6 +58,10 @@ export function createTransport( // Creates client report for each item in an envelope const recordEnvelopeLoss = (reason: EventDropReason): void => { + // Don't record outcomes for client reports - we don't want to create a feedback loop if client reports themselves fail to send + if (envelopeContainsItemType(filteredEnvelope, ['client_report'])) { + return; + } forEachEnvelopeItem(filteredEnvelope, (item, type) => { options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type)); }); diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts index ef2220ac1f8b..df11d0fafc29 100644 --- a/packages/core/test/lib/transports/base.test.ts +++ b/packages/core/test/lib/transports/base.test.ts @@ -1,9 +1,11 @@ import { describe, expect, it, vi } from 'vitest'; import { createTransport } from '../../../src/transports/base'; +import type { ClientReport } from '../../../src/types-hoist/clientreport'; import type { AttachmentItem, EventEnvelope, EventItem } from '../../../src/types-hoist/envelope'; import type { TransportMakeRequestResponse } from '../../../src/types-hoist/transport'; +import { createClientReportEnvelope } from '../../../src/utils/clientreport'; import { createEnvelope, serializeEnvelope } from '../../../src/utils/envelope'; -import type { PromiseBuffer } from '../../../src/utils/promisebuffer'; +import { type PromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from '../../../src/utils/promisebuffer'; import { resolvedSyncPromise } from '../../../src/utils/syncpromise'; const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ @@ -31,6 +33,25 @@ const ATTACHMENT_ENVELOPE = createEnvelope( ], ); +const defaultDiscardedEvents: ClientReport['discarded_events'] = [ + { + reason: 'before_send', + category: 'error', + quantity: 30, + }, + { + reason: 'network_error', + category: 'transaction', + quantity: 23, + }, +]; + +const CLIENT_REPORT_ENVELOPE = createClientReportEnvelope( + defaultDiscardedEvents, + 'https://public@dsn.ingest.sentry.io/1337', + 123456, +); + const transportOptions = { recordDroppedEvent: () => undefined, // noop }; @@ -304,5 +325,71 @@ describe('createTransport', () => { expect(recordDroppedEventCallback).not.toHaveBeenCalled(); }); }); + + describe('Client Reports', () => { + it('should not record outcomes when client reports fail to send', async () => { + expect.assertions(2); + + const mockRecordDroppedEventCallback = vi.fn(); + + const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, req => { + expect(req.body).toEqual(serializeEnvelope(CLIENT_REPORT_ENVELOPE)); + return Promise.reject(new Error('Network error')); + }); + + try { + await transport.send(CLIENT_REPORT_ENVELOPE); + } catch (e) { + // Expected to throw + } + + // recordDroppedEvent should NOT be called when a client report fails + expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled(); + }); + + it('should not record outcomes when client reports fail due to buffer overflow', async () => { + expect.assertions(2); + + const mockRecordDroppedEventCallback = vi.fn(); + const mockBuffer: PromiseBuffer = { + $: [], + add: vi.fn(() => Promise.reject(SENTRY_BUFFER_FULL_ERROR)), + drain: vi.fn(), + }; + + const transport = createTransport( + { recordDroppedEvent: mockRecordDroppedEventCallback }, + _ => resolvedSyncPromise({}), + mockBuffer, + ); + + const result = await transport.send(CLIENT_REPORT_ENVELOPE); + + // Should resolve without throwing + expect(result).toEqual({}); + // recordDroppedEvent should NOT be called when a client report fails + expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled(); + }); + + it('should record outcomes when regular events fail to send', async () => { + expect.assertions(2); + + const mockRecordDroppedEventCallback = vi.fn(); + + const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, req => { + expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE)); + return Promise.reject(new Error('Network error')); + }); + + try { + await transport.send(ERROR_ENVELOPE); + } catch (e) { + // Expected to throw + } + + // recordDroppedEvent SHOULD be called for regular events + expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('network_error', 'error'); + }); + }); }); }); From db0b856855510853a096271d1a418f4fac54c081 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Wed, 14 Jan 2026 11:13:01 +0100 Subject: [PATCH 4/4] add log --- packages/core/src/transports/base.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index 372c27c6b275..b0fc1abcb433 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -60,6 +60,7 @@ export function createTransport( const recordEnvelopeLoss = (reason: EventDropReason): void => { // Don't record outcomes for client reports - we don't want to create a feedback loop if client reports themselves fail to send if (envelopeContainsItemType(filteredEnvelope, ['client_report'])) { + DEBUG_BUILD && debug.warn(`Dropping client report. Will not send outcomes (reason: ${reason}).`); return; } forEachEnvelopeItem(filteredEnvelope, (item, type) => {