ref: Make hook afterSendEvent

Author: mydea

packages/core/src/baseclient.ts

@@ -322,8 +322,8 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
       }
 
       const promise = this._sendEnvelope(env);
-      if (promise && isErrorEvent(event)) {
-        promise.then(sendResponse => this.emit('afterSendErrorEvent', event, sendResponse), null);
+      if (promise) {
+        promise.then(sendResponse => this.emit('afterSendEvent', event, sendResponse), null);
       }
     }
   }
@@ -369,8 +369,8 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
 
   /** @inheritdoc */
   public on(
-    hook: 'afterSendErrorEvent',
-    callback: (event: ErrorEvent, sendResponse: TransportMakeRequestResponse | void) => void,
+    hook: 'afterSendEvent',
+    callback: (event: Event, sendResponse: TransportMakeRequestResponse | void) => void,
   ): void;
 
   /** @inheritdoc */
@@ -390,7 +390,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   public emit(hook: 'beforeEnvelope', envelope: Envelope): void;
 
   /** @inheritdoc */
-  public emit(hook: 'afterSendErrorEvent', event: ErrorEvent, sendResponse: TransportMakeRequestResponse | void): void;
+  public emit(hook: 'afterSendEvent', event: Event, sendResponse: TransportMakeRequestResponse | void): void;
 
   /** @inheritdoc */
   public emit(hook: string, ...rest: unknown[]): void {

packages/core/test/lib/base.test.ts

@@ -1643,7 +1643,7 @@ describe('BaseClient', () => {
       jest.useRealTimers();
     });
 
-    it('emits `afterSendErrorEvent` when sending an error', async () => {
+    it('emits `afterSendEvent` when sending an error', async () => {
       const client = new TestClient(
         getDefaultTestClientOptions({
           dsn: PUBLIC_DSN,
@@ -1657,7 +1657,7 @@ describe('BaseClient', () => {
       const errorEvent: Event = { message: 'error' };
 
       const callback = jest.fn();
-      client.on('afterSendErrorEvent', callback);
+      client.on('afterSendEvent', callback);
 
       client.sendEvent(errorEvent);
       jest.runAllTimers();
@@ -1671,7 +1671,35 @@ describe('BaseClient', () => {
       expect(callback).toBeCalledWith(errorEvent, {});
     });
 
-    it('still triggers `afterSendErrorEvent` when transport.send rejects', async () => {
+    it('emits `afterSendEvent` when sending a transaction', async () => {
+      const client = new TestClient(
+        getDefaultTestClientOptions({
+          dsn: PUBLIC_DSN,
+          enableSend: true,
+        }),
+      );
+
+      // @ts-ignore
+      const mockSend = jest.spyOn(client._transport, 'send');
+
+      const transactionEvent: Event = { type: 'transaction', event_id: 'tr1' };
+
+      const callback = jest.fn();
+      client.on('afterSendEvent', callback);
+
+      client.sendEvent(transactionEvent);
+      jest.runAllTimers();
+      // Wait for two ticks
+      // note that for whatever reason, await new Promise(resolve => setTimeout(resolve, 0)) causes the test to hang
+      await undefined;
+      await undefined;
+
+      expect(mockSend).toBeCalledTimes(1);
+      expect(callback).toBeCalledTimes(1);
+      expect(callback).toBeCalledWith(transactionEvent, {});
+    });
+
+    it('still triggers `afterSendEvent` when transport.send rejects', async () => {
       expect.assertions(3);
 
       const client = new TestClient(
@@ -1689,7 +1717,7 @@ describe('BaseClient', () => {
       const errorEvent: Event = { message: 'error' };
 
       const callback = jest.fn();
-      client.on('afterSendErrorEvent', callback);
+      client.on('afterSendEvent', callback);
 
       client.sendEvent(errorEvent);
       jest.runAllTimers();
@@ -1721,7 +1749,7 @@ describe('BaseClient', () => {
       const errorEvent: Event = { message: 'error' };
 
       const callback = jest.fn();
-      client.on('afterSendErrorEvent', callback);
+      client.on('afterSendEvent', callback);
 
       client.sendEvent(errorEvent);
       jest.runAllTimers();

packages/replay/src/coreHandlers/handleAfterSendError.ts renamed to packages/replay/src/coreHandlers/handleAfterSendEvent.ts

@@ -1,28 +1,45 @@
 import { getCurrentHub } from '@sentry/core';
-import type { ErrorEvent, Transport, TransportMakeRequestResponse } from '@sentry/types';
+import type { Event, Transport, TransportMakeRequestResponse } from '@sentry/types';
 
 import { UNABLE_TO_SEND_REPLAY } from '../constants';
 import type { ReplayContainer } from '../types';
+import { isErrorEvent, isTransactionEvent } from '../util/eventUtils';
 
 /**
  * Returns a listener to be added to `client.on('afterSendErrorEvent, listener)`.
  */
-export function handleAfterSendError(
+export function handleAfterSendEvent(
   replay: ReplayContainer,
-): (event: ErrorEvent, sendResponse: TransportMakeRequestResponse | void) => void {
+): (event: Event, sendResponse: TransportMakeRequestResponse | void) => void {
   // Custom transports may still be returning `Promise<void>`, which means we cannot expect the status code to be available there
   const enforceStatusCode = isBaseTransportSend();
 
-  return (event: ErrorEvent, sendResponse: TransportMakeRequestResponse | void) => {
+  return (event: Event, sendResponse: TransportMakeRequestResponse | void) => {
+    if (!isErrorEvent(event) && !isTransactionEvent(event)) {
+      return;
+    }
+
     const statusCode = sendResponse && sendResponse.statusCode;
 
-    // We only want to do stuff on successfull error sending, otherwise you get error replays without errors attached
+    // We only want to do stuff on successful error sending, otherwise you get error replays without errors attached
     // If not using the base transport, we allow `undefined` response (as a custom transport may not implement this correctly yet)
     // If we do use the base transport, we skip if we encountered an non-OK status code
     if (enforceStatusCode && (!statusCode || statusCode < 200 || statusCode >= 300)) {
       return;
     }
 
+    // Collect traceIds in _context regardless of `recordingMode`
+    // In error mode, _context gets cleared on every checkout
+    if (isTransactionEvent(event) && event.contexts && event.contexts.trace && event.contexts.trace.trace_id) {
+      replay.getContext().traceIds.add(event.contexts.trace.trace_id as string);
+      return;
+    }
+
+    // Everything below is just for error events
+    if (!isErrorEvent(event)) {
+      return;
+    }
+
     // Add error to list of errorIds of replay
     if (event.event_id) {
       replay.getContext().errorIds.add(event.event_id);

packages/replay/src/coreHandlers/handleGlobalEvent.ts

@@ -1,30 +1,34 @@
 import { addBreadcrumb } from '@sentry/core';
-import type { ErrorEvent, Event, EventHint } from '@sentry/types';
+import type { Event, EventHint } from '@sentry/types';
 import { logger } from '@sentry/utils';
 
-import { REPLAY_EVENT_NAME } from '../constants';
 import type { ReplayContainer } from '../types';
+import { isErrorEvent, isReplayEvent, isTransactionEvent } from '../util/eventUtils';
 import { isRrwebError } from '../util/isRrwebError';
-import { handleAfterSendError } from './handleAfterSendError';
+import { handleAfterSendEvent } from './handleAfterSendEvent';
 
 /**
  * Returns a listener to be added to `addGlobalEventProcessor(listener)`.
  */
 export function handleGlobalEventListener(
   replay: ReplayContainer,
-  includeErrorHandling = false,
+  includeAfterSendEventHandling = false,
 ): (event: Event, hint: EventHint) => Event | null {
-  const errorHandler = includeErrorHandling ? handleAfterSendError(replay) : undefined;
+  const afterSendHandler = includeAfterSendEventHandling ? handleAfterSendEvent(replay) : undefined;
 
   return (event: Event, hint: EventHint) => {
-    // Do not apply replayId to the root event
-    if (event.type === REPLAY_EVENT_NAME) {
+    if (isReplayEvent(event)) {
       // Replays have separate set of breadcrumbs, do not include breadcrumbs
       // from core SDK
       delete event.breadcrumbs;
       return event;
     }
 
+    // We only want to handle errors & transactions, nothing else
+    if (!isErrorEvent(event) && !isTransactionEvent(event)) {
+      return event;
+    }
+
     // Unless `captureExceptions` is enabled, we want to ignore errors coming from rrweb
     // As there can be a bunch of stuff going wrong in internals there, that we don't want to bubble up to users
     if (isRrwebError(event, hint) && !replay.getOptions()._experiments.captureExceptions) {
@@ -33,28 +37,20 @@ export function handleGlobalEventListener(
     }
 
     // Only tag transactions with replayId if not waiting for an error
-    // @ts-ignore private
-    if (!event.type || replay.recordingMode === 'session') {
+    if (isErrorEvent(event) || (isTransactionEvent(event) && replay.recordingMode === 'session')) {
       event.tags = { ...event.tags, replayId: replay.getSessionId() };
     }
 
-    // Collect traceIds in _context regardless of `recordingMode` - if it's true,
-    // _context gets cleared on every checkout
-    if (event.type === 'transaction' && event.contexts && event.contexts.trace && event.contexts.trace.trace_id) {
-      replay.getContext().traceIds.add(event.contexts.trace.trace_id as string);
-      return event;
-    }
-
-    if (__DEBUG_BUILD__ && replay.getOptions()._experiments.traceInternals) {
+    if (__DEBUG_BUILD__ && replay.getOptions()._experiments.traceInternals && isErrorEvent(event)) {
       const exc = getEventExceptionValues(event);
       addInternalBreadcrumb({
         message: `Tagging event (${event.event_id}) - ${event.message} - ${exc.type}: ${exc.value}`,
       });
     }
 
-    if (errorHandler && !event.type) {
+    if (afterSendHandler) {
       // Pretend the error had a 200 response so we capture it
-      errorHandler(event as ErrorEvent, { statusCode: 200 });
+      afterSendHandler(event, { statusCode: 200 });
     }
 
     return event;

packages/replay/src/util/addGlobalListeners.ts

@@ -2,7 +2,7 @@ import type { BaseClient } from '@sentry/core';
 import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core';
 import { addInstrumentationHandler } from '@sentry/utils';
 
-import { handleAfterSendError } from '../coreHandlers/handleAfterSendError';
+import { handleAfterSendEvent } from '../coreHandlers/handleAfterSendEvent';
 import { handleDomListener } from '../coreHandlers/handleDom';
 import { handleFetchSpanListener } from '../coreHandlers/handleFetch';
 import { handleGlobalEventListener } from '../coreHandlers/handleGlobalEvent';
@@ -36,6 +36,6 @@ export function addGlobalListeners(replay: ReplayContainer): void {
 
   if (hasHooks) {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    (client as BaseClient<any>).on('afterSendErrorEvent', handleAfterSendError(replay));
+    (client as BaseClient<any>).on('afterSendEvent', handleAfterSendEvent(replay));
   }
 }

packages/replay/src/util/eventUtils.ts

@@ -0,0 +1,16 @@
+import type { ErrorEvent, Event, ReplayEvent, TransactionEvent } from '@sentry/types';
+
+/** If the event is an error event */
+export function isErrorEvent(event: Event): event is ErrorEvent {
+  return !event.type;
+}
+
+/** If the event is a transaction event */
+export function isTransactionEvent(event: Event): event is TransactionEvent {
+  return event.type === 'transaction';
+}
+
+/** If the event is an replay event */
+export function isReplayEvent(event: Event): event is ReplayEvent {
+  return event.type === 'replay_event';
+}

packages/replay/test/fixtures/transaction.ts

@@ -1,6 +1,6 @@
 import type { Event, SeverityLevel } from '@sentry/types';
 
-export function Transaction(obj?: Partial<Event>): any {
+export function Transaction(traceId?: string, obj?: Partial<Event>): any {
   const timestamp = new Date().getTime() / 1000;
 
   return {
@@ -22,7 +22,7 @@ export function Transaction(obj?: Partial<Event>): any {
           hardwareConcurrency: '10',
           sentry_reportAllChanges: false,
         },
-        trace_id: 'trace_id',
+        trace_id: traceId || 'trace_id',
       },
     }, // }}}
     spans: [