From 3ba23563c7336b5f3b6bdcb4e3f479e47105674a Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 18 Jan 2023 16:27:59 +0000 Subject: [PATCH 01/17] in progress --- packages/core/src/index.ts | 1 + packages/core/src/transports/offline.ts | 123 ++++++++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 packages/core/src/transports/offline.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index ccb7169808bd..a3287fe7d65a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -24,6 +24,7 @@ export { getEnvelopeEndpointWithUrlEncodedAuth, getReportDialogEndpoint } from ' export { BaseClient } from './baseclient'; export { initAndBind } from './sdk'; export { createTransport } from './transports/base'; +export { makeOfflineTransport } from './transports/offline'; export { SDK_VERSION } from './version'; export { getIntegrationsToSetup } from './integration'; export { FunctionToString, InboundFilters } from './integrations'; diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts new file mode 100644 index 000000000000..46a64612c483 --- /dev/null +++ b/packages/core/src/transports/offline.ts @@ -0,0 +1,123 @@ +import type { Envelope, InternalBaseTransportOptions, Transport, TransportMakeRequestResponse } from '@sentry/types'; + +function wasRateLimited(result: TransportMakeRequestResponse): boolean { + return !!(result.headers && 'x-sentry-rate-limits' in result.headers); +} + +type BeforeSendResponse = 'send' | 'queue' | 'drop'; + +interface OfflineTransportOptions extends InternalBaseTransportOptions { + /** + * The maximum number of days to keep an event in the queue. + */ + maxQueueAgeDays?: number; + + /** + * The maximum number of events to keep in the queue. + */ + maxQueueCount?: number; + + /** + * Called every time the number of requests in the queue changes. + */ + queuedLengthChanged?: (length: number) => void; + + /** + * Called before attempting to send an event to Sentry. + * + * Return 'send' to attempt to send the event. + * Return 'queue' to queue the event for sending later. + * Return 'drop' to drop the event. + */ + beforeSend?: (request: Envelope) => BeforeSendResponse | Promise; +} + +interface OfflineTransportStore { + add(env: Envelope): Promise; + pop(): Promise<[Envelope | undefined, number]>; +} + +const START_DELAY = 5_000; +const MAX_DELAY = 2_000_000_000; + +/** */ +export function makeOfflineTransport( + transport: (options: TO) => Transport, + store: OfflineTransportStore, +): (options: TO & OfflineTransportOptions) => Transport { + return (options: TO & OfflineTransportOptions) => { + const baseTransport = transport(options); + + let retryDelay = START_DELAY; + let lastQueueLength = -1; + + function queueLengthChanged(length: number): void { + if (options.queuedLengthChanged && length !== lastQueueLength) { + lastQueueLength = length; + options.queuedLengthChanged(length); + } + } + + function queueRequest(envelope: Envelope): Promise { + return store.add(envelope).then(count => { + queueLengthChanged(count); + + setTimeout(() => { + flushQueue(); + }, retryDelay); + + retryDelay *= 3; + + // If the delay is bigger than 2^31 (max signed 32-bit int), setTimeout throws + // an error on node.js and falls back to 1 which can cause a huge number of requests. + if (retryDelay > MAX_DELAY) { + retryDelay = MAX_DELAY; + } + }); + } + + function flushQueue(): void { + void store.pop().then(([found, count]) => { + if (found) { + // We have pending plus just found + queueLengthChanged(count + 1); + void send(found); + } else { + queueLengthChanged(0); + } + }); + } + + // eslint-disable-next-line @sentry-internal/sdk/no-async-await + async function send(request: Envelope): Promise { + let action = (await options.beforeSend?.(request)) || 'send'; + + if (action === 'send') { + try { + const result = await baseTransport.send(request); + if (!wasRateLimited(result || {})) { + // Reset the retry delay + retryDelay = START_DELAY; + // We were successful so check the queue + flushQueue(); + return result; + } + } catch (_) { + // + } + action = 'queue'; + } + + if (action == 'queue') { + void queueRequest(request); + } + + return {}; + } + + return { + send, + flush: (timeout?: number) => baseTransport.flush(timeout), + }; + }; +} From 26e5b0ca14539b48ffb11a653c0a8054fa38cded Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 19 Jan 2023 17:13:05 +0000 Subject: [PATCH 02/17] Simplify a bit and add some tests --- packages/core/src/transports/offline.ts | 107 ++++++----- .../core/test/lib/transports/offline.test.ts | 175 ++++++++++++++++++ 2 files changed, 240 insertions(+), 42 deletions(-) create mode 100644 packages/core/test/lib/transports/offline.test.ts diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index 46a64612c483..490666620846 100644 --- a/packages/core/src/transports/offline.ts +++ b/packages/core/src/transports/offline.ts @@ -1,26 +1,35 @@ import type { Envelope, InternalBaseTransportOptions, Transport, TransportMakeRequestResponse } from '@sentry/types'; +import { logger } from '@sentry/utils'; + +export const START_DELAY = 5_000; +const MAX_DELAY = 2_000_000_000; +const DEFAULT_QUEUE_SIZE = 30; function wasRateLimited(result: TransportMakeRequestResponse): boolean { - return !!(result.headers && 'x-sentry-rate-limits' in result.headers); + return !!(result.headers && 'x-sentry-rate-limits' in result.headers && result.headers['x-sentry-rate-limits']); } type BeforeSendResponse = 'send' | 'queue' | 'drop'; interface OfflineTransportOptions extends InternalBaseTransportOptions { /** - * The maximum number of days to keep an event in the queue. + * The maximum number of events to keep in the offline queue. + * + * Defaults: 30 */ - maxQueueAgeDays?: number; + maxQueueSize?: number; /** - * The maximum number of events to keep in the queue. + * Flush the offline queue shortly after startup. + * + * Defaults: false */ - maxQueueCount?: number; + flushAtStartup?: boolean; /** - * Called every time the number of requests in the queue changes. + * Called when an event is queued . */ - queuedLengthChanged?: (length: number) => void; + eventQueued?: () => void; /** * Called before attempting to send an event to Sentry. @@ -32,38 +41,42 @@ interface OfflineTransportOptions extends InternalBaseTransportOptions { beforeSend?: (request: Envelope) => BeforeSendResponse | Promise; } -interface OfflineTransportStore { - add(env: Envelope): Promise; - pop(): Promise<[Envelope | undefined, number]>; +interface OfflineStore { + insert(env: Envelope): Promise; + pop(): Promise; } -const START_DELAY = 5_000; -const MAX_DELAY = 2_000_000_000; +export type CreateOfflineStore = (maxQueueCount: number) => OfflineStore; -/** */ +/** + * Wraps a transport and queues events when envelopes fail to send. + * + * @param createTransport The transport to wrap. + * @param createStore The store implementation to use. + */ export function makeOfflineTransport( - transport: (options: TO) => Transport, - store: OfflineTransportStore, + createTransport: (options: TO) => Transport, + createStore: CreateOfflineStore, ): (options: TO & OfflineTransportOptions) => Transport { return (options: TO & OfflineTransportOptions) => { - const baseTransport = transport(options); + const baseTransport = createTransport(options); + const maxQueueSize = options.maxQueueSize === undefined ? DEFAULT_QUEUE_SIZE : options.maxQueueSize; + const store = createStore(maxQueueSize); let retryDelay = START_DELAY; - let lastQueueLength = -1; - function queueLengthChanged(length: number): void { - if (options.queuedLengthChanged && length !== lastQueueLength) { - lastQueueLength = length; - options.queuedLengthChanged(length); + function queued(): void { + if (options.eventQueued) { + options.eventQueued(); } } function queueRequest(envelope: Envelope): Promise { - return store.add(envelope).then(count => { - queueLengthChanged(count); + return store.insert(envelope).then(() => { + queued(); setTimeout(() => { - flushQueue(); + void flushQueue(); }, retryDelay); retryDelay *= 3; @@ -76,36 +89,40 @@ export function makeOfflineTransport( }); } - function flushQueue(): void { - void store.pop().then(([found, count]) => { - if (found) { - // We have pending plus just found - queueLengthChanged(count + 1); - void send(found); - } else { - queueLengthChanged(0); - } - }); + async function flushQueue(): Promise { + const found = await store.pop(); + + if (found) { + __DEBUG_BUILD__ && logger.info('[Offline]: Attempting to send previously queued event'); + void send(found); + } } - // eslint-disable-next-line @sentry-internal/sdk/no-async-await async function send(request: Envelope): Promise { - let action = (await options.beforeSend?.(request)) || 'send'; + let action = 'send'; + + if (options.beforeSend) { + action = await options.beforeSend(request); + } if (action === 'send') { try { const result = await baseTransport.send(request); - if (!wasRateLimited(result || {})) { + if (wasRateLimited(result || {})) { + __DEBUG_BUILD__ && logger.info('[Offline]: Event queued due to rate limiting'); + action = 'queue'; + } else { + // Envelope was successfully sent // Reset the retry delay retryDelay = START_DELAY; - // We were successful so check the queue - flushQueue(); + // Check if there are any more in the queue + void flushQueue(); return result; } - } catch (_) { - // + } catch (e) { + __DEBUG_BUILD__ && logger.info('[Offline]: Event queued due to error', e); + action = 'queue'; } - action = 'queue'; } if (action == 'queue') { @@ -115,6 +132,12 @@ export function makeOfflineTransport( return {}; } + if (options.flushAtStartup) { + setTimeout(() => { + void flushQueue(); + }, retryDelay); + } + return { send, flush: (timeout?: number) => baseTransport.flush(timeout), diff --git a/packages/core/test/lib/transports/offline.test.ts b/packages/core/test/lib/transports/offline.test.ts new file mode 100644 index 000000000000..f4faf53b0281 --- /dev/null +++ b/packages/core/test/lib/transports/offline.test.ts @@ -0,0 +1,175 @@ +import type { + EventEnvelope, + EventItem, + TransportMakeRequestResponse, + Envelope, + Transport, + InternalBaseTransportOptions, +} from '@sentry/types'; +import { createEnvelope } from '@sentry/utils'; +import { TextEncoder } from 'util'; + +import { makeOfflineTransport, createTransport } from '../../../src'; +import { CreateOfflineStore, START_DELAY } from '../../../src/transports/offline'; + +const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ + [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, +]); + +const transportOptions = { + recordDroppedEvent: () => undefined, // noop + textEncoder: new TextEncoder(), +}; + +type MockResult = T | Error; + +const createTestTransport = ( + ...sendResults: MockResult[] +): { getSendCount: () => number; baseTransport: (options: InternalBaseTransportOptions) => Transport } => { + let sendCount = 0; + + return { + getSendCount: () => sendCount, + baseTransport: (options: InternalBaseTransportOptions) => + createTransport(options, () => { + return new Promise((resolve, reject) => { + const next = sendResults.shift(); + + if (next instanceof Error) { + reject(next); + } else { + sendCount += 1; + resolve(next as TransportMakeRequestResponse | undefined); + } + }); + }), + }; +}; + +type StoreEvents = ('add' | 'pop')[]; + +function createTestStore(...popResults: MockResult[]): { + getCalls: () => StoreEvents; + store: CreateOfflineStore; +} { + const calls: StoreEvents = []; + + return { + getCalls: () => calls, + store: (maxQueueCount: number) => ({ + insert: async env => { + if (popResults.length < maxQueueCount) { + popResults.push(env); + calls.push('add'); + } + }, + pop: async () => { + calls.push('pop'); + const next = popResults.shift(); + + if (next instanceof Error) { + throw next; + } + + return next; + }, + }), + }; +} + +function delay(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +describe('makeOfflineTransport', () => { + it('Sends envelope and checks the store for further envelopes', async () => { + expect.assertions(3); + + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }); + const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const result = await transport.send(ERROR_ENVELOPE); + + expect(result).toEqual({ statusCode: 200 }); + expect(getSendCount()).toEqual(1); + // After a successful send, the store should be checked + expect(getCalls()).toEqual(['pop']); + }); + + it('After successfully sending, sends further envelopes found in the store', async () => { + const { getCalls, store } = createTestStore(ERROR_ENVELOPE); + const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }, { statusCode: 200 }); + const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const result = await transport.send(ERROR_ENVELOPE); + + expect(result).toEqual({ statusCode: 200 }); + + await delay(100); + + expect(getSendCount()).toEqual(2); + // After a successful send, the store should be checked again to ensure it's empty + expect(getCalls()).toEqual(['pop', 'pop']); + }); + + it('Queues envelope if wrapped transport throws error', async () => { + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport(new Error()); + const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const result = await transport.send(ERROR_ENVELOPE); + + expect(result).toEqual({}); + + await delay(100); + + expect(getSendCount()).toEqual(0); + expect(getCalls()).toEqual(['add']); + }); + + it('Queues envelope if rate limited', async () => { + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport({ + headers: { 'x-sentry-rate-limits': 'something', 'retry-after': null }, + }); + const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const result = await transport.send(ERROR_ENVELOPE); + expect(result).toEqual({}); + + await delay(100); + + expect(getSendCount()).toEqual(1); + expect(getCalls()).toEqual(['add']); + }); + + it( + 'Retries sending envelope after failure', + async () => { + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport(new Error(), { statusCode: 200 }); + const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const result = await transport.send(ERROR_ENVELOPE); + expect(result).toEqual({}); + expect(getCalls()).toEqual(['add']); + + await delay(START_DELAY + 1_000); + + expect(getSendCount()).toEqual(1); + expect(getCalls()).toEqual(['add', 'pop', 'pop']); + }, + START_DELAY + 2_000, + ); + + it( + 'When enabled, sends envelopes found in store shortly after startup', + async () => { + const { getCalls, store } = createTestStore(ERROR_ENVELOPE, ERROR_ENVELOPE); + const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }, { statusCode: 200 }); + const _transport = makeOfflineTransport(baseTransport, store)({ ...transportOptions, flushAtStartup: true }); + + await delay(START_DELAY + 1_000); + + expect(getSendCount()).toEqual(2); + expect(getCalls()).toEqual(['pop', 'pop', 'pop']); + }, + START_DELAY + 2_000, + ); +}); From 9a252cac99513779adbf2bb8553d4351cf0a842d Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 19 Jan 2023 23:00:18 +0000 Subject: [PATCH 03/17] Fix linting --- packages/core/src/transports/offline.ts | 4 +- .../core/test/lib/transports/offline.test.ts | 352 +++++++++--------- 2 files changed, 179 insertions(+), 177 deletions(-) diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index 490666620846..689c139aa51a 100644 --- a/packages/core/src/transports/offline.ts +++ b/packages/core/src/transports/offline.ts @@ -6,7 +6,7 @@ const MAX_DELAY = 2_000_000_000; const DEFAULT_QUEUE_SIZE = 30; function wasRateLimited(result: TransportMakeRequestResponse): boolean { - return !!(result.headers && 'x-sentry-rate-limits' in result.headers && result.headers['x-sentry-rate-limits']); + return !!(result.headers && result.headers['x-sentry-rate-limits']); } type BeforeSendResponse = 'send' | 'queue' | 'drop'; @@ -58,7 +58,7 @@ export function makeOfflineTransport( createTransport: (options: TO) => Transport, createStore: CreateOfflineStore, ): (options: TO & OfflineTransportOptions) => Transport { - return (options: TO & OfflineTransportOptions) => { + return options => { const baseTransport = createTransport(options); const maxQueueSize = options.maxQueueSize === undefined ? DEFAULT_QUEUE_SIZE : options.maxQueueSize; const store = createStore(maxQueueSize); diff --git a/packages/core/test/lib/transports/offline.test.ts b/packages/core/test/lib/transports/offline.test.ts index f4faf53b0281..3b3562e2940d 100644 --- a/packages/core/test/lib/transports/offline.test.ts +++ b/packages/core/test/lib/transports/offline.test.ts @@ -1,175 +1,177 @@ -import type { - EventEnvelope, - EventItem, - TransportMakeRequestResponse, - Envelope, - Transport, - InternalBaseTransportOptions, -} from '@sentry/types'; -import { createEnvelope } from '@sentry/utils'; -import { TextEncoder } from 'util'; - -import { makeOfflineTransport, createTransport } from '../../../src'; -import { CreateOfflineStore, START_DELAY } from '../../../src/transports/offline'; - -const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ - [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, -]); - -const transportOptions = { - recordDroppedEvent: () => undefined, // noop - textEncoder: new TextEncoder(), -}; - -type MockResult = T | Error; - -const createTestTransport = ( - ...sendResults: MockResult[] -): { getSendCount: () => number; baseTransport: (options: InternalBaseTransportOptions) => Transport } => { - let sendCount = 0; - - return { - getSendCount: () => sendCount, - baseTransport: (options: InternalBaseTransportOptions) => - createTransport(options, () => { - return new Promise((resolve, reject) => { - const next = sendResults.shift(); - - if (next instanceof Error) { - reject(next); - } else { - sendCount += 1; - resolve(next as TransportMakeRequestResponse | undefined); - } - }); - }), - }; -}; - -type StoreEvents = ('add' | 'pop')[]; - -function createTestStore(...popResults: MockResult[]): { - getCalls: () => StoreEvents; - store: CreateOfflineStore; -} { - const calls: StoreEvents = []; - - return { - getCalls: () => calls, - store: (maxQueueCount: number) => ({ - insert: async env => { - if (popResults.length < maxQueueCount) { - popResults.push(env); - calls.push('add'); - } - }, - pop: async () => { - calls.push('pop'); - const next = popResults.shift(); - - if (next instanceof Error) { - throw next; - } - - return next; - }, - }), - }; -} - -function delay(ms: number): Promise { - return new Promise(resolve => setTimeout(resolve, ms)); -} - -describe('makeOfflineTransport', () => { - it('Sends envelope and checks the store for further envelopes', async () => { - expect.assertions(3); - - const { getCalls, store } = createTestStore(); - const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }); - const transport = makeOfflineTransport(baseTransport, store)(transportOptions); - const result = await transport.send(ERROR_ENVELOPE); - - expect(result).toEqual({ statusCode: 200 }); - expect(getSendCount()).toEqual(1); - // After a successful send, the store should be checked - expect(getCalls()).toEqual(['pop']); - }); - - it('After successfully sending, sends further envelopes found in the store', async () => { - const { getCalls, store } = createTestStore(ERROR_ENVELOPE); - const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }, { statusCode: 200 }); - const transport = makeOfflineTransport(baseTransport, store)(transportOptions); - const result = await transport.send(ERROR_ENVELOPE); - - expect(result).toEqual({ statusCode: 200 }); - - await delay(100); - - expect(getSendCount()).toEqual(2); - // After a successful send, the store should be checked again to ensure it's empty - expect(getCalls()).toEqual(['pop', 'pop']); - }); - - it('Queues envelope if wrapped transport throws error', async () => { - const { getCalls, store } = createTestStore(); - const { getSendCount, baseTransport } = createTestTransport(new Error()); - const transport = makeOfflineTransport(baseTransport, store)(transportOptions); - const result = await transport.send(ERROR_ENVELOPE); - - expect(result).toEqual({}); - - await delay(100); - - expect(getSendCount()).toEqual(0); - expect(getCalls()).toEqual(['add']); - }); - - it('Queues envelope if rate limited', async () => { - const { getCalls, store } = createTestStore(); - const { getSendCount, baseTransport } = createTestTransport({ - headers: { 'x-sentry-rate-limits': 'something', 'retry-after': null }, - }); - const transport = makeOfflineTransport(baseTransport, store)(transportOptions); - const result = await transport.send(ERROR_ENVELOPE); - expect(result).toEqual({}); - - await delay(100); - - expect(getSendCount()).toEqual(1); - expect(getCalls()).toEqual(['add']); - }); - - it( - 'Retries sending envelope after failure', - async () => { - const { getCalls, store } = createTestStore(); - const { getSendCount, baseTransport } = createTestTransport(new Error(), { statusCode: 200 }); - const transport = makeOfflineTransport(baseTransport, store)(transportOptions); - const result = await transport.send(ERROR_ENVELOPE); - expect(result).toEqual({}); - expect(getCalls()).toEqual(['add']); - - await delay(START_DELAY + 1_000); - - expect(getSendCount()).toEqual(1); - expect(getCalls()).toEqual(['add', 'pop', 'pop']); - }, - START_DELAY + 2_000, - ); - - it( - 'When enabled, sends envelopes found in store shortly after startup', - async () => { - const { getCalls, store } = createTestStore(ERROR_ENVELOPE, ERROR_ENVELOPE); - const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }, { statusCode: 200 }); - const _transport = makeOfflineTransport(baseTransport, store)({ ...transportOptions, flushAtStartup: true }); - - await delay(START_DELAY + 1_000); - - expect(getSendCount()).toEqual(2); - expect(getCalls()).toEqual(['pop', 'pop', 'pop']); - }, - START_DELAY + 2_000, - ); -}); +import type { + Envelope, + EventEnvelope, + EventItem, + InternalBaseTransportOptions, + Transport, + TransportMakeRequestResponse, +} from '@sentry/types'; +import { createEnvelope } from '@sentry/utils'; +import { TextEncoder } from 'util'; + +import { createTransport, makeOfflineTransport } from '../../../src'; +import type { CreateOfflineStore } from '../../../src/transports/offline'; +import { START_DELAY } from '../../../src/transports/offline'; + +const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ + [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, +]); + +const transportOptions = { + recordDroppedEvent: () => undefined, // noop + textEncoder: new TextEncoder(), +}; + +type MockResult = T | Error; + +const createTestTransport = ( + ...sendResults: MockResult[] +): { getSendCount: () => number; baseTransport: (options: InternalBaseTransportOptions) => Transport } => { + let sendCount = 0; + + return { + getSendCount: () => sendCount, + baseTransport: (options: InternalBaseTransportOptions) => + createTransport(options, () => { + return new Promise((resolve, reject) => { + const next = sendResults.shift(); + + if (next instanceof Error) { + reject(next); + } else { + sendCount += 1; + resolve(next as TransportMakeRequestResponse | undefined); + } + }); + }), + }; +}; + +type StoreEvents = ('add' | 'pop')[]; + +function createTestStore(...popResults: MockResult[]): { + getCalls: () => StoreEvents; + store: CreateOfflineStore; +} { + const calls: StoreEvents = []; + + return { + getCalls: () => calls, + store: (maxQueueCount: number) => ({ + insert: async env => { + if (popResults.length < maxQueueCount) { + popResults.push(env); + calls.push('add'); + } + }, + pop: async () => { + calls.push('pop'); + const next = popResults.shift(); + + if (next instanceof Error) { + throw next; + } + + return next; + }, + }), + }; +} + +function delay(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +describe('makeOfflineTransport', () => { + it('Sends envelope and checks the store for further envelopes', async () => { + expect.assertions(3); + + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }); + const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const result = await transport.send(ERROR_ENVELOPE); + + expect(result).toEqual({ statusCode: 200 }); + expect(getSendCount()).toEqual(1); + // After a successful send, the store should be checked + expect(getCalls()).toEqual(['pop']); + }); + + it('After successfully sending, sends further envelopes found in the store', async () => { + const { getCalls, store } = createTestStore(ERROR_ENVELOPE); + const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }, { statusCode: 200 }); + const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const result = await transport.send(ERROR_ENVELOPE); + + expect(result).toEqual({ statusCode: 200 }); + + await delay(100); + + expect(getSendCount()).toEqual(2); + // After a successful send, the store should be checked again to ensure it's empty + expect(getCalls()).toEqual(['pop', 'pop']); + }); + + it('Queues envelope if wrapped transport throws error', async () => { + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport(new Error()); + const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const result = await transport.send(ERROR_ENVELOPE); + + expect(result).toEqual({}); + + await delay(100); + + expect(getSendCount()).toEqual(0); + expect(getCalls()).toEqual(['add']); + }); + + it('Queues envelope if rate limited', async () => { + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport({ + headers: { 'x-sentry-rate-limits': 'something', 'retry-after': null }, + }); + const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const result = await transport.send(ERROR_ENVELOPE); + expect(result).toEqual({}); + + await delay(100); + + expect(getSendCount()).toEqual(1); + expect(getCalls()).toEqual(['add']); + }); + + it( + 'Retries sending envelope after failure', + async () => { + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport(new Error(), { statusCode: 200 }); + const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const result = await transport.send(ERROR_ENVELOPE); + expect(result).toEqual({}); + expect(getCalls()).toEqual(['add']); + + await delay(START_DELAY + 1_000); + + expect(getSendCount()).toEqual(1); + expect(getCalls()).toEqual(['add', 'pop', 'pop']); + }, + START_DELAY + 2_000, + ); + + it( + 'When enabled, sends envelopes found in store shortly after startup', + async () => { + const { getCalls, store } = createTestStore(ERROR_ENVELOPE, ERROR_ENVELOPE); + const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }, { statusCode: 200 }); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const _transport = makeOfflineTransport(baseTransport, store)({ ...transportOptions, flushAtStartup: true }); + + await delay(START_DELAY + 1_000); + + expect(getSendCount()).toEqual(2); + expect(getCalls()).toEqual(['pop', 'pop', 'pop']); + }, + START_DELAY + 2_000, + ); +}); From b93b02b21fc159881c5bb92fe5f8bee467df6deb Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 20 Jan 2023 17:34:10 +0000 Subject: [PATCH 04/17] Improve --- packages/core/src/transports/offline.ts | 164 +++++++++------- .../core/test/lib/transports/offline.test.ts | 181 ++++++++++++++++-- 2 files changed, 257 insertions(+), 88 deletions(-) diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index 689c139aa51a..f217340e2168 100644 --- a/packages/core/src/transports/offline.ts +++ b/packages/core/src/transports/offline.ts @@ -1,44 +1,48 @@ import type { Envelope, InternalBaseTransportOptions, Transport, TransportMakeRequestResponse } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { forEachEnvelopeItem, logger, parseRetryAfterHeader } from '@sentry/utils'; -export const START_DELAY = 5_000; -const MAX_DELAY = 2_000_000_000; +export const MIN_DELAY = 100; // 100 ms +export const START_DELAY = 5_000; // 5 seconds +const MAX_DELAY = 3.6e6; // 1 hour const DEFAULT_QUEUE_SIZE = 30; -function wasRateLimited(result: TransportMakeRequestResponse): boolean { - return !!(result.headers && result.headers['x-sentry-rate-limits']); -} +function isReplayEnvelope(envelope: Envelope): boolean { + let isReplay = false; + + forEachEnvelopeItem(envelope, (_, type) => { + if (type === 'replay_event') { + isReplay = true; + } + }); -type BeforeSendResponse = 'send' | 'queue' | 'drop'; + return isReplay; +} interface OfflineTransportOptions extends InternalBaseTransportOptions { /** - * The maximum number of events to keep in the offline queue. + * The maximum number of events to keep in the offline store. * * Defaults: 30 */ maxQueueSize?: number; /** - * Flush the offline queue shortly after startup. + * Flush the offline store shortly after startup. * * Defaults: false */ flushAtStartup?: boolean; /** - * Called when an event is queued . - */ - eventQueued?: () => void; - - /** - * Called before attempting to send an event to Sentry. + * Called before an event is stored. * - * Return 'send' to attempt to send the event. - * Return 'queue' to queue the event for sending later. - * Return 'drop' to drop the event. + * Return false to drop the envelope rather than store it. + * + * @param envelope The envelope that failed to send. + * @param error The error that occurred. + * @param retryDelay The current retry delay in milliseconds. */ - beforeSend?: (request: Envelope) => BeforeSendResponse | Promise; + shouldStore?: (envelope: Envelope, error: Error, retryDelay: number) => boolean | Promise; } interface OfflineStore { @@ -48,8 +52,10 @@ interface OfflineStore { export type CreateOfflineStore = (maxQueueCount: number) => OfflineStore; +type Timer = number | { unref?: () => void }; + /** - * Wraps a transport and queues events when envelopes fail to send. + * Wraps a transport and stores and retries events when they fail to send. * * @param createTransport The transport to wrap. * @param createStore The store implementation to use. @@ -59,88 +65,104 @@ export function makeOfflineTransport( createStore: CreateOfflineStore, ): (options: TO & OfflineTransportOptions) => Transport { return options => { - const baseTransport = createTransport(options); + const transport = createTransport(options); const maxQueueSize = options.maxQueueSize === undefined ? DEFAULT_QUEUE_SIZE : options.maxQueueSize; const store = createStore(maxQueueSize); let retryDelay = START_DELAY; + let flushTimer: Timer | undefined; - function queued(): void { - if (options.eventQueued) { - options.eventQueued(); - } + function log(msg: string, error?: Error): void { + __DEBUG_BUILD__ && logger.info(`[Offline]: ${msg}`, error); } - function queueRequest(envelope: Envelope): Promise { - return store.insert(envelope).then(() => { - queued(); + function shouldQueue(env: Envelope, error: Error, retryDelay: number): boolean | Promise { + if (isReplayEnvelope(env)) { + return false; + } - setTimeout(() => { - void flushQueue(); - }, retryDelay); + if (options.shouldStore) { + return options.shouldStore(env, error, retryDelay); + } - retryDelay *= 3; + return true; + } - // If the delay is bigger than 2^31 (max signed 32-bit int), setTimeout throws - // an error on node.js and falls back to 1 which can cause a huge number of requests. - if (retryDelay > MAX_DELAY) { - retryDelay = MAX_DELAY; + function flushIn(delay: number): void { + if (flushTimer) { + clearTimeout(flushTimer as ReturnType); + } + + flushTimer = setTimeout(async () => { + flushTimer = undefined; + + const found = await store.pop(); + if (found) { + log('Attempting to send previously queued event'); + void send(found).catch(e => { + log('Failed to retry sending', e); + }); } - }); + }, delay) as Timer; + + // We need to unref the timer in node.js, otherwise the node process never exit. + if (typeof flushTimer !== 'number' && typeof flushTimer.unref === 'function') { + flushTimer.unref(); + } } - async function flushQueue(): Promise { - const found = await store.pop(); + function flushWithBackOff(): void { + if (flushTimer) { + return; + } + + flushIn(retryDelay); + + retryDelay *= 2; - if (found) { - __DEBUG_BUILD__ && logger.info('[Offline]: Attempting to send previously queued event'); - void send(found); + if (retryDelay > MAX_DELAY) { + retryDelay = MAX_DELAY; } } - async function send(request: Envelope): Promise { - let action = 'send'; + async function send(envelope: Envelope): Promise { + try { + const result = await transport.send(envelope); - if (options.beforeSend) { - action = await options.beforeSend(request); - } + let delay = MIN_DELAY; - if (action === 'send') { - try { - const result = await baseTransport.send(request); - if (wasRateLimited(result || {})) { - __DEBUG_BUILD__ && logger.info('[Offline]: Event queued due to rate limiting'); - action = 'queue'; - } else { - // Envelope was successfully sent - // Reset the retry delay - retryDelay = START_DELAY; - // Check if there are any more in the queue - void flushQueue(); + if (result) { + // If there's a retry-after header, use that as the next delay. + if (result.headers && result.headers['retry-after']) { + delay = parseRetryAfterHeader(result.headers['retry-after']); + } // If we have a server error, return now so we don't flush the queue. + else if ((result.statusCode || 0) >= 400) { return result; } - } catch (e) { - __DEBUG_BUILD__ && logger.info('[Offline]: Event queued due to error', e); - action = 'queue'; } - } - if (action == 'queue') { - void queueRequest(request); + flushIn(delay); + retryDelay = START_DELAY; + return result; + } catch (e) { + if (await shouldQueue(envelope, e, retryDelay)) { + await store.insert(envelope); + flushWithBackOff(); + log('Error sending. Event queued', e); + return {}; + } else { + throw e; + } } - - return {}; } if (options.flushAtStartup) { - setTimeout(() => { - void flushQueue(); - }, retryDelay); + flushWithBackOff(); } return { send, - flush: (timeout?: number) => baseTransport.flush(timeout), + flush: (timeout?: number) => transport.flush(timeout), }; }; } diff --git a/packages/core/test/lib/transports/offline.test.ts b/packages/core/test/lib/transports/offline.test.ts index 3b3562e2940d..d2fefa4a2948 100644 --- a/packages/core/test/lib/transports/offline.test.ts +++ b/packages/core/test/lib/transports/offline.test.ts @@ -3,20 +3,57 @@ import type { EventEnvelope, EventItem, InternalBaseTransportOptions, + ReplayEnvelope, + ReplayEvent, Transport, TransportMakeRequestResponse, } from '@sentry/types'; -import { createEnvelope } from '@sentry/utils'; +import { + createEnvelope, + createEventEnvelopeHeaders, + dsnFromString, + getSdkMetadataForEnvelopeHeader, +} from '@sentry/utils'; import { TextEncoder } from 'util'; -import { createTransport, makeOfflineTransport } from '../../../src'; +import { createTransport } from '../../../src'; import type { CreateOfflineStore } from '../../../src/transports/offline'; -import { START_DELAY } from '../../../src/transports/offline'; +import { makeOfflineTransport, MIN_DELAY, START_DELAY } from '../../../src/transports/offline'; const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, ]); +const REPLAY_EVENT: ReplayEvent = { + // @ts-ignore private api + type: 'replay_event', + timestamp: 1670837008.634, + error_ids: ['errorId'], + trace_ids: ['traceId'], + urls: ['https://example.com'], + replay_id: 'MY_REPLAY_ID', + segment_id: 3, + replay_type: 'error', +}; + +const DSN = dsnFromString('https://public@dsn.ingest.sentry.io/1337'); + +const DATA = 'nothing'; + +const RELAY_ENVELOPE = createEnvelope( + createEventEnvelopeHeaders(REPLAY_EVENT, getSdkMetadataForEnvelopeHeader(REPLAY_EVENT), undefined, DSN), + [ + [{ type: 'replay_event' }, REPLAY_EVENT], + [ + { + type: 'replay_recording', + length: DATA.length, + }, + DATA, + ], + ], +); + const transportOptions = { recordDroppedEvent: () => undefined, // noop textEncoder: new TextEncoder(), @@ -84,15 +121,27 @@ function delay(ms: number): Promise { describe('makeOfflineTransport', () => { it('Sends envelope and checks the store for further envelopes', async () => { - expect.assertions(3); - const { getCalls, store } = createTestStore(); const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }); - const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + let queuedCount = 0; + const transport = makeOfflineTransport( + baseTransport, + store, + )({ + ...transportOptions, + shouldStore: () => { + queuedCount += 1; + return true; + }, + }); const result = await transport.send(ERROR_ENVELOPE); expect(result).toEqual({ statusCode: 200 }); + expect(queuedCount).toEqual(0); expect(getSendCount()).toEqual(1); + + await delay(MIN_DELAY * 2); + // After a successful send, the store should be checked expect(getCalls()).toEqual(['pop']); }); @@ -105,40 +154,61 @@ describe('makeOfflineTransport', () => { expect(result).toEqual({ statusCode: 200 }); - await delay(100); + await delay(MIN_DELAY * 3); expect(getSendCount()).toEqual(2); - // After a successful send, the store should be checked again to ensure it's empty + // After a successful send from the store, the store should be checked again to ensure it's empty expect(getCalls()).toEqual(['pop', 'pop']); }); it('Queues envelope if wrapped transport throws error', async () => { const { getCalls, store } = createTestStore(); const { getSendCount, baseTransport } = createTestTransport(new Error()); - const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + let queuedCount = 0; + const transport = makeOfflineTransport( + baseTransport, + store, + )({ + ...transportOptions, + shouldStore: () => { + queuedCount += 1; + return true; + }, + }); const result = await transport.send(ERROR_ENVELOPE); expect(result).toEqual({}); - await delay(100); + await delay(MIN_DELAY * 2); expect(getSendCount()).toEqual(0); + expect(queuedCount).toEqual(1); expect(getCalls()).toEqual(['add']); }); - it('Queues envelope if rate limited', async () => { + it('Does not queue envelopes if status code >= 400', async () => { const { getCalls, store } = createTestStore(); - const { getSendCount, baseTransport } = createTestTransport({ - headers: { 'x-sentry-rate-limits': 'something', 'retry-after': null }, + const { getSendCount, baseTransport } = createTestTransport({ statusCode: 500 }); + let queuedCount = 0; + const transport = makeOfflineTransport( + baseTransport, + store, + )({ + ...transportOptions, + shouldStore: () => { + queuedCount += 1; + return true; + }, }); - const transport = makeOfflineTransport(baseTransport, store)(transportOptions); const result = await transport.send(ERROR_ENVELOPE); - expect(result).toEqual({}); - await delay(100); + expect(result).toEqual({ statusCode: 500 }); + + await delay(MIN_DELAY * 2); expect(getSendCount()).toEqual(1); - expect(getCalls()).toEqual(['add']); + expect(queuedCount).toEqual(0); + expect(getCalls()).toEqual([]); }); it( @@ -174,4 +244,81 @@ describe('makeOfflineTransport', () => { }, START_DELAY + 2_000, ); + + it('shouldStore can stop envelopes from being stored on send failure', async () => { + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport(new Error()); + const queuedCount = 0; + const transport = makeOfflineTransport( + baseTransport, + store, + )({ + ...transportOptions, + shouldStore: () => false, + }); + const result = transport.send(ERROR_ENVELOPE); + + await expect(result).rejects.toBeInstanceOf(Error); + expect(queuedCount).toEqual(0); + expect(getSendCount()).toEqual(0); + expect(getCalls()).toEqual([]); + }); + + it('should not store Relay envelopes on send failure', async () => { + const { getCalls, store } = createTestStore(); + const { getSendCount, baseTransport } = createTestTransport(new Error()); + const queuedCount = 0; + const transport = makeOfflineTransport( + baseTransport, + store, + )({ + ...transportOptions, + shouldStore: () => true, + }); + const result = transport.send(RELAY_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 () => { + const { getCalls, store } = createTestStore(ERROR_ENVELOPE); + const { getSendCount, baseTransport } = createTestTransport( + { + statusCode: 429, + headers: { 'x-sentry-rate-limits': '', 'retry-after': '3' }, + }, + { statusCode: 200 }, + ); + + let queuedCount = 0; + const transport = makeOfflineTransport( + baseTransport, + store, + )({ + ...transportOptions, + shouldStore: () => { + queuedCount += 1; + return true; + }, + }); + const result = await transport.send(ERROR_ENVELOPE); + + expect(result).toEqual({ + statusCode: 429, + headers: { 'x-sentry-rate-limits': '', 'retry-after': '3' }, + }); + + await delay(MIN_DELAY * 2); + + expect(getSendCount()).toEqual(1); + + await delay(4_000); + + expect(getSendCount()).toEqual(2); + expect(queuedCount).toEqual(0); + expect(getCalls()).toEqual(['pop', 'pop']); + }, 7_000); }); From 5f1af25cd3d1aa8e943b517f07acb3a6b462439e Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 24 Jan 2023 12:36:28 +0000 Subject: [PATCH 05/17] Review changes --- packages/core/src/transports/offline.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index f217340e2168..80422841a0b5 100644 --- a/packages/core/src/transports/offline.ts +++ b/packages/core/src/transports/offline.ts @@ -18,6 +18,10 @@ function isReplayEnvelope(envelope: Envelope): boolean { return isReplay; } +function log(msg: string, error?: Error): void { + __DEBUG_BUILD__ && logger.info(`[Offline]: ${msg}`, error); +} + interface OfflineTransportOptions extends InternalBaseTransportOptions { /** * The maximum number of events to keep in the offline store. @@ -72,11 +76,10 @@ export function makeOfflineTransport( let retryDelay = START_DELAY; let flushTimer: Timer | undefined; - function log(msg: string, error?: Error): void { - __DEBUG_BUILD__ && logger.info(`[Offline]: ${msg}`, error); - } - function shouldQueue(env: Envelope, error: Error, retryDelay: number): boolean | Promise { + // 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)) { return false; } @@ -162,7 +165,7 @@ export function makeOfflineTransport( return { send, - flush: (timeout?: number) => transport.flush(timeout), + flush: t => transport.flush(t), }; }; } From f86f4e62c6f78f9c029bb4a79179c5ec73b05730 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 26 Jan 2023 18:44:29 +0000 Subject: [PATCH 06/17] Make store a wrapper --- packages/core/src/transports/offline.ts | 27 ++++------ .../core/test/lib/transports/offline.test.ts | 52 ++++++++----------- 2 files changed, 33 insertions(+), 46 deletions(-) diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index 80422841a0b5..17fb5191a09d 100644 --- a/packages/core/src/transports/offline.ts +++ b/packages/core/src/transports/offline.ts @@ -4,7 +4,6 @@ import { forEachEnvelopeItem, logger, parseRetryAfterHeader } from '@sentry/util export const MIN_DELAY = 100; // 100 ms export const START_DELAY = 5_000; // 5 seconds const MAX_DELAY = 3.6e6; // 1 hour -const DEFAULT_QUEUE_SIZE = 30; function isReplayEnvelope(envelope: Envelope): boolean { let isReplay = false; @@ -22,13 +21,18 @@ function log(msg: string, error?: Error): void { __DEBUG_BUILD__ && logger.info(`[Offline]: ${msg}`, error); } -interface OfflineTransportOptions extends InternalBaseTransportOptions { +interface OfflineStore { + insert(env: Envelope): Promise; + pop(): Promise; +} + +export type CreateOfflineStore = (options: OfflineTransportOptions) => OfflineStore; + +export interface OfflineTransportOptions extends InternalBaseTransportOptions { /** - * The maximum number of events to keep in the offline store. - * - * Defaults: 30 + * A function that creates the offline store instance. */ - maxQueueSize?: number; + createStore: CreateOfflineStore; /** * Flush the offline store shortly after startup. @@ -49,13 +53,6 @@ interface OfflineTransportOptions extends InternalBaseTransportOptions { shouldStore?: (envelope: Envelope, error: Error, retryDelay: number) => boolean | Promise; } -interface OfflineStore { - insert(env: Envelope): Promise; - pop(): Promise; -} - -export type CreateOfflineStore = (maxQueueCount: number) => OfflineStore; - type Timer = number | { unref?: () => void }; /** @@ -66,12 +63,10 @@ type Timer = number | { unref?: () => void }; */ export function makeOfflineTransport( createTransport: (options: TO) => Transport, - createStore: CreateOfflineStore, ): (options: TO & OfflineTransportOptions) => Transport { return options => { const transport = createTransport(options); - const maxQueueSize = options.maxQueueSize === undefined ? DEFAULT_QUEUE_SIZE : options.maxQueueSize; - const store = createStore(maxQueueSize); + const store = options.createStore(options); let retryDelay = START_DELAY; let flushTimer: Timer | undefined; diff --git a/packages/core/test/lib/transports/offline.test.ts b/packages/core/test/lib/transports/offline.test.ts index d2fefa4a2948..33fc465d2bcd 100644 --- a/packages/core/test/lib/transports/offline.test.ts +++ b/packages/core/test/lib/transports/offline.test.ts @@ -17,7 +17,7 @@ import { import { TextEncoder } from 'util'; import { createTransport } from '../../../src'; -import type { CreateOfflineStore } from '../../../src/transports/offline'; +import type { CreateOfflineStore, OfflineTransportOptions } from '../../../src/transports/offline'; import { makeOfflineTransport, MIN_DELAY, START_DELAY } from '../../../src/transports/offline'; const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ @@ -94,9 +94,9 @@ function createTestStore(...popResults: MockResult[]): { return { getCalls: () => calls, - store: (maxQueueCount: number) => ({ + store: (_: OfflineTransportOptions) => ({ insert: async env => { - if (popResults.length < maxQueueCount) { + if (popResults.length < 30) { popResults.push(env); calls.push('add'); } @@ -124,11 +124,9 @@ describe('makeOfflineTransport', () => { const { getCalls, store } = createTestStore(); const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }); let queuedCount = 0; - const transport = makeOfflineTransport( - baseTransport, - store, - )({ + const transport = makeOfflineTransport(baseTransport)({ ...transportOptions, + createStore: store, shouldStore: () => { queuedCount += 1; return true; @@ -149,7 +147,7 @@ describe('makeOfflineTransport', () => { it('After successfully sending, sends further envelopes found in the store', async () => { const { getCalls, store } = createTestStore(ERROR_ENVELOPE); const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }, { statusCode: 200 }); - const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const transport = makeOfflineTransport(baseTransport)({ ...transportOptions, createStore: store }); const result = await transport.send(ERROR_ENVELOPE); expect(result).toEqual({ statusCode: 200 }); @@ -165,11 +163,9 @@ describe('makeOfflineTransport', () => { const { getCalls, store } = createTestStore(); const { getSendCount, baseTransport } = createTestTransport(new Error()); let queuedCount = 0; - const transport = makeOfflineTransport( - baseTransport, - store, - )({ + const transport = makeOfflineTransport(baseTransport)({ ...transportOptions, + createStore: store, shouldStore: () => { queuedCount += 1; return true; @@ -190,11 +186,9 @@ describe('makeOfflineTransport', () => { const { getCalls, store } = createTestStore(); const { getSendCount, baseTransport } = createTestTransport({ statusCode: 500 }); let queuedCount = 0; - const transport = makeOfflineTransport( - baseTransport, - store, - )({ + const transport = makeOfflineTransport(baseTransport)({ ...transportOptions, + createStore: store, shouldStore: () => { queuedCount += 1; return true; @@ -216,7 +210,7 @@ describe('makeOfflineTransport', () => { async () => { const { getCalls, store } = createTestStore(); const { getSendCount, baseTransport } = createTestTransport(new Error(), { statusCode: 200 }); - const transport = makeOfflineTransport(baseTransport, store)(transportOptions); + const transport = makeOfflineTransport(baseTransport)({ ...transportOptions, createStore: store }); const result = await transport.send(ERROR_ENVELOPE); expect(result).toEqual({}); expect(getCalls()).toEqual(['add']); @@ -235,7 +229,11 @@ describe('makeOfflineTransport', () => { const { getCalls, store } = createTestStore(ERROR_ENVELOPE, ERROR_ENVELOPE); const { getSendCount, baseTransport } = createTestTransport({ statusCode: 200 }, { statusCode: 200 }); // eslint-disable-next-line @typescript-eslint/no-unused-vars - const _transport = makeOfflineTransport(baseTransport, store)({ ...transportOptions, flushAtStartup: true }); + const _transport = makeOfflineTransport(baseTransport)({ + ...transportOptions, + createStore: store, + flushAtStartup: true, + }); await delay(START_DELAY + 1_000); @@ -249,11 +247,9 @@ describe('makeOfflineTransport', () => { const { getCalls, store } = createTestStore(); const { getSendCount, baseTransport } = createTestTransport(new Error()); const queuedCount = 0; - const transport = makeOfflineTransport( - baseTransport, - store, - )({ + const transport = makeOfflineTransport(baseTransport)({ ...transportOptions, + createStore: store, shouldStore: () => false, }); const result = transport.send(ERROR_ENVELOPE); @@ -268,11 +264,9 @@ describe('makeOfflineTransport', () => { const { getCalls, store } = createTestStore(); const { getSendCount, baseTransport } = createTestTransport(new Error()); const queuedCount = 0; - const transport = makeOfflineTransport( - baseTransport, - store, - )({ + const transport = makeOfflineTransport(baseTransport)({ ...transportOptions, + createStore: store, shouldStore: () => true, }); const result = transport.send(RELAY_ENVELOPE); @@ -294,11 +288,9 @@ describe('makeOfflineTransport', () => { ); let queuedCount = 0; - const transport = makeOfflineTransport( - baseTransport, - store, - )({ + const transport = makeOfflineTransport(baseTransport)({ ...transportOptions, + createStore: store, shouldStore: () => { queuedCount += 1; return true; From c0938e4f110b9ddb3edab3d9ab873b242f28ec6c Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 26 Jan 2023 19:24:50 +0000 Subject: [PATCH 07/17] createStore needs to be optional --- packages/core/src/transports/offline.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index 17fb5191a09d..2b83460aeccb 100644 --- a/packages/core/src/transports/offline.ts +++ b/packages/core/src/transports/offline.ts @@ -32,7 +32,7 @@ export interface OfflineTransportOptions extends InternalBaseTransportOptions { /** * A function that creates the offline store instance. */ - createStore: CreateOfflineStore; + createStore?: CreateOfflineStore; /** * Flush the offline store shortly after startup. @@ -66,7 +66,7 @@ export function makeOfflineTransport( ): (options: TO & OfflineTransportOptions) => Transport { return options => { const transport = createTransport(options); - const store = options.createStore(options); + const store = options.createStore ? options.createStore(options) : undefined; let retryDelay = START_DELAY; let flushTimer: Timer | undefined; @@ -87,6 +87,10 @@ export function makeOfflineTransport( } function flushIn(delay: number): void { + if (!store) { + return; + } + if (flushTimer) { clearTimeout(flushTimer as ReturnType); } @@ -143,7 +147,7 @@ export function makeOfflineTransport( retryDelay = START_DELAY; return result; } catch (e) { - if (await shouldQueue(envelope, e, retryDelay)) { + if (store && (await shouldQueue(envelope, e, retryDelay))) { await store.insert(envelope); flushWithBackOff(); log('Error sending. Event queued', e); From 80703375ec6a90cec18b94110a6e505e3459bdb9 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 29 Jan 2023 01:08:06 +0000 Subject: [PATCH 08/17] Remove redundant jsdoc param --- packages/core/src/transports/offline.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index 2b83460aeccb..9f727f4cc1c6 100644 --- a/packages/core/src/transports/offline.ts +++ b/packages/core/src/transports/offline.ts @@ -59,7 +59,6 @@ type Timer = number | { unref?: () => void }; * Wraps a transport and stores and retries events when they fail to send. * * @param createTransport The transport to wrap. - * @param createStore The store implementation to use. */ export function makeOfflineTransport( createTransport: (options: TO) => Transport, From fd9bb075e6ba6edae994c811eb090b83d2cb96be Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 30 Jan 2023 11:55:18 +0000 Subject: [PATCH 09/17] Review changes and add some exports --- packages/core/src/index.ts | 1 + packages/core/src/transports/offline.ts | 10 +++------- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a3287fe7d65a..e63591f4bc99 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,5 +1,6 @@ export type { ClientClass } from './sdk'; export type { Carrier, Layer } from './hub'; +export type { OfflineStore, OfflineTransportOptions } from './transports/offline'; export { addBreadcrumb, diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index 9f727f4cc1c6..9105ad63558a 100644 --- a/packages/core/src/transports/offline.ts +++ b/packages/core/src/transports/offline.ts @@ -21,7 +21,7 @@ function log(msg: string, error?: Error): void { __DEBUG_BUILD__ && logger.info(`[Offline]: ${msg}`, error); } -interface OfflineStore { +export interface OfflineStore { insert(env: Envelope): Promise; pop(): Promise; } @@ -107,7 +107,7 @@ export function makeOfflineTransport( }, delay) as Timer; // We need to unref the timer in node.js, otherwise the node process never exit. - if (typeof flushTimer !== 'number' && typeof flushTimer.unref === 'function') { + if (typeof flushTimer !== 'number' && flushTimer.unref) { flushTimer.unref(); } } @@ -119,11 +119,7 @@ export function makeOfflineTransport( flushIn(retryDelay); - retryDelay *= 2; - - if (retryDelay > MAX_DELAY) { - retryDelay = MAX_DELAY; - } + retryDelay = Math.min(retryDelay * 2, MAX_DELAY); } async function send(envelope: Envelope): Promise { From f788ff39cbc02181edcc5007a18a9c350989aa57 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 30 Jan 2023 23:21:54 +0000 Subject: [PATCH 10/17] indexedDb store --- packages/browser/package.json | 1 + packages/browser/src/exports.ts | 2 +- packages/browser/src/transports/index.ts | 1 + packages/browser/src/transports/offline.ts | 158 ++++++++++++++++++ .../test/unit/transports/offline.test.ts | 111 ++++++++++++ packages/utils/src/envelope.ts | 2 +- yarn.lock | 58 +++++++ 7 files changed, 331 insertions(+), 2 deletions(-) create mode 100644 packages/browser/src/transports/offline.ts create mode 100644 packages/browser/test/unit/transports/offline.test.ts diff --git a/packages/browser/package.json b/packages/browser/package.json index a78571d0bb61..faaf772698ca 100644 --- a/packages/browser/package.json +++ b/packages/browser/package.json @@ -27,6 +27,7 @@ "btoa": "^1.2.1", "chai": "^4.1.2", "chokidar": "^3.0.2", + "fake-indexeddb": "^4.0.1", "karma": "^6.3.16", "karma-chai": "^0.1.0", "karma-chrome-launcher": "^2.2.0", diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 97a7b5e93b01..80db6197a8c0 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -47,7 +47,7 @@ export { export { WINDOW } from './helpers'; export { BrowserClient } from './client'; -export { makeFetchTransport, makeXHRTransport } from './transports'; +export { makeFetchTransport, makeXHRTransport, makeOfflineTransport } from './transports'; export { defaultStackParser, defaultStackLineParsers, diff --git a/packages/browser/src/transports/index.ts b/packages/browser/src/transports/index.ts index c30287e3e616..aadcb8e6ae15 100644 --- a/packages/browser/src/transports/index.ts +++ b/packages/browser/src/transports/index.ts @@ -1,2 +1,3 @@ export { makeFetchTransport } from './fetch'; export { makeXHRTransport } from './xhr'; +export { makeOfflineTransport } from './offline'; diff --git a/packages/browser/src/transports/offline.ts b/packages/browser/src/transports/offline.ts new file mode 100644 index 000000000000..3051a92f8b4c --- /dev/null +++ b/packages/browser/src/transports/offline.ts @@ -0,0 +1,158 @@ +import type { OfflineStore, OfflineTransportOptions } from '@sentry/core'; +import { makeOfflineTransport as makeOfflineTransportBase } from '@sentry/core'; +import type { Envelope, InternalBaseTransportOptions, Transport } from '@sentry/types'; +import type { TextDecoderInternal } from '@sentry/utils'; +import { parseEnvelope, serializeEnvelope } from '@sentry/utils'; + +// 'Store', 'promisifyRequest' and 'createStore' were originally copied from the 'idb-keyval' package before being +// modified and simplified: https://github.com/jakearchibald/idb-keyval +// +// At commit: 0420a704fd6cbb4225429c536b1f61112d012fca +// Original licence: + +// Copyright 2016, Jake Archibald +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +type Store = (callback: (store: IDBObjectStore) => T | PromiseLike) => Promise; + +function promisifyRequest(request: IDBRequest | IDBTransaction): Promise { + return new Promise((resolve, reject) => { + // @ts-ignore - file size hacks + request.oncomplete = request.onsuccess = () => resolve(request.result); + // @ts-ignore - file size hacks + request.onabort = request.onerror = () => reject(request.error); + }); +} + +/** Create or open an IndexedDb store */ +export function createStore(dbName: string, storeName: string): Store { + const request = indexedDB.open(dbName); + request.onupgradeneeded = () => request.result.createObjectStore(storeName); + const dbp = promisifyRequest(request); + + return callback => dbp.then(db => callback(db.transaction(storeName, 'readwrite').objectStore(storeName))); +} + +function keys(store: IDBObjectStore): Promise { + return promisifyRequest(store.getAllKeys() as IDBRequest); +} + +/** Insert into the store */ +export function insert(store: Store, value: Uint8Array | string, maxQueueSize: number): Promise { + return store(store => { + return keys(store).then(keys => { + if (keys.length >= maxQueueSize) { + return; + } + + // We insert with an incremented key so that the entries are popped in order + store.put(value, Math.max(...keys, 0) + 1); + return promisifyRequest(store.transaction); + }); + }); +} + +/** Pop the oldest value from the store */ +export function pop(store: Store): Promise { + return store(store => { + return keys(store).then(keys => { + if (keys.length === 0) { + return undefined; + } + + return promisifyRequest(store.get(keys[0])).then(value => { + store.delete(keys[0]); + return promisifyRequest(store.transaction).then(() => value); + }); + }); + }); +} + +interface IndexedDbOptions extends OfflineTransportOptions { + /** + * Name of indexedDb database to store envelopes in + * Default: 'sentry-offline' + */ + dbName?: string; + /** + * Name of indexedDb object store to store envelopes in + * Default: 'queue' + */ + storeName?: string; + /** + * Maximum number of envelopes to store + * Default: 30 + */ + maxQueueSize?: number; + /** + * Only required for testing on node.js + * @ignore + */ + textDecoder?: TextDecoderInternal; +} + +function createIndexedDbStore(options: IndexedDbOptions): OfflineStore { + let store: Store | undefined; + + // Lazily create the store only when it's needed + function getStore(): Store { + if (store == undefined) { + store = createStore(options.dbName || 'sentry-offline', options.storeName || 'queue'); + } + + return store; + } + + return { + insert: async (env: Envelope) => { + try { + const serialized = await serializeEnvelope(env, options.textEncoder); + await insert(getStore(), serialized, options.maxQueueSize || 30); + } catch (_) { + // + } + }, + pop: async () => { + try { + const deserialized = await pop(getStore()); + if (deserialized) { + return parseEnvelope( + deserialized, + options.textEncoder || new TextEncoder(), + options.textDecoder || new TextDecoder(), + ); + } + } catch (_) { + // + } + + return undefined; + }, + }; +} + +function makeIndexedDbOfflineTransport( + createTransport: (options: T) => Transport, +): (options: T & IndexedDbOptions) => Transport { + return options => createTransport({ ...options, createStore: createIndexedDbStore }); +} + +/** + * Creates a transport that uses IndexedDb to store events when offline. + */ +export function makeOfflineTransport( + createTransport: (options: T) => Transport, +): (options: T & IndexedDbOptions) => Transport { + return makeIndexedDbOfflineTransport(makeOfflineTransportBase(createTransport)); +} diff --git a/packages/browser/test/unit/transports/offline.test.ts b/packages/browser/test/unit/transports/offline.test.ts new file mode 100644 index 000000000000..a90253b8b87a --- /dev/null +++ b/packages/browser/test/unit/transports/offline.test.ts @@ -0,0 +1,111 @@ +import 'fake-indexeddb/auto'; + +import { createTransport } from '@sentry/core'; +import type { + EventEnvelope, + EventItem, + InternalBaseTransportOptions, + TransportMakeRequestResponse, +} from '@sentry/types'; +import { createEnvelope } from '@sentry/utils'; +import { TextDecoder, TextEncoder } from 'util'; + +import { MIN_DELAY } from '../../../../core/src/transports/offline'; +import { createStore, insert, makeOfflineTransport, pop } from '../../../src/transports/offline'; + +function deleteDatabase(name: string): Promise { + return new Promise((resolve, reject) => { + const request = indexedDB.deleteDatabase(name); + request.onsuccess = () => resolve(); + request.onerror = () => reject(request.error); + }); +} + +const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ + [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, +]); + +const transportOptions = { + recordDroppedEvent: () => undefined, // noop + textEncoder: new TextEncoder(), + textDecoder: new TextDecoder(), +}; + +type MockResult = T | Error; + +export const createTestTransport = (...sendResults: MockResult[]) => { + let sendCount = 0; + + return { + getSendCount: () => sendCount, + baseTransport: (options: InternalBaseTransportOptions) => + createTransport(options, () => { + return new Promise((resolve, reject) => { + const next = sendResults.shift(); + + if (next instanceof Error) { + reject(next); + } else { + sendCount += 1; + resolve(next as TransportMakeRequestResponse | undefined); + } + }); + }), + }; +}; + +function delay(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +describe('makeOfflineTransport', () => { + beforeAll(async () => { + await deleteDatabase('sentry'); + }); + + it('indexedDb wrappers insert and pop', async () => { + const store = createStore('test', 'test'); + const found = await pop(store); + expect(found).toBeUndefined(); + + await insert(store, 'test1', 30); + await insert(store, new Uint8Array([1, 2, 3, 4, 5]), 30); + + const found2 = await pop(store); + expect(found2).toEqual('test1'); + const found3 = await pop(store); + expect(found3).toEqual(new Uint8Array([1, 2, 3, 4, 5])); + + const found4 = await pop(store); + expect(found4).toBeUndefined(); + }); + + it('Queues and retries envelope if wrapped transport throws error', async () => { + const { getSendCount, baseTransport } = createTestTransport(new Error(), { statusCode: 200 }, { statusCode: 200 }); + let queuedCount = 0; + const transport = makeOfflineTransport(baseTransport)({ + ...transportOptions, + shouldStore: () => { + queuedCount += 1; + return true; + }, + }); + const result = await transport.send(ERROR_ENVELOPE); + + expect(result).toEqual({}); + + await delay(MIN_DELAY * 2); + + expect(getSendCount()).toEqual(0); + expect(queuedCount).toEqual(1); + + // Sending again will retry the queued envelope too + const result2 = await transport.send(ERROR_ENVELOPE); + expect(result2).toEqual({ statusCode: 200 }); + + await delay(MIN_DELAY * 2); + + expect(queuedCount).toEqual(1); + expect(getSendCount()).toEqual(2); + }); +}); diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index b5d315e1cd81..82de32a2e67f 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -115,7 +115,7 @@ function concatBuffers(buffers: Uint8Array[]): Uint8Array { return merged; } -interface TextDecoderInternal { +export interface TextDecoderInternal { decode(input?: Uint8Array): string; } diff --git a/yarn.lock b/yarn.lock index c2dbdbaef239..0dde8e922de5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7067,6 +7067,11 @@ balanced-match@^1.0.0: resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.2.tgz#e83e3a7e3f300b34cb9d87f615fa0cbf357690ee" integrity sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw== +base64-arraybuffer-es6@^0.7.0: + version "0.7.0" + resolved "https://registry.yarnpkg.com/base64-arraybuffer-es6/-/base64-arraybuffer-es6-0.7.0.tgz#dbe1e6c87b1bf1ca2875904461a7de40f21abc86" + integrity sha512-ESyU/U1CFZDJUdr+neHRhNozeCv72Y7Vm0m1DCbjX3KBjT6eYocvAJlSk6+8+HkVwXlT1FNxhGW6q3UKAlCvvw== + base64-arraybuffer@^1.0.1: version "1.0.2" resolved "https://registry.yarnpkg.com/base64-arraybuffer/-/base64-arraybuffer-1.0.2.tgz#1c37589a7c4b0746e34bd1feb951da2df01c1bdc" @@ -10250,6 +10255,13 @@ domelementtype@^2.0.1, domelementtype@^2.2.0: resolved "https://registry.yarnpkg.com/domelementtype/-/domelementtype-2.2.0.tgz#9a0b6c2782ed6a1c7323d42267183df9bd8b1d57" integrity sha512-DtBMo82pv1dFtUmHyr48beiuq792Sxohr+8Hm9zoxklYPfa6n0Z3Byjj2IV7bmr2IyqClnqEQhfgHJJ5QF0R5A== +domexception@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/domexception/-/domexception-1.0.1.tgz#937442644ca6a31261ef36e3ec677fe805582c90" + integrity sha512-raigMkn7CJNNo6Ihro1fzG7wr3fHuYVytzquZKX5n0yizGsTcYgzdIUwj1X9pK0VvjeihV+XiclP+DjwbsSKug== + dependencies: + webidl-conversions "^4.0.2" + domexception@^2.0.1: version "2.0.1" resolved "https://registry.yarnpkg.com/domexception/-/domexception-2.0.1.tgz#fb44aefba793e1574b0af6aed2801d057529f304" @@ -11975,6 +11987,13 @@ extsprintf@^1.2.0: resolved "https://registry.yarnpkg.com/extsprintf/-/extsprintf-1.4.0.tgz#e2689f8f356fad62cca65a3a91c5df5f9551692f" integrity sha1-4mifjzVvrWLMplo6kcXfX5VRaS8= +fake-indexeddb@^4.0.1: + version "4.0.1" + resolved "https://registry.yarnpkg.com/fake-indexeddb/-/fake-indexeddb-4.0.1.tgz#09bb2468e21d0832b2177e894765fb109edac8fb" + integrity sha512-hFRyPmvEZILYgdcLBxVdHLik4Tj3gDTu/g7s9ZDOiU3sTNiGx+vEu1ri/AMsFJUZ/1sdRbAVrEcKndh3sViBcA== + dependencies: + realistic-structured-clone "^3.0.0" + fast-deep-equal@^3.1.1, fast-deep-equal@^3.1.3: version "3.1.3" resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz#3a7d56b559d6cbc3eb512325244e619a65c6c525" @@ -20878,6 +20897,15 @@ readdirp@~3.6.0: dependencies: picomatch "^2.2.1" +realistic-structured-clone@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/realistic-structured-clone/-/realistic-structured-clone-3.0.0.tgz#7b518049ce2dad41ac32b421cd297075b00e3e35" + integrity sha512-rOjh4nuWkAqf9PWu6JVpOWD4ndI+JHfgiZeMmujYcPi+fvILUu7g6l26TC1K5aBIp34nV+jE1cDO75EKOfHC5Q== + dependencies: + domexception "^1.0.1" + typeson "^6.1.0" + typeson-registry "^1.0.0-alpha.20" + realpath-native@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/realpath-native/-/realpath-native-1.1.0.tgz#2003294fea23fb0672f2476ebe22fcf498a2d65c" @@ -23774,6 +23802,13 @@ tr46@^2.0.2: dependencies: punycode "^2.1.1" +tr46@^2.1.0: + version "2.1.0" + resolved "https://registry.yarnpkg.com/tr46/-/tr46-2.1.0.tgz#fa87aa81ca5d5941da8cbf1f9b749dc969a4e240" + integrity sha512-15Ih7phfcdP5YxqiB+iDtLoaTz4Nd35+IiAv0kQ5FNKHzXgdWqPoTIqEDDJmXceQt4JZk6lVPT8lnDlPpGDppw== + dependencies: + punycode "^2.1.1" + tr46@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/tr46/-/tr46-3.0.0.tgz#555c4e297a950617e8eeddef633c87d4d9d6cbf9" @@ -24090,6 +24125,20 @@ typescript@~4.5.2: resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.5.5.tgz#d8c953832d28924a9e3d37c73d729c846c5896f3" integrity sha512-TCTIul70LyWe6IJWT8QSYeA54WQe8EjQFU4wY52Fasj5UKx88LNYKCgBEHcOMOrFF1rKGbD8v/xcNWVUq9SymA== +typeson-registry@^1.0.0-alpha.20: + version "1.0.0-alpha.39" + resolved "https://registry.yarnpkg.com/typeson-registry/-/typeson-registry-1.0.0-alpha.39.tgz#9e0f5aabd5eebfcffd65a796487541196f4b1211" + integrity sha512-NeGDEquhw+yfwNhguLPcZ9Oj0fzbADiX4R0WxvoY8nGhy98IbzQy1sezjoEFWOywOboj/DWehI+/aUlRVrJnnw== + dependencies: + base64-arraybuffer-es6 "^0.7.0" + typeson "^6.0.0" + whatwg-url "^8.4.0" + +typeson@^6.0.0, typeson@^6.1.0: + version "6.1.0" + resolved "https://registry.yarnpkg.com/typeson/-/typeson-6.1.0.tgz#5b2a53705a5f58ff4d6f82f965917cabd0d7448b" + integrity sha512-6FTtyGr8ldU0pfbvW/eOZrEtEkczHRUtduBnA90Jh9kMPCiFNnXIon3vF41N0S4tV1HHQt4Hk1j4srpESziCaA== + ua-parser-js@^0.7.18, ua-parser-js@^0.7.30: version "0.7.31" resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-0.7.31.tgz#649a656b191dffab4f21d5e053e27ca17cbff5c6" @@ -25102,6 +25151,15 @@ whatwg-url@^8.0.0, whatwg-url@^8.5.0: tr46 "^2.0.2" webidl-conversions "^6.1.0" +whatwg-url@^8.4.0: + version "8.7.0" + resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-8.7.0.tgz#656a78e510ff8f3937bc0bcbe9f5c0ac35941b77" + integrity sha512-gAojqb/m9Q8a5IV96E3fHJM70AzCkgt4uXYX2O7EmuyOnLrViCQlsEBmF9UQIu3/aeAIp2U17rtbpZWNntQqdg== + dependencies: + lodash "^4.7.0" + tr46 "^2.1.0" + webidl-conversions "^6.1.0" + when@~3.6.x: version "3.6.4" resolved "https://registry.yarnpkg.com/when/-/when-3.6.4.tgz#473b517ec159e2b85005497a13983f095412e34e" From 5f61a46b4b0d188cab90f9d04ce056b6510fb55f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 31 Jan 2023 10:13:04 +0000 Subject: [PATCH 11/17] rename --- packages/browser/src/transports/offline.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/browser/src/transports/offline.ts b/packages/browser/src/transports/offline.ts index 3051a92f8b4c..256019effd53 100644 --- a/packages/browser/src/transports/offline.ts +++ b/packages/browser/src/transports/offline.ts @@ -79,7 +79,7 @@ export function pop(store: Store): Promise { }); } -interface IndexedDbOptions extends OfflineTransportOptions { +interface BrowserOfflineTransportOptions extends OfflineTransportOptions { /** * Name of indexedDb database to store envelopes in * Default: 'sentry-offline' @@ -102,7 +102,7 @@ interface IndexedDbOptions extends OfflineTransportOptions { textDecoder?: TextDecoderInternal; } -function createIndexedDbStore(options: IndexedDbOptions): OfflineStore { +function createIndexedDbStore(options: BrowserOfflineTransportOptions): OfflineStore { let store: Store | undefined; // Lazily create the store only when it's needed @@ -144,7 +144,7 @@ function createIndexedDbStore(options: IndexedDbOptions): OfflineStore { function makeIndexedDbOfflineTransport( createTransport: (options: T) => Transport, -): (options: T & IndexedDbOptions) => Transport { +): (options: T & BrowserOfflineTransportOptions) => Transport { return options => createTransport({ ...options, createStore: createIndexedDbStore }); } @@ -153,6 +153,6 @@ function makeIndexedDbOfflineTransport( */ export function makeOfflineTransport( createTransport: (options: T) => Transport, -): (options: T & IndexedDbOptions) => Transport { +): (options: T & BrowserOfflineTransportOptions) => Transport { return makeIndexedDbOfflineTransport(makeOfflineTransportBase(createTransport)); } From 3c20d984fc9cc9311d33e36cc9481594a2900c52 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 31 Jan 2023 10:37:51 +0000 Subject: [PATCH 12/17] rename for no clash --- packages/browser/src/exports.ts | 2 +- packages/browser/src/transports/index.ts | 2 +- packages/browser/src/transports/offline.ts | 2 +- packages/browser/test/unit/transports/offline.test.ts | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 80db6197a8c0..53e63488497e 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -47,7 +47,7 @@ export { export { WINDOW } from './helpers'; export { BrowserClient } from './client'; -export { makeFetchTransport, makeXHRTransport, makeOfflineTransport } from './transports'; +export { makeFetchTransport, makeXHRTransport, makeBrowserOfflineTransport } from './transports'; export { defaultStackParser, defaultStackLineParsers, diff --git a/packages/browser/src/transports/index.ts b/packages/browser/src/transports/index.ts index aadcb8e6ae15..ceaf1d7a5885 100644 --- a/packages/browser/src/transports/index.ts +++ b/packages/browser/src/transports/index.ts @@ -1,3 +1,3 @@ export { makeFetchTransport } from './fetch'; export { makeXHRTransport } from './xhr'; -export { makeOfflineTransport } from './offline'; +export { makeBrowserOfflineTransport } from './offline'; diff --git a/packages/browser/src/transports/offline.ts b/packages/browser/src/transports/offline.ts index 256019effd53..099967362a55 100644 --- a/packages/browser/src/transports/offline.ts +++ b/packages/browser/src/transports/offline.ts @@ -151,7 +151,7 @@ function makeIndexedDbOfflineTransport( /** * Creates a transport that uses IndexedDb to store events when offline. */ -export function makeOfflineTransport( +export function makeBrowserOfflineTransport( createTransport: (options: T) => Transport, ): (options: T & BrowserOfflineTransportOptions) => Transport { return makeIndexedDbOfflineTransport(makeOfflineTransportBase(createTransport)); diff --git a/packages/browser/test/unit/transports/offline.test.ts b/packages/browser/test/unit/transports/offline.test.ts index a90253b8b87a..b224df725bc6 100644 --- a/packages/browser/test/unit/transports/offline.test.ts +++ b/packages/browser/test/unit/transports/offline.test.ts @@ -11,7 +11,7 @@ import { createEnvelope } from '@sentry/utils'; import { TextDecoder, TextEncoder } from 'util'; import { MIN_DELAY } from '../../../../core/src/transports/offline'; -import { createStore, insert, makeOfflineTransport, pop } from '../../../src/transports/offline'; +import { createStore, insert, makeBrowserOfflineTransport, pop } from '../../../src/transports/offline'; function deleteDatabase(name: string): Promise { return new Promise((resolve, reject) => { @@ -83,7 +83,7 @@ describe('makeOfflineTransport', () => { it('Queues and retries envelope if wrapped transport throws error', async () => { const { getSendCount, baseTransport } = createTestTransport(new Error(), { statusCode: 200 }, { statusCode: 200 }); let queuedCount = 0; - const transport = makeOfflineTransport(baseTransport)({ + const transport = makeBrowserOfflineTransport(baseTransport)({ ...transportOptions, shouldStore: () => { queuedCount += 1; From 0fd220c1608203d7f12dee872167664c85392f2e Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 31 Jan 2023 10:43:15 +0000 Subject: [PATCH 13/17] rename --- packages/browser/src/transports/offline.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/browser/src/transports/offline.ts b/packages/browser/src/transports/offline.ts index 099967362a55..9bb3e5dbfe2e 100644 --- a/packages/browser/src/transports/offline.ts +++ b/packages/browser/src/transports/offline.ts @@ -1,5 +1,5 @@ import type { OfflineStore, OfflineTransportOptions } from '@sentry/core'; -import { makeOfflineTransport as makeOfflineTransportBase } from '@sentry/core'; +import { makeOfflineTransport } from '@sentry/core'; import type { Envelope, InternalBaseTransportOptions, Transport } from '@sentry/types'; import type { TextDecoderInternal } from '@sentry/utils'; import { parseEnvelope, serializeEnvelope } from '@sentry/utils'; @@ -154,5 +154,5 @@ function makeIndexedDbOfflineTransport( export function makeBrowserOfflineTransport( createTransport: (options: T) => Transport, ): (options: T & BrowserOfflineTransportOptions) => Transport { - return makeIndexedDbOfflineTransport(makeOfflineTransportBase(createTransport)); + return makeIndexedDbOfflineTransport(makeOfflineTransport(createTransport)); } From bfc2b30f0daf3bdad318027cb9a4a70d48d3799d Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 5 Feb 2023 19:45:29 +0100 Subject: [PATCH 14/17] add an integration test --- .../suites/transport/offline/init.js | 8 ++++ .../transport/offline/queued/subject.js | 3 ++ .../suites/transport/offline/queued/test.ts | 39 +++++++++++++++++++ 3 files changed, 50 insertions(+) create mode 100644 packages/integration-tests/suites/transport/offline/init.js create mode 100644 packages/integration-tests/suites/transport/offline/queued/subject.js create mode 100644 packages/integration-tests/suites/transport/offline/queued/test.ts diff --git a/packages/integration-tests/suites/transport/offline/init.js b/packages/integration-tests/suites/transport/offline/init.js new file mode 100644 index 000000000000..a69f8a32b32a --- /dev/null +++ b/packages/integration-tests/suites/transport/offline/init.js @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: Sentry.makeBrowserOfflineTransport(Sentry.makeFetchTransport), +}); diff --git a/packages/integration-tests/suites/transport/offline/queued/subject.js b/packages/integration-tests/suites/transport/offline/queued/subject.js new file mode 100644 index 000000000000..9075d7bac69a --- /dev/null +++ b/packages/integration-tests/suites/transport/offline/queued/subject.js @@ -0,0 +1,3 @@ +setTimeout(() => { + Sentry.captureMessage(`foo ${Math.random()}`); +}, 500); diff --git a/packages/integration-tests/suites/transport/offline/queued/test.ts b/packages/integration-tests/suites/transport/offline/queued/test.ts new file mode 100644 index 000000000000..db830c4a18fc --- /dev/null +++ b/packages/integration-tests/suites/transport/offline/queued/test.ts @@ -0,0 +1,39 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getMultipleSentryEnvelopeRequests } from '../../../../utils/helpers'; + +function delay(ms: number) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +sentryTest('should queue and retry events when they fail to send', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + // This would be the obvious way to test offline support but it doesn't appear to work! + // await context.setOffline(true); + + // Abort all envelope requests so the event gets queued + await page.route(/ingest\.sentry\.io/, route => route.abort()); + await page.goto(url); + await delay(1_000); + await page.unroute(/ingest\.sentry\.io/); + + // The previous event should now be queued + + // This will force the page to be reloaded and a new event to be sent + const eventData = await getMultipleSentryEnvelopeRequests(page, 3, { url, timeout: 10_000 }); + + // Filter out any client reports + const events = eventData.filter(e => !('discarded_events' in e)) as Event[]; + + expect(events).toHaveLength(2); + + // The next two events will be message events starting with 'foo' + expect(events[0].message?.startsWith('foo')); + expect(events[1].message?.startsWith('foo')); + + // But because these are two different events, they should have different random numbers in the message + expect(events[0].message !== events[1].message); +}); From cfd20a4d7974ccc1a22098a5a4491869a81f676a Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 5 Feb 2023 19:52:59 +0100 Subject: [PATCH 15/17] ensure there was an aborted request --- .../suites/transport/offline/queued/test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/integration-tests/suites/transport/offline/queued/test.ts b/packages/integration-tests/suites/transport/offline/queued/test.ts index db830c4a18fc..159186e6a519 100644 --- a/packages/integration-tests/suites/transport/offline/queued/test.ts +++ b/packages/integration-tests/suites/transport/offline/queued/test.ts @@ -14,12 +14,19 @@ sentryTest('should queue and retry events when they fail to send', async ({ getL // This would be the obvious way to test offline support but it doesn't appear to work! // await context.setOffline(true); + let abortedCount = 0; + // Abort all envelope requests so the event gets queued - await page.route(/ingest\.sentry\.io/, route => route.abort()); + await page.route(/ingest\.sentry\.io/, route => { + abortedCount += 1; + return route.abort(); + }); await page.goto(url); await delay(1_000); await page.unroute(/ingest\.sentry\.io/); + expect(abortedCount).toBe(1); + // The previous event should now be queued // This will force the page to be reloaded and a new event to be sent From 69887921da22fdb46464ac3c5e7308ecb8b309eb Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 5 Feb 2023 22:31:08 +0100 Subject: [PATCH 16/17] Fix missing export from tracing bundles --- packages/tracing/src/index.bundle.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/tracing/src/index.bundle.ts b/packages/tracing/src/index.bundle.ts index 635be146d639..ae8c168cb0fc 100644 --- a/packages/tracing/src/index.bundle.ts +++ b/packages/tracing/src/index.bundle.ts @@ -35,6 +35,7 @@ export { startTransaction, makeFetchTransport, makeXHRTransport, + makeBrowserOfflineTransport, withScope, } from '@sentry/browser'; From 83ec9a0bbdac51c542972eda680950fb2142559f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 6 Feb 2023 16:56:46 +0100 Subject: [PATCH 17/17] Exclude makeBrowserOfflineTransport from CDN bundles and remove from integration tests --- packages/browser/src/exports.ts | 2 +- packages/browser/src/index.ts | 14 ++++++++----- packages/browser/src/transports/index.ts | 1 - .../suites/transport/offline/queued/test.ts | 5 +++++ packages/tracing/src/index.bundle.ts | 1 - rollup/bundleHelpers.js | 21 +++++++++++++++---- rollup/plugins/bundlePlugins.js | 8 +++++-- 7 files changed, 38 insertions(+), 14 deletions(-) diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 53e63488497e..97a7b5e93b01 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -47,7 +47,7 @@ export { export { WINDOW } from './helpers'; export { BrowserClient } from './client'; -export { makeFetchTransport, makeXHRTransport, makeBrowserOfflineTransport } from './transports'; +export { makeFetchTransport, makeXHRTransport } from './transports'; export { defaultStackParser, defaultStackLineParsers, diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 8185cbc6259e..381055dcbef0 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -21,10 +21,14 @@ const INTEGRATIONS = { export { INTEGRATIONS as Integrations }; // DO NOT DELETE THESE COMMENTS! -// We want to exclude Replay from CDN bundles, so we remove the block below with our -// excludeReplay Rollup plugin when generating bundles. Everything between -// ROLLUP_EXCLUDE_FROM_BUNDLES_BEGIN and _END__ is removed for bundles. +// We want to exclude Replay/Offline from CDN bundles, so we remove the block below with our +// makeExcludeBlockPlugin Rollup plugin when generating bundles. Everything between +// ROLLUP_EXCLUDE_*_FROM_BUNDLES_BEGIN and _END__ is removed for bundles. -// __ROLLUP_EXCLUDE_FROM_BUNDLES_BEGIN__ +// __ROLLUP_EXCLUDE_REPLAY_FROM_BUNDLES_BEGIN__ export { Replay } from '@sentry/replay'; -// __ROLLUP_EXCLUDE_FROM_BUNDLES_END__ +// __ROLLUP_EXCLUDE_REPLAY_FROM_BUNDLES_END__ + +// __ROLLUP_EXCLUDE_OFFLINE_FROM_BUNDLES_BEGIN__ +export { makeBrowserOfflineTransport } from './transports/offline'; +// __ROLLUP_EXCLUDE_OFFLINE_FROM_BUNDLES_END__ diff --git a/packages/browser/src/transports/index.ts b/packages/browser/src/transports/index.ts index ceaf1d7a5885..c30287e3e616 100644 --- a/packages/browser/src/transports/index.ts +++ b/packages/browser/src/transports/index.ts @@ -1,3 +1,2 @@ export { makeFetchTransport } from './fetch'; export { makeXHRTransport } from './xhr'; -export { makeBrowserOfflineTransport } from './offline'; diff --git a/packages/integration-tests/suites/transport/offline/queued/test.ts b/packages/integration-tests/suites/transport/offline/queued/test.ts index 159186e6a519..61020eceb79b 100644 --- a/packages/integration-tests/suites/transport/offline/queued/test.ts +++ b/packages/integration-tests/suites/transport/offline/queued/test.ts @@ -9,6 +9,11 @@ function delay(ms: number) { } sentryTest('should queue and retry events when they fail to send', async ({ getLocalTestPath, page }) => { + // makeBrowserOfflineTransport is not included in any CDN bundles + if (process.env.PW_BUNDLE && process.env.PW_BUNDLE.startsWith('bundle_')) { + sentryTest.skip(); + } + const url = await getLocalTestPath({ testDir: __dirname }); // This would be the obvious way to test offline support but it doesn't appear to work! diff --git a/packages/tracing/src/index.bundle.ts b/packages/tracing/src/index.bundle.ts index ae8c168cb0fc..635be146d639 100644 --- a/packages/tracing/src/index.bundle.ts +++ b/packages/tracing/src/index.bundle.ts @@ -35,7 +35,6 @@ export { startTransaction, makeFetchTransport, makeXHRTransport, - makeBrowserOfflineTransport, withScope, } from '@sentry/browser'; diff --git a/rollup/bundleHelpers.js b/rollup/bundleHelpers.js index 43a5f26563d9..ffc9b1c796d0 100644 --- a/rollup/bundleHelpers.js +++ b/rollup/bundleHelpers.js @@ -16,7 +16,7 @@ import { makeSucrasePlugin, makeTerserPlugin, makeTSPlugin, - makeExcludeReplayPlugin, + makeExcludeBlockPlugin, makeSetSDKSourcePlugin, } from './plugins/index.js'; import { mergePlugins } from './utils'; @@ -24,8 +24,16 @@ import { mergePlugins } from './utils'; const BUNDLE_VARIANTS = ['.js', '.min.js', '.debug.min.js']; export function makeBaseBundleConfig(options) { - const { bundleType, entrypoints, jsVersion, licenseTitle, outputFileBase, packageSpecificConfig, includeReplay } = - options; + const { + bundleType, + entrypoints, + jsVersion, + licenseTitle, + outputFileBase, + packageSpecificConfig, + includeReplay, + includeOffline, + } = options; const nodeResolvePlugin = makeNodeResolvePlugin(); const sucrasePlugin = makeSucrasePlugin(); @@ -33,7 +41,8 @@ export function makeBaseBundleConfig(options) { const markAsBrowserBuildPlugin = makeBrowserBuildPlugin(true); const licensePlugin = makeLicensePlugin(licenseTitle); const tsPlugin = makeTSPlugin(jsVersion.toLowerCase()); - const excludeReplayPlugin = makeExcludeReplayPlugin(); + const excludeReplayPlugin = makeExcludeBlockPlugin('REPLAY'); + const excludeOfflineTransport = makeExcludeBlockPlugin('OFFLINE'); // The `commonjs` plugin is the `esModuleInterop` of the bundling world. When used with `transformMixedEsModules`, it // will include all dependencies, imported or required, in the final bundle. (Without it, CJS modules aren't included @@ -54,6 +63,10 @@ export function makeBaseBundleConfig(options) { standAloneBundleConfig.plugins.push(excludeReplayPlugin); } + if (!includeOffline) { + standAloneBundleConfig.plugins.push(excludeOfflineTransport); + } + // used by `@sentry/integrations` and `@sentry/wasm` (bundles which need to be combined with a stand-alone SDK bundle) const addOnBundleConfig = { // These output settings are designed to mimic an IIFE. We don't use Rollup's `iife` format because we don't want to diff --git a/rollup/plugins/bundlePlugins.js b/rollup/plugins/bundlePlugins.js index 7e48452e3d33..be91a495fc1e 100644 --- a/rollup/plugins/bundlePlugins.js +++ b/rollup/plugins/bundlePlugins.js @@ -187,8 +187,12 @@ export function makeTSPlugin(jsVersion) { * from the browser and browser+tracing bundles. * If we need to add more such guards in the future, we might want to refactor this into a more generic plugin. */ -export function makeExcludeReplayPlugin() { - const replacementRegex = /\/\/ __ROLLUP_EXCLUDE_FROM_BUNDLES_BEGIN__(.|\n)*__ROLLUP_EXCLUDE_FROM_BUNDLES_END__/m; +export function makeExcludeBlockPlugin(type) { + const replacementRegex = new RegExp( + `\\/\\/ __ROLLUP_EXCLUDE_${type}_FROM_BUNDLES_BEGIN__(.|\n)*__ROLLUP_EXCLUDE_${type}_FROM_BUNDLES_END__`, + 'm', + ); + const browserIndexFilePath = path.resolve(__dirname, '../../packages/browser/src/index.ts'); const plugin = {