diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index 9105ad63558a..e318793624ce 100644 --- a/packages/core/src/transports/offline.ts +++ b/packages/core/src/transports/offline.ts @@ -1,22 +1,10 @@ import type { Envelope, InternalBaseTransportOptions, Transport, TransportMakeRequestResponse } from '@sentry/types'; -import { forEachEnvelopeItem, logger, parseRetryAfterHeader } from '@sentry/utils'; +import { envelopeContainsItemType, logger, parseRetryAfterHeader } from '@sentry/utils'; export const MIN_DELAY = 100; // 100 ms export const START_DELAY = 5_000; // 5 seconds const MAX_DELAY = 3.6e6; // 1 hour -function isReplayEnvelope(envelope: Envelope): boolean { - let isReplay = false; - - forEachEnvelopeItem(envelope, (_, type) => { - if (type === 'replay_event') { - isReplay = true; - } - }); - - return isReplay; -} - function log(msg: string, error?: Error): void { __DEBUG_BUILD__ && logger.info(`[Offline]: ${msg}`, error); } @@ -74,7 +62,8 @@ export function makeOfflineTransport( // We don't queue Session Replay envelopes because they are: // - Ordered and Replay relies on the response status to know when they're successfully sent. // - Likely to fill the queue quickly and block other events from being sent. - if (isReplayEnvelope(env)) { + // We also want to drop client reports because they can be generated when we retry sending events while offline. + if (envelopeContainsItemType(env, ['replay_event', 'replay_recording', 'client_report'])) { return false; } diff --git a/packages/core/test/lib/transports/offline.test.ts b/packages/core/test/lib/transports/offline.test.ts index ad38f0363279..eebe7c8874ed 100644 --- a/packages/core/test/lib/transports/offline.test.ts +++ b/packages/core/test/lib/transports/offline.test.ts @@ -1,4 +1,5 @@ import type { + ClientReport, Envelope, EventEnvelope, EventItem, @@ -9,6 +10,7 @@ import type { TransportMakeRequestResponse, } from '@sentry/types'; import { + createClientReportEnvelope, createEnvelope, createEventEnvelopeHeaders, dsnFromString, @@ -54,6 +56,25 @@ const RELAY_ENVELOPE = createEnvelope( ], ); +const DEFAULT_DISCARDED_EVENTS: ClientReport['discarded_events'] = [ + { + reason: 'before_send', + category: 'error', + quantity: 30, + }, + { + reason: 'network_error', + category: 'transaction', + quantity: 23, + }, +]; + +const CLIENT_REPORT_ENVELOPE = createClientReportEnvelope( + DEFAULT_DISCARDED_EVENTS, + 'https://public@dsn.ingest.sentry.io/1337', + 123456, +); + const transportOptions = { recordDroppedEvent: () => undefined, // noop textEncoder: new TextEncoder(), @@ -288,6 +309,23 @@ describe('makeOfflineTransport', () => { expect(getCalls()).toEqual([]); }); + it('should not store client report envelopes on send failure', async () => { + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport(new Error()); + const queuedCount = 0; + const transport = makeOfflineTransport(baseTransport)({ + ...transportOptions, + createStore: store, + shouldStore: () => true, + }); + const result = transport.send(CLIENT_REPORT_ENVELOPE); + + await expect(result).rejects.toBeInstanceOf(Error); + expect(queuedCount).toEqual(0); + expect(getSendCount()).toEqual(0); + expect(getCalls()).toEqual([]); + }); + it( 'Follows the Retry-After header', async () => { diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index b72f905fcbd4..705326b6c9ba 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -6,7 +6,6 @@ import type { DataCategory, DsnComponents, Envelope, - EnvelopeItem, EnvelopeItemType, Event, EventEnvelopeHeaders, @@ -41,16 +40,32 @@ export function addItemToEnvelope(envelope: E, newItem: E[1] /** * Convenience function to loop through the items and item types of an envelope. * (This function was mostly created because working with envelope types is painful at the moment) + * + * If the callback returns true, the rest of the items will be skipped. */ export function forEachEnvelopeItem( envelope: Envelope, - callback: (envelopeItem: E[1][number], envelopeItemType: E[1][number][0]['type']) => void, -): void { + callback: (envelopeItem: E[1][number], envelopeItemType: E[1][number][0]['type']) => boolean | void, +): boolean { const envelopeItems = envelope[1]; - envelopeItems.forEach((envelopeItem: EnvelopeItem) => { + + for (const envelopeItem of envelopeItems) { const envelopeItemType = envelopeItem[0].type; - callback(envelopeItem, envelopeItemType); - }); + const result = callback(envelopeItem, envelopeItemType); + + if (result) { + return true; + } + } + + return false; +} + +/** + * Returns true if the envelope contains any of the given envelope item types + */ +export function envelopeContainsItemType(envelope: Envelope, types: EnvelopeItemType[]): boolean { + return forEachEnvelopeItem(envelope, (_, type) => types.includes(type)); } /**