diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index 33877b1c70b3..af003380e4ca 100644 --- a/packages/core/src/transports/offline.ts +++ b/packages/core/src/transports/offline.ts @@ -3,6 +3,7 @@ import type { Envelope } from '../types/envelope'; import type { InternalBaseTransportOptions, Transport, TransportMakeRequestResponse } from '../types/transport'; import { debug } from '../utils/debug-logger'; import { envelopeContainsItemType } from '../utils/envelope'; +import { isThenable } from '../utils/is'; import { safeDateNow } from '../utils/randomSafeContext'; import { parseRetryAfterHeader } from '../utils/ratelimit'; import { safeUnref } from '../utils/timer'; @@ -139,8 +140,15 @@ export function makeOfflineTransport( } try { - if (options.shouldSend && (await options.shouldSend(envelope)) === false) { - throw new Error('Envelope not sent because `shouldSend` callback returned false'); + if (options.shouldSend) { + const decision = options.shouldSend(envelope); + // avoid extra microtask tick, as some hosts stop JS execution + // when the app goes to the background. + const shouldSend = isThenable(decision) ? await decision : decision; + + if (shouldSend === false) { + throw new Error('Envelope not sent because `shouldSend` callback returned false'); + } } const result = await transport.send(envelope); @@ -163,7 +171,9 @@ export function makeOfflineTransport( retryDelay = START_DELAY; return result; } catch (e) { - if (await shouldQueue(envelope, e as Error, retryDelay)) { + // do not unnecessarily await if it's not a Promise + const decision = shouldQueue(envelope, e as Error, retryDelay); + if (isThenable(decision) ? await decision : decision) { // If this envelope was a retry, we want to add it to the front of the queue so it's retried again first. if (isRetry) { await store.unshift(envelope); diff --git a/packages/core/test/lib/transports/offline.test.ts b/packages/core/test/lib/transports/offline.test.ts index a43a86a2c409..e78b6d1bda89 100644 --- a/packages/core/test/lib/transports/offline.test.ts +++ b/packages/core/test/lib/transports/offline.test.ts @@ -377,6 +377,54 @@ describe('makeOfflineTransport', () => { expect(getCalls()).toEqual(['push']); }); + it('a synchronous shouldSend does not defer the send by a microtask', async () => { + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); + + const { store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }); + const transport = makeOfflineTransport(baseTransport)({ + ...transportOptions, + createStore: store, + shouldSend: () => true, + }); + + // Some hosts stop JS execution when the app goes to the background. + // They only send what the transport already got. So a synchronous + // `shouldSend` must not push the send into the next microtask. + const result = transport.send(ERROR_ENVELOPE); + expect(getSendCount()).toEqual(1); + + await expect(result).resolves.toEqual({ statusCode: 200 }); + }); + + it('a synchronous shouldStore does not defer the store by a microtask', async () => { + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); + + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }); + const transport = makeOfflineTransport(baseTransport)({ + ...transportOptions, + createStore: store, + shouldSend: () => false, + shouldStore: () => true, + }); + + // A synchronous `shouldSend` that says no throws in the same tick, + // so the envelope must reach the store before the host can stop JS + // execution. + const result = transport.send(ERROR_ENVELOPE); + expect(getCalls()).toEqual(['push']); + + await expect(result).resolves.toEqual({}); + expect(getSendCount()).toEqual(0); + }); + it('should not store client report envelopes on send failure', async () => { const { getCalls, store } = createTestStore(); const { getSendCount, baseTransport } = createTestTransport(new Error());