ref(core): Make on and emit required on client (#10603)

This allows us to remove a bunch of code around fallbacks etc. Esp. in
replay this also allows us to remove a bunch of code around breadcrumbs
etc.

note @JonasBa I did not remove this in profiling-node yet because the
test rely on this quite a bunch, can you take a look at this when you've
got some time?

Author: mydea

packages/angular/src/errorhandler.ts

@@ -118,7 +118,7 @@ class SentryErrorHandler implements AngularErrorHandler {
     if (this._options.showDialog) {
       const client = Sentry.getClient();
 
-      if (client && client.on && !this._registeredAfterSendEventHandler) {
+      if (client && !this._registeredAfterSendEventHandler) {
         client.on('afterSendEvent', (event: Event) => {
           if (!event.type) {
             // eslint-disable-next-line deprecation/deprecation
@@ -128,7 +128,7 @@ class SentryErrorHandler implements AngularErrorHandler {
 
         // We only want to register this hook once in the lifetime of the error handler
         this._registeredAfterSendEventHandler = true;
-      } else if (!client || !client.on) {
+      } else if (!client) {
         Sentry.showReportDialog({ ...this._options.dialogOptions, eventId });
       }
     }

packages/browser/src/integrations/breadcrumbs.ts

@@ -88,7 +88,7 @@ const _breadcrumbsIntegration = ((options: Partial<BreadcrumbsOptions> = {}) =>
       if (_options.history) {
         addHistoryInstrumentationHandler(_getHistoryBreadcrumbHandler(client));
       }
-      if (_options.sentry && client.on) {
+      if (_options.sentry) {
         client.on('beforeSendEvent', _getSentryBreadcrumbHandler(client));
       }
     },

packages/browser/src/profiling/integration.ts

@@ -35,11 +35,6 @@ const _browserProfilingIntegration = (() => {
         }
       }
 
-      if (typeof client.on !== 'function') {
-        logger.warn('[Profiling] Client does not support hooks, profiling will be disabled');
-        return;
-      }
-
       client.on('startTransaction', (transaction: Transaction) => {
         if (shouldProfileTransaction(transaction)) {
           startProfileForTransaction(transaction);

packages/core/src/hub.ts

@@ -419,9 +419,7 @@ export class Hub implements HubInterface {
 
     if (finalBreadcrumb === null) return;
 
-    if (client.emit) {
-      client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);
-    }
+    client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);
 
     // TODO(v8): I know this comment doesn't make much sense because the hub will be deprecated but I still wanted to
     // write it down. In theory, we would have to add the breadcrumbs to the isolation scope here, however, that would

packages/core/src/integration.ts

@@ -140,7 +140,7 @@ export function setupIntegration(client: Client, integration: Integration, integ
     integration.setup(client);
   }
 
-  if (client.on && typeof integration.preprocessEvent === 'function') {
+  if (typeof integration.preprocessEvent === 'function') {
     const callback = integration.preprocessEvent.bind(integration) as typeof integration.preprocessEvent;
     client.on('preprocessEvent', (event, hint) => callback(event, hint, client));
   }

packages/core/src/integrations/metadata.ts

@@ -12,10 +12,6 @@ const _moduleMetadataIntegration = (() => {
     // TODO v8: Remove this
     setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
-      if (typeof client.on !== 'function') {
-        return;
-      }
-
       // We need to strip metadata from stack frames before sending them to Sentry since these are client side only.
       client.on('beforeEnvelope', envelope => {
         forEachEnvelopeItem(envelope, (item, type) => {

packages/core/src/tracing/dynamicSamplingContext.ts

@@ -23,7 +23,7 @@ export function getDynamicSamplingContextFromClient(trace_id: string, client: Cl
     trace_id,
   }) as DynamicSamplingContext;
 
-  client.emit && client.emit('createDsc', dsc);
+  client.emit('createDsc', dsc);
 
   return dsc;
 }
@@ -81,7 +81,7 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
 
   dsc.sampled = String(spanIsSampled(txn));
 
-  client.emit && client.emit('createDsc', dsc);
+  client.emit('createDsc', dsc);
 
   return dsc;
 }

packages/core/src/tracing/hubextensions.ts

@@ -78,7 +78,7 @@ The transaction will not be sampled. Please use the ${configInstrumenter} instru
   if (transaction.isRecording()) {
     transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
   }
-  if (client && client.emit) {
+  if (client) {
     client.emit('startTransaction', transaction);
   }
   return transaction;
@@ -125,7 +125,7 @@ export function startIdleTransaction(
   if (transaction.isRecording()) {
     transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
   }
-  if (client && client.emit) {
+  if (client) {
     client.emit('startTransaction', transaction);
   }
   return transaction;

packages/core/src/tracing/transaction.ts

@@ -276,7 +276,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
 
     // eslint-disable-next-line deprecation/deprecation
     const client = this._hub.getClient();
-    if (client && client.emit) {
+    if (client) {
       client.emit('finishTransaction', this);
     }
 

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

@@ -1957,11 +1957,11 @@ describe('BaseClient', () => {
           traceId: '86f39e84263a4de99c326acab3bfe3bd',
         } as Transaction;
 
-        client.on?.('startTransaction', transaction => {
+        client.on('startTransaction', transaction => {
           expect(transaction).toEqual(mockTransaction);
         });
 
-        client.emit?.('startTransaction', mockTransaction);
+        client.emit('startTransaction', mockTransaction);
       });
 
       it('should call a beforeEnvelope hook', () => {
@@ -1974,11 +1974,11 @@ describe('BaseClient', () => {
           {},
         ] as Envelope;
 
-        client.on?.('beforeEnvelope', envelope => {
+        client.on('beforeEnvelope', envelope => {
           expect(envelope).toEqual(mockEnvelope);
         });
 
-        client.emit?.('beforeEnvelope', mockEnvelope);
+        client.emit('beforeEnvelope', mockEnvelope);
       });
     });
   });

packages/feedback/src/util/prepareFeedbackEvent.ts

@@ -17,9 +17,7 @@ export async function prepareFeedbackEvent({
   event,
 }: PrepareFeedbackEventParams): Promise<FeedbackEvent | null> {
   const eventHint = {};
-  if (client.emit) {
-    client.emit('preprocessEvent', event, eventHint);
-  }
+  client.emit('preprocessEvent', event, eventHint);
 
   const preparedEvent = (await prepareEvent(
     client.getOptions(),

packages/feedback/src/util/sendFeedbackRequest.ts

@@ -51,9 +51,7 @@ export async function sendFeedbackRequest(
       return;
     }
 
-    if (client.emit) {
-      client.emit('beforeSendFeedback', feedbackEvent, { includeReplay: Boolean(includeReplay) });
-    }
+    client.emit('beforeSendFeedback', feedbackEvent, { includeReplay: Boolean(includeReplay) });
 
     const envelope = createEventEnvelope(feedbackEvent, dsn, client.getOptions()._metadata, client.getOptions().tunnel);
 

packages/integrations/src/debug.ts

@@ -23,10 +23,6 @@ const _debugIntegration = ((options: DebugOptions = {}) => {
     // TODO v8: Remove this
     setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
-      if (!client.on) {
-        return;
-      }
-
       client.on('beforeSendEvent', (event: Event, hint?: EventHint) => {
         if (_options.debugger) {
           // eslint-disable-next-line no-debugger

packages/node-experimental/src/sdk/api.ts

@@ -134,9 +134,7 @@ export function addBreadcrumb(breadcrumb: Breadcrumb, hint?: BreadcrumbHint): vo
 
   if (finalBreadcrumb === null) return;
 
-  if (client.emit) {
-    client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);
-  }
+  client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);
 
   getIsolationScope().addBreadcrumb(finalBreadcrumb, maxBreadcrumbs);
 }

packages/node/src/integrations/spotlight.ts

@@ -65,11 +65,6 @@ function connectToSpotlight(client: Client, options: Required<SpotlightConnectio
 
   let failedRequests = 0;
 
-  if (typeof client.on !== 'function') {
-    logger.warn('[Spotlight] Cannot connect to spotlight due to missing method on SDK client (`client.on`)');
-    return;
-  }
-
   client.on('beforeEnvelope', (envelope: Envelope) => {
     if (failedRequests > 3) {
       logger.warn('[Spotlight] Disabled Sentry -> Spotlight integration due to too many failed requests');

packages/node/test/integrations/spotlight.test.ts

@@ -120,16 +120,6 @@ describe('Spotlight', () => {
       integration.setup!(client);
       expect(loggerSpy).toHaveBeenCalledWith(expect.stringContaining('Invalid sidecar URL: invalid-url'));
     });
-
-    it("the client doesn't support life cycle hooks", () => {
-      const integration = spotlightIntegration({ sidecarUrl: 'http://mylocalhost:8969' });
-      const clientWithoutHooks = { ...client };
-      // @ts-expect-error - this is fine in tests
-      delete client.on;
-      // @ts-expect-error - this is fine in tests
-      integration.setup(clientWithoutHooks);
-      expect(loggerSpy).toHaveBeenCalledWith(expect.stringContaining(' missing method on SDK client (`client.on`)'));
-    });
   });
 
   it('warns if the NODE_ENV variable doesn\'t equal "development"', () => {

packages/opentelemetry-node/src/spanprocessor.ts

@@ -114,7 +114,7 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
     const client = getClient();
 
     const mutableOptions = { drop: false };
-    client && client.emit && client?.emit('otelSpanEnd', otelSpan, mutableOptions);
+    client && client.emit('otelSpanEnd', otelSpan, mutableOptions);
 
     if (mutableOptions.drop) {
       clearSpan(otelSpanId);

packages/opentelemetry-node/test/propagator.test.ts

@@ -45,6 +45,7 @@ describe('SentryPropagator', () => {
         getDsn: () => ({
           publicKey: 'abc',
         }),
+        emit: () => {},
       };
       // @ts-expect-error Use mock client for unit tests
       // eslint-disable-next-line deprecation/deprecation

packages/opentelemetry/src/custom/transaction.ts

@@ -17,7 +17,7 @@ export function startTransaction(hub: HubInterface, transactionContext: Transact
   // Any sampling decision happens in OpenTelemetry's sampler
   transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
 
-  if (client && client.emit) {
+  if (client) {
     client.emit('startTransaction', transaction);
   }
   return transaction;

packages/opentelemetry/test/propagator.test.ts

@@ -39,6 +39,7 @@ describe('SentryPropagator', () => {
       getDsn: () => ({
         publicKey: 'abc',
       }),
+      emit: () => {},
     };
 
     // @ts-expect-error Use mock client for unit tests

packages/react/src/errorboundary.tsx

@@ -108,7 +108,7 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
     this._openFallbackReportDialog = true;
 
     const client = getClient();
-    if (client && client.on && props.showDialog) {
+    if (client && props.showDialog) {
       this._openFallbackReportDialog = false;
       client.on('afterSendEvent', event => {
         if (!event.type && this._lastEventId && event.event_id === this._lastEventId) {

packages/replay/src/coreHandlers/handleBreadcrumbs.ts

@@ -16,7 +16,7 @@ type BreadcrumbWithCategory = Required<Pick<Breadcrumb, 'category'>>;
 export function handleBreadcrumbs(replay: ReplayContainer): void {
   const client = getClient();
 
-  if (!client || !client.on) {
+  if (!client) {
     return;
   }
 

packages/replay/src/coreHandlers/handleFetch.ts

@@ -5,19 +5,13 @@ import { DEBUG_BUILD } from '../debug-build';
 import type { ReplayContainer } from '../types';
 import { isErrorEvent, isFeedbackEvent, isReplayEvent, isTransactionEvent } from '../util/eventUtils';
 import { isRrwebError } from '../util/isRrwebError';
-import { handleAfterSendEvent } from './handleAfterSendEvent';
 import { addFeedbackBreadcrumb } from './util/addFeedbackBreadcrumb';
 import { shouldSampleForBufferEvent } from './util/shouldSampleForBufferEvent';
 
 /**
  * Returns a listener to be added to `addEventProcessor(listener)`.
  */
-export function handleGlobalEventListener(
-  replay: ReplayContainer,
-  includeAfterSendEventHandling = false,
-): (event: Event, hint: EventHint) => Event | null {
-  const afterSendHandler = includeAfterSendEventHandling ? handleAfterSendEvent(replay) : undefined;
-
+export function handleGlobalEventListener(replay: ReplayContainer): (event: Event, hint: EventHint) => Event | null {
   return Object.assign(
     (event: Event, hint: EventHint) => {
       // Do nothing if replay has been disabled
@@ -73,13 +67,6 @@ export function handleGlobalEventListener(
         event.tags = { ...event.tags, replayId: replay.getSessionId() };
       }
 
-      // In cases where a custom client is used that does not support the new hooks (yet),
-      // we manually call this hook method here
-      if (afterSendHandler) {
-        // Pretend the error had a 200 response so we always capture it
-        afterSendHandler(event, { statusCode: 200 });
-      }
-
       return event;
     },
     { id: 'Replay' },

packages/replay/src/coreHandlers/handleGlobalEvent.ts

@@ -6,12 +6,10 @@ import type {
   TextEncoderInternal,
   XhrBreadcrumbData,
 } from '@sentry/types';
-import { addFetchInstrumentationHandler, addXhrInstrumentationHandler, logger } from '@sentry/utils';
+import { logger } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
 import type { FetchHint, ReplayContainer, ReplayNetworkOptions, XhrHint } from '../types';
-import { handleFetchSpanListener } from './handleFetch';
-import { handleXhrSpanListener } from './handleXhr';
 import { captureFetchBreadcrumbToReplay, enrichFetchBreadcrumb } from './util/fetchUtils';
 import { captureXhrBreadcrumbToReplay, enrichXhrBreadcrumb } from './util/xhrUtils';
 
@@ -50,12 +48,8 @@ export function handleNetworkBreadcrumbs(replay: ReplayContainer): void {
       networkResponseHeaders,
     };
 
-    if (client && client.on) {
+    if (client) {
       client.on('beforeAddBreadcrumb', (breadcrumb, hint) => beforeAddNetworkBreadcrumb(options, breadcrumb, hint));
-    } else {
-      // Fallback behavior
-      addFetchInstrumentationHandler(handleFetchSpanListener(replay));
-      addXhrInstrumentationHandler(handleXhrSpanListener(replay));
     }
   } catch {
     // Do nothing