feat(replay): Add onError callback + other small improvements to debugging (#13721)

* Adds an `onError` callback for replay SDK exceptions
* Do not log empty messages when calling `logger.exception`
* Send `ratelimit_backoff` client report when necessary (instead of
generic `send_error`)

Author: billyvg

packages/replay-internal/src/replay.ts

@@ -54,6 +54,7 @@ import { getHandleRecordingEmit } from './util/handleRecordingEmit';
 import { isExpired } from './util/isExpired';
 import { isSessionExpired } from './util/isSessionExpired';
 import { sendReplay } from './util/sendReplay';
+import { RateLimitError } from './util/sendReplayRequest';
 import type { SKIPPED } from './util/throttle';
 import { THROTTLED, throttle } from './util/throttle';
 
@@ -245,6 +246,9 @@ export class ReplayContainer implements ReplayContainerInterface {
   /** A wrapper to conditionally capture exceptions. */
   public handleException(error: unknown): void {
     DEBUG_BUILD && logger.exception(error);
+    if (this._options.onError) {
+      this._options.onError(error);
+    }
   }
 
   /**
@@ -1157,8 +1161,8 @@ export class ReplayContainer implements ReplayContainerInterface {
         segmentId,
         eventContext,
         session: this.session,
-        options: this.getOptions(),
         timestamp,
+        onError: err => this.handleException(err),
       });
     } catch (err) {
       this.handleException(err);
@@ -1173,7 +1177,8 @@ export class ReplayContainer implements ReplayContainerInterface {
       const client = getClient();
 
       if (client) {
-        client.recordDroppedEvent('send_error', 'replay');
+        const dropReason = err instanceof RateLimitError ? 'ratelimit_backoff' : 'send_error';
+        client.recordDroppedEvent(dropReason, 'replay');
       }
     }
   }

packages/replay-internal/src/types/replay.ts

@@ -26,7 +26,7 @@ export interface SendReplayData {
   eventContext: PopEventContext;
   timestamp: number;
   session: Session;
-  options: ReplayPluginOptions;
+  onError?: (err: unknown) => void;
 }
 
 export interface Timeouts {
@@ -222,6 +222,12 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions {
    */
   beforeErrorSampling?: (event: ErrorEvent) => boolean;
 
+  /**
+   * Callback when an internal SDK error occurs. This can be used to debug SDK
+   * issues.
+   */
+  onError?: (err: unknown) => void;
+
   /**
    * _experiments allows users to enable experimental or internal features.
    * We don't consider such features as part of the public API and hence we don't guarantee semver for them.

packages/replay-internal/src/util/logger.ts

@@ -1,6 +1,6 @@
 import { addBreadcrumb, captureException } from '@sentry/core';
 import type { ConsoleLevel, SeverityLevel } from '@sentry/types';
-import { logger as coreLogger } from '@sentry/utils';
+import { logger as coreLogger, severityLevelFromString } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
 
@@ -64,13 +64,13 @@ function makeReplayLogger(): ReplayLogger {
       _logger[name] = (...args: unknown[]) => {
         coreLogger[name](PREFIX, ...args);
         if (_trace) {
-          _addBreadcrumb(args[0]);
+          _addBreadcrumb(args.join(''), severityLevelFromString(name));
         }
       };
     });
 
     _logger.exception = (error: unknown, ...message: unknown[]) => {
-      if (_logger.error) {
+      if (message.length && _logger.error) {
         _logger.error(...message);
       }
 
@@ -79,9 +79,9 @@ function makeReplayLogger(): ReplayLogger {
       if (_capture) {
         captureException(error);
       } else if (_trace) {
-        // No need for a breadcrumb is `_capture` is enabled since it should be
+        // No need for a breadcrumb if `_capture` is enabled since it should be
         // captured as an exception
-        _addBreadcrumb(error);
+        _addBreadcrumb(error, 'error');
       }
     };
 

packages/replay-internal/src/util/sendReplay.ts

@@ -1,8 +1,7 @@
 import { setTimeout } from '@sentry-internal/browser-utils';
-import { captureException, setContext } from '@sentry/core';
+import { setContext } from '@sentry/core';
 
 import { RETRY_BASE_INTERVAL, RETRY_MAX_COUNT, UNABLE_TO_SEND_REPLAY } from '../constants';
-import { DEBUG_BUILD } from '../debug-build';
 import type { SendReplayData } from '../types';
 import { RateLimitError, TransportStatusCodeError, sendReplayRequest } from './sendReplayRequest';
 
@@ -16,7 +15,7 @@ export async function sendReplay(
     interval: RETRY_BASE_INTERVAL,
   },
 ): Promise<unknown> {
-  const { recordingData, options } = replayData;
+  const { recordingData, onError } = replayData;
 
   // short circuit if there's no events to upload (this shouldn't happen as _runFlush makes this check)
   if (!recordingData.length) {
@@ -36,8 +35,8 @@ export async function sendReplay(
       _retryCount: retryConfig.count,
     });
 
-    if (DEBUG_BUILD && options._experiments && options._experiments.captureExceptions) {
-      captureException(err);
+    if (onError) {
+      onError(err);
     }
 
     // If an error happened here, it's likely that uploading the attachment

packages/replay-internal/test/integration/flush.test.ts

@@ -188,8 +188,8 @@ describe('Integration | flush', () => {
       segmentId: 0,
       eventContext: expect.anything(),
       session: expect.any(Object),
-      options: expect.any(Object),
       timestamp: expect.any(Number),
+      onError: expect.any(Function),
     });
 
     // Add this to test that segment ID increases
@@ -238,7 +238,7 @@ describe('Integration | flush', () => {
       segmentId: 1,
       eventContext: expect.anything(),
       session: expect.any(Object),
-      options: expect.any(Object),
+      onError: expect.any(Function),
       timestamp: expect.any(Number),
     });
 

packages/replay-internal/test/unit/util/logger.test.ts

@@ -0,0 +1,88 @@
+import { beforeEach, describe, expect, it } from 'vitest';
+
+import * as SentryCore from '@sentry/core';
+import { logger as coreLogger } from '@sentry/utils';
+import { logger } from '../../../src/util/logger';
+
+const mockCaptureException = vi.spyOn(SentryCore, 'captureException');
+const mockAddBreadcrumb = vi.spyOn(SentryCore, 'addBreadcrumb');
+const mockLogError = vi.spyOn(coreLogger, 'error');
+vi.spyOn(coreLogger, 'info');
+vi.spyOn(coreLogger, 'log');
+vi.spyOn(coreLogger, 'warn');
+
+describe('logger', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  describe.each([
+    [false, false],
+    [false, true],
+    [true, false],
+    [true, true],
+  ])('with options: captureExceptions:%s, traceInternals:%s', (captureExceptions, traceInternals) => {
+    beforeEach(() => {
+      logger.setConfig({
+        captureExceptions,
+        traceInternals,
+      });
+    });
+
+    it.each([
+      ['info', 'info', 'info message'],
+      ['log', 'log', 'log message'],
+      ['warn', 'warning', 'warn message'],
+      ['error', 'error', 'error message'],
+    ])('%s', (fn, level, message) => {
+      logger[fn](message);
+      expect(coreLogger[fn]).toHaveBeenCalledWith('[Replay] ', message);
+
+      if (traceInternals) {
+        expect(mockAddBreadcrumb).toHaveBeenLastCalledWith(
+          {
+            category: 'console',
+            data: { logger: 'replay' },
+            level,
+            message: `[Replay] ${message}`,
+          },
+          { level },
+        );
+      }
+    });
+
+    it('logs exceptions with a message', () => {
+      const err = new Error('An error');
+      logger.exception(err, 'a message');
+      if (captureExceptions) {
+        expect(mockCaptureException).toHaveBeenCalledWith(err);
+      }
+      expect(mockLogError).toHaveBeenCalledWith('[Replay] ', 'a message');
+      expect(mockLogError).toHaveBeenLastCalledWith('[Replay] ', err);
+      expect(mockLogError).toHaveBeenCalledTimes(2);
+
+      if (traceInternals) {
+        expect(mockAddBreadcrumb).toHaveBeenCalledWith(
+          {
+            category: 'console',
+            data: { logger: 'replay' },
+            level: 'error',
+            message: '[Replay] a message',
+          },
+          { level: 'error' },
+        );
+      }
+    });
+
+    it('logs exceptions without a message', () => {
+      const err = new Error('An error');
+      logger.exception(err);
+      if (captureExceptions) {
+        expect(mockCaptureException).toHaveBeenCalledWith(err);
+        expect(mockAddBreadcrumb).not.toHaveBeenCalled();
+      }
+      expect(mockLogError).toHaveBeenCalledTimes(1);
+      expect(mockLogError).toHaveBeenLastCalledWith('[Replay] ', err);
+    });
+  });
+});