getsentry · Lms24 · Apr 9, 2024 · Apr 3, 2024 · Apr 5, 2024 · Apr 5, 2024
diff --git a/dev-packages/browser-integration-tests/playwright.config.ts b/dev-packages/browser-integration-tests/playwright.config.ts
@@ -11,7 +11,7 @@ const config: PlaywrightTestConfig = {
   testMatch: /test.ts/,
 
   use: {
-    trace: process.env.CI ? 'retry-with-trace' : 'off',
+    trace: process.env.CI ? 'retain-on-failure' : 'off',
   },
 
   projects: [

@@ -0,0 +1,17 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+window.Replay = Sentry.replayIntegration({
+  flushMinDelay: 200,
+  flushMaxDelay: 200,
+  minReplayDuration: 0,
+});
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  sampleRate: 0,
+  replaysSessionSampleRate: 1.0,
+  replaysOnErrorSampleRate: 0.0,
+  transport: Sentry.makeBrowserOfflineTransport(),
+  integrations: [window.Replay],
+});
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+    <button onclick="console.log('Test log')">Click me</button>
+  </body>
+</html>
@@ -0,0 +1,45 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../utils/fixtures';
+import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers';
+
+sentryTest('should capture replays offline', async ({ getLocalTestPath, page }) => {
+  // makeBrowserOfflineTransport is not included in any CDN bundles
+  if (shouldSkipReplayTest() || (process.env.PW_BUNDLE && process.env.PW_BUNDLE.startsWith('bundle'))) {
+    sentryTest.skip();
+  }
+
+  const reqPromise0 = waitForReplayRequest(page, 0);
+  const reqPromise1 = waitForReplayRequest(page, 1);
+
+  await page.route('https://dsn.ingest.sentry.io/**/*', route => {
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  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 the first envelope request so the event gets queued
+  await page.route(/ingest\.sentry\.io/, route => route.abort('internetdisconnected'), { times: 1 });
+
+  await page.goto(url);
+
+  await new Promise(resolve => setTimeout(resolve, 2_000));
+
+  // Now send a second event which should be queued after the the first one and force flushing the queue
+  await page.locator('button').click();
+
+  const replayEvent0 = getReplayEvent(await reqPromise0);
+  const replayEvent1 = getReplayEvent(await reqPromise1);
+
+  // Check that we received the envelopes in the correct order
+  expect(replayEvent0.timestamp).toBeGreaterThan(0);
+  expect(replayEvent1.timestamp).toBeGreaterThan(0);
+  expect(replayEvent0.timestamp).toBeLessThan(replayEvent1.timestamp || 0);
+});
diff --git a/dev-packages/browser-integration-tests/suites/transport/offline/init.js b/dev-packages/browser-integration-tests/suites/transport/offline/init.js
@@ -4,5 +4,5 @@ window.Sentry = Sentry;
 
 Sentry.init({
   dsn: 'https://[email protected]/1337',
-  transport: Sentry.makeBrowserOfflineTransport(Sentry.makeFetchTransport),
+  transport: Sentry.makeBrowserOfflineTransport(),
 });
diff --git a/packages/browser/src/transports/offline.ts b/packages/browser/src/transports/offline.ts
@@ -48,8 +48,8 @@ function keys(store: IDBObjectStore): Promise<number[]> {
   return promisifyRequest(store.getAllKeys() as IDBRequest<number[]>);
 }
 
-/** Insert into the store */
-export function insert(store: Store, value: Uint8Array | string, maxQueueSize: number): Promise<void> {
+/** Insert into the end of the store */
+export function push(store: Store, value: Uint8Array | string, maxQueueSize: number): Promise<void> {
   return store(store => {
     return keys(store).then(keys => {
       if (keys.length >= maxQueueSize) {
@@ -63,8 +63,23 @@ export function insert(store: Store, value: Uint8Array | string, maxQueueSize: n
   });
 }
 
+/** Insert into the front of the store */
+export function unshift(store: Store, value: Uint8Array | string, maxQueueSize: number): Promise<void> {
+  return store(store => {
+    return keys(store).then(keys => {
+      if (keys.length >= maxQueueSize) {
+        return;
+      }
+
+      // We insert with an decremented key so that the entries are popped in order
+      store.put(value, Math.min(...keys, 0) - 1);
+      return promisifyRequest(store.transaction);
+    });
+  });
+}
+
 /** Pop the oldest value from the store */
-export function pop(store: Store): Promise<Uint8Array | string | undefined> {
+export function shift(store: Store): Promise<Uint8Array | string | undefined> {
   return store(store => {
     return keys(store).then(keys => {
       if (keys.length === 0) {
@@ -79,7 +94,7 @@ export function pop(store: Store): Promise<Uint8Array | string | undefined> {
   });
 }
 
-export interface BrowserOfflineTransportOptions extends OfflineTransportOptions {
+export interface BrowserOfflineTransportOptions extends Omit<OfflineTransportOptions, 'createStore'> {
   /**
    * Name of indexedDb database to store envelopes in
    * Default: 'sentry-offline'
@@ -110,17 +125,25 @@ function createIndexedDbStore(options: BrowserOfflineTransportOptions): OfflineS
   }
 
   return {
-    insert: async (env: Envelope) => {
+    push: async (env: Envelope) => {
+      try {
+        const serialized = await serializeEnvelope(env);
+        await push(getStore(), serialized, options.maxQueueSize || 30);
+      } catch (_) {
+        //
+      }
+    },
+    unshift: async (env: Envelope) => {
       try {
         const serialized = await serializeEnvelope(env);
-        await insert(getStore(), serialized, options.maxQueueSize || 30);
+        await unshift(getStore(), serialized, options.maxQueueSize || 30);
       } catch (_) {
         //
       }
     },
-    pop: async () => {
+    shift: async () => {
       try {
-        const deserialized = await pop(getStore());
+        const deserialized = await shift(getStore());
         if (deserialized) {
           return parseEnvelope(deserialized);
         }

diff --git a/packages/browser/test/unit/transports/offline.test.ts b/packages/browser/test/unit/transports/offline.test.ts
@@ -11,7 +11,7 @@ import type {
 import { createEnvelope } from '@sentry/utils';
 
 import { MIN_DELAY } from '../../../../core/src/transports/offline';
-import { createStore, insert, makeBrowserOfflineTransport, pop } from '../../../src/transports/offline';
+import { createStore, makeBrowserOfflineTransport, push, shift, unshift } from '../../../src/transports/offline';
 
 function deleteDatabase(name: string): Promise<void> {
   return new Promise<void>((resolve, reject) => {
@@ -63,21 +63,24 @@ describe('makeOfflineTransport', () => {
     (global as any).TextDecoder = TextDecoder;
   });
 
-  it('indexedDb wrappers insert and pop', async () => {
+  it('indexedDb wrappers push, unshift and pop', async () => {
     const store = createStore('test', 'test');
-    const found = await pop(store);
+    const found = await shift(store);
     expect(found).toBeUndefined();
 
-    await insert(store, 'test1', 30);
-    await insert(store, new Uint8Array([1, 2, 3, 4, 5]), 30);
+    await push(store, 'test1', 30);
+    await push(store, new Uint8Array([1, 2, 3, 4, 5]), 30);
+    await unshift(store, 'test2', 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 found2 = await shift(store);
+    expect(found2).toEqual('test2');
+    const found3 = await shift(store);
+    expect(found3).toEqual('test1');
+    const found4 = await shift(store);
+    expect(found4).toEqual(new Uint8Array([1, 2, 3, 4, 5]));
 
-    const found4 = await pop(store);
-    expect(found4).toBeUndefined();
+    const found5 = await shift(store);
+    expect(found5).toBeUndefined();
   });
 
   it('Queues and retries envelope if wrapped transport throws error', async () => {
@@ -104,7 +107,7 @@ describe('makeOfflineTransport', () => {
     const result2 = await transport.send(ERROR_ENVELOPE);
     expect(result2).toEqual({ statusCode: 200 });
 
-    await delay(MIN_DELAY * 2);
+    await delay(MIN_DELAY * 5);
 
     expect(queuedCount).toEqual(1);
     expect(getSendCount()).toEqual(2);

diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts
@@ -7,13 +7,10 @@ export const MIN_DELAY = 100; // 100 ms
 export const START_DELAY = 5_000; // 5 seconds
 const MAX_DELAY = 3.6e6; // 1 hour
 
-function log(msg: string, error?: Error): void {
-  DEBUG_BUILD && logger.info(`[Offline]: ${msg}`, error);
-}
-
 export interface OfflineStore {
-  insert(env: Envelope): Promise<void>;
-  pop(): Promise<Envelope | undefined>;
+  push(env: Envelope): Promise<void>;
+  unshift(env: Envelope): Promise<void>;
+  shift(): Promise<Envelope | undefined>;
 }
 
 export type CreateOfflineStore = (options: OfflineTransportOptions) => OfflineStore;
@@ -53,19 +50,25 @@ type Timer = number | { unref?: () => void };
 export function makeOfflineTransport<TO>(
   createTransport: (options: TO) => Transport,
 ): (options: TO & OfflineTransportOptions) => Transport {
+  function log(...args: unknown[]): void {
+    DEBUG_BUILD && logger.info('[Offline]:', ...args);
+  }
+
   return options => {
     const transport = createTransport(options);
-    const store = options.createStore ? options.createStore(options) : undefined;
+
+    if (!options.createStore) {
+      throw new Error('No `createStore` function was provided');
+    }
+
+    const store = options.createStore(options);
 
     let retryDelay = START_DELAY;
     let flushTimer: Timer | undefined;
 
     function shouldQueue(env: Envelope, error: Error, retryDelay: number): boolean | Promise<boolean> {
-      // 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.
-      // We also want to drop client reports because they can be generated when we retry sending events while offline.
-      if (envelopeContainsItemType(env, ['replay_event', 'replay_recording', 'client_report'])) {
+      // We want to drop client reports because they can be generated when we retry sending events while offline.
+      if (envelopeContainsItemType(env, ['client_report'])) {
         return false;
       }
 
@@ -77,21 +80,21 @@ export function makeOfflineTransport<TO>(
     }
 
     function flushIn(delay: number): void {
-      if (!store) {
-        return;
-      }
-
       if (flushTimer) {
         clearTimeout(flushTimer as ReturnType<typeof setTimeout>);
       }
 
       flushTimer = setTimeout(async () => {
         flushTimer = undefined;
 
-        const found = await store.pop();
+        const found = await store.shift();
         if (found) {
           log('Attempting to send previously queued event');
-          void send(found).catch(e => {
+
+          // We should to update the sent_at timestamp to the current time.
+          found[0].sent_at = new Date().toISOString();
+
+          void send(found, true).catch(e => {
             log('Failed to retry sending', e);
           });
         }
@@ -113,7 +116,15 @@ export function makeOfflineTransport<TO>(
       retryDelay = Math.min(retryDelay * 2, MAX_DELAY);
     }
 
-    async function send(envelope: Envelope): Promise<TransportMakeRequestResponse> {
+    async function send(envelope: Envelope, isRetry: boolean = false): Promise<TransportMakeRequestResponse> {
+      // We queue all replay envelopes to avoid multiple replay envelopes being sent at the same time. If one fails, we
+      // need to retry them in order.
+      if (!isRetry && envelopeContainsItemType(envelope, ['replay_event', 'replay_recording'])) {
+        await store.push(envelope);
+        flushIn(MIN_DELAY);
+        return {};
+      }
+
       try {
         const result = await transport.send(envelope);
 
@@ -123,6 +134,8 @@ export function makeOfflineTransport<TO>(
           // 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']);
+          } else if (result.headers && result.headers['x-sentry-rate-limits']) {
+            delay = 60_000; // 60 seconds
           } // If we have a server error, return now so we don't flush the queue.
           else if ((result.statusCode || 0) >= 400) {
             return result;
@@ -133,10 +146,15 @@ export function makeOfflineTransport<TO>(
         retryDelay = START_DELAY;
         return result;
       } catch (e) {
-        if (store && (await shouldQueue(envelope, e as Error, retryDelay))) {
-          await store.insert(envelope);
+        if (await shouldQueue(envelope, e as Error, retryDelay)) {
+          // 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);
+          } else {
+            await store.push(envelope);
+          }
           flushWithBackOff();
-          log('Error sending. Event queued', e as Error);
+          log('Error sending. Event queued.', e as Error);
           return {};
         } else {
           throw e;