ref(core): Avoid using SentryError for event processing control flow (#15823)

mydea · web-flow · commit c14ab92c54fa · 2025-03-25T16:56:57.000+01:00
This PR replaces our usage of `SentryError` for control flow in our event processing. Instead, we now throw either an `InternalError` or `DoNotSendEventError` (which are just POJOs with a symbol, without a stackframe). These two also allow us to differentiate between these two use cases, which so far have been kind of combined via the log level, but are really different things - one is "expected"/configured by a user, the other is unexpected and more of a warning. I also removed the handling for `SentryError` from inbound filters, as this should then become unused. This also deprecates `SentryError`, it is no longer used and we can eventually remove it. ref #15725 (comment)
diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts
@@ -53,7 +53,6 @@ import {
 import { createClientReportEnvelope } from './utils-hoist/clientreport';
 import { dsnToString, makeDsn } from './utils-hoist/dsn';
 import { addItemToEnvelope, createAttachmentEnvelopeItem } from './utils-hoist/envelope';
-import { SentryError } from './utils-hoist/error';
 import { isParameterizedString, isPlainObject, isPrimitive, isThenable } from './utils-hoist/is';
 import { logger } from './utils-hoist/logger';
 import { checkOrSetAlreadyCaught, uuid4 } from './utils-hoist/misc';
@@ -69,6 +68,41 @@ import { _getSpanForScope } from './utils/spanOnScope';
 const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured.";
 const MISSING_RELEASE_FOR_SESSION_ERROR = 'Discarded session because of missing or non-string release';
 
+const INTERNAL_ERROR_SYMBOL = Symbol.for('SentryInternalError');
+const DO_NOT_SEND_EVENT_SYMBOL = Symbol.for('SentryDoNotSendEventError');
+
+interface InternalError {
+  message: string;
+  [INTERNAL_ERROR_SYMBOL]: true;
+}
+
+interface DoNotSendEventError {
+  message: string;
+  [DO_NOT_SEND_EVENT_SYMBOL]: true;
+}
+
+function _makeInternalError(message: string): InternalError {
+  return {
+    message,
+    [INTERNAL_ERROR_SYMBOL]: true,
+  };
+}
+
+function _makeDoNotSendEventError(message: string): DoNotSendEventError {
+  return {
+    message,
+    [DO_NOT_SEND_EVENT_SYMBOL]: true,
+  };
+}
+
+function _isInternalError(error: unknown): error is InternalError {
+  return !!error && typeof error === 'object' && INTERNAL_ERROR_SYMBOL in error;
+}
+
+function _isDoNotSendEventError(error: unknown): error is DoNotSendEventError {
+  return !!error && typeof error === 'object' && DO_NOT_SEND_EVENT_SYMBOL in error;
+}
+
 /**
  * Base implementation for all JavaScript SDK clients.
  *
@@ -975,10 +1009,10 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
       },
       reason => {
         if (DEBUG_BUILD) {
-          // If something's gone wrong, log the error as a warning. If it's just us having used a `SentryError` for
-          // control flow, log just the message (no stack) as a log-level log.
-          if (reason instanceof SentryError && reason.logLevel === 'log') {
+          if (_isDoNotSendEventError(reason)) {
             logger.log(reason.message);
+          } else if (_isInternalError(reason)) {
+            logger.warn(reason.message);
           } else {
             logger.warn(reason);
           }
@@ -1022,9 +1056,8 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
     if (isError && typeof parsedSampleRate === 'number' && Math.random() > parsedSampleRate) {
       this.recordDroppedEvent('sample_rate', 'error');
       return rejectedSyncPromise(
-        new SentryError(
+        _makeDoNotSendEventError(
           `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`,
-          'log',
         ),
       );
     }
@@ -1035,7 +1068,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
       .then(prepared => {
         if (prepared === null) {
           this.recordDroppedEvent('event_processor', dataCategory);
-          throw new SentryError('An event processor returned `null`, will not send event.', 'log');
+          throw _makeDoNotSendEventError('An event processor returned `null`, will not send event.');
         }
 
         const isInternalException = hint.data && (hint.data as { __sentry__: boolean }).__sentry__ === true;
@@ -1055,7 +1088,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
             const spanCount = 1 + spans.length;
             this.recordDroppedEvent('before_send', 'span', spanCount);
           }
-          throw new SentryError(`${beforeSendLabel} returned \`null\`, will not send event.`, 'log');
+          throw _makeDoNotSendEventError(`${beforeSendLabel} returned \`null\`, will not send event.`);
         }
 
         const session = currentScope.getSession() || isolationScope.getSession();
@@ -1089,7 +1122,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
         return processedEvent;
       })
       .then(null, reason => {
-        if (reason instanceof SentryError) {
+        if (_isDoNotSendEventError(reason) || _isInternalError(reason)) {
           throw reason;
         }
 
@@ -1099,7 +1132,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
           },
           originalException: reason,
         });
-        throw new SentryError(
+        throw _makeInternalError(
           `Event processing pipeline threw an error, original event will not be sent. Details have been sent as a new event.\nReason: ${reason}`,
         );
       });
@@ -1205,16 +1238,16 @@ function _validateBeforeSendResult(
     return beforeSendResult.then(
       event => {
         if (!isPlainObject(event) && event !== null) {
-          throw new SentryError(invalidValueError);
+          throw _makeInternalError(invalidValueError);
         }
         return event;
       },
       e => {
-        throw new SentryError(`${beforeSendLabel} rejected with ${e}`);
+        throw _makeInternalError(`${beforeSendLabel} rejected with ${e}`);
       },
     );
   } else if (!isPlainObject(beforeSendResult) && beforeSendResult !== null) {
-    throw new SentryError(invalidValueError);
+    throw _makeInternalError(invalidValueError);
   }
   return beforeSendResult;
 }
diff --git a/packages/core/src/integrations/eventFilters.ts b/packages/core/src/integrations/eventFilters.ts
@@ -102,19 +102,12 @@ function _mergeOptions(
       ...(internalOptions.disableErrorDefaults ? [] : DEFAULT_IGNORE_ERRORS),
     ],
     ignoreTransactions: [...(internalOptions.ignoreTransactions || []), ...(clientOptions.ignoreTransactions || [])],
-    ignoreInternal: internalOptions.ignoreInternal !== undefined ? internalOptions.ignoreInternal : true,
   };
 }
 
 function _shouldDropEvent(event: Event, options: Partial<EventFiltersOptions>): boolean {
   if (!event.type) {
     // Filter errors
-
-    if (options.ignoreInternal && _isSentryError(event)) {
-      DEBUG_BUILD &&
-        logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`);
-      return true;
-    }
     if (_isIgnoredError(event, options.ignoreErrors)) {
       DEBUG_BUILD &&
         logger.warn(
@@ -196,16 +189,6 @@ function _isAllowedUrl(event: Event, allowUrls?: Array<string | RegExp>): boolea
   return !url ? true : stringMatchesSomePattern(url, allowUrls);
 }
 
-function _isSentryError(event: Event): boolean {
-  try {
-    // @ts-expect-error can't be a sentry error if undefined
-    return event.exception.values[0].type === 'SentryError';
-  } catch (e) {
-    // ignore
-  }
-  return false;
-}
-
 function _getLastValidUrl(frames: StackFrame[] = []): string | null {
   for (let i = frames.length - 1; i >= 0; i--) {
     const frame = frames[i];
diff --git a/packages/core/src/utils-hoist/error.ts b/packages/core/src/utils-hoist/error.ts
@@ -1,6 +1,9 @@
 import type { ConsoleLevel } from '../types-hoist';
 
-/** An error emitted by Sentry SDKs and related utilities. */
+/**
+ * An error emitted by Sentry SDKs and related utilities.
+ * @deprecated This class is no longer used and will be removed in a future version. Use `Error` instead.
+ */
 export class SentryError extends Error {
   public logLevel: ConsoleLevel;
 
diff --git a/packages/core/src/utils-hoist/index.ts b/packages/core/src/utils-hoist/index.ts
@@ -2,6 +2,7 @@ export { applyAggregateErrorsToEvent } from './aggregate-errors';
 export { getBreadcrumbLogLevelFromHttpStatusCode } from './breadcrumb-log-level';
 export { getComponentName, getLocationHref, htmlTreeAsString } from './browser';
 export { dsnFromString, dsnToString, makeDsn } from './dsn';
+// eslint-disable-next-line deprecation/deprecation
 export { SentryError } from './error';
 export { GLOBAL_OBJ } from './worldwide';
 export type { InternalGlobal } from './worldwide';
diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts
@@ -1,7 +1,6 @@
 import { afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest';
 import {
   Scope,
-  SentryError,
   SyncPromise,
   addBreadcrumb,
   dsnToString,
@@ -533,7 +532,7 @@ describe('Client', () => {
       );
     });
 
-    test('it adds a trace context to all events xxx', () => {
+    test('it adds a trace context to all events', () => {
       expect.assertions(1);
 
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
@@ -1206,7 +1205,7 @@ describe('Client', () => {
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend });
       const client = new TestClient(options);
       const captureExceptionSpy = vi.spyOn(client, 'captureException');
-      const loggerWarnSpy = vi.spyOn(loggerModule.logger, 'log');
+      const loggerLogSpy = vi.spyOn(loggerModule.logger, 'log');
 
       client.captureEvent({ message: 'hello' });
 
@@ -1215,7 +1214,7 @@ describe('Client', () => {
       // This proves that the reason the event didn't send/didn't get set on the test client is not because there was an
       // error, but because `beforeSend` returned `null`
       expect(captureExceptionSpy).not.toBeCalled();
-      expect(loggerWarnSpy).toBeCalledWith('before send for type `error` returned `null`, will not send event.');
+      expect(loggerLogSpy).toBeCalledWith('before send for type `error` returned `null`, will not send event.');
     });
 
     test('calls `beforeSendTransaction` and discards the event', () => {
@@ -1225,7 +1224,7 @@ describe('Client', () => {
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction });
       const client = new TestClient(options);
       const captureExceptionSpy = vi.spyOn(client, 'captureException');
-      const loggerWarnSpy = vi.spyOn(loggerModule.logger, 'log');
+      const loggerLogSpy = vi.spyOn(loggerModule.logger, 'log');
 
       client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' });
 
@@ -1234,7 +1233,7 @@ describe('Client', () => {
       // This proves that the reason the event didn't send/didn't get set on the test client is not because there was an
       // error, but because `beforeSendTransaction` returned `null`
       expect(captureExceptionSpy).not.toBeCalled();
-      expect(loggerWarnSpy).toBeCalledWith('before send for type `transaction` returned `null`, will not send event.');
+      expect(loggerLogSpy).toBeCalledWith('before send for type `transaction` returned `null`, will not send event.');
     });
 
     test('does not discard span and warn when returning null from `beforeSendSpan', () => {
@@ -1293,9 +1292,7 @@ describe('Client', () => {
 
         expect(beforeSend).toHaveBeenCalled();
         expect(TestClient.instance!.event).toBeUndefined();
-        expect(loggerWarnSpy).toBeCalledWith(
-          new SentryError('before send for type `error` must return `null` or a valid event.'),
-        );
+        expect(loggerWarnSpy).toBeCalledWith('before send for type `error` must return `null` or a valid event.');
       }
     });
 
@@ -1314,9 +1311,7 @@ describe('Client', () => {
 
         expect(beforeSendTransaction).toHaveBeenCalled();
         expect(TestClient.instance!.event).toBeUndefined();
-        expect(loggerWarnSpy).toBeCalledWith(
-          new SentryError('before send for type `transaction` must return `null` or a valid event.'),
-        );
+        expect(loggerWarnSpy).toBeCalledWith('before send for type `transaction` must return `null` or a valid event.');
       }
     });
 
@@ -1688,9 +1683,7 @@ describe('Client', () => {
         originalException: exception,
       });
       expect(loggerWarnSpy).toBeCalledWith(
-        new SentryError(
-          `Event processing pipeline threw an error, original event will not be sent. Details have been sent as a new event.\nReason: ${exception}`,
-        ),
+        `Event processing pipeline threw an error, original event will not be sent. Details have been sent as a new event.\nReason: ${exception}`,
       );
     });
 
diff --git a/packages/core/test/lib/integrations/eventFilters.test.ts b/packages/core/test/lib/integrations/eventFilters.test.ts
@@ -311,17 +311,6 @@ const EVENT_WITH_VALUE: Event = {
   },
 };
 
-const SENTRY_EVENT: Event = {
-  exception: {
-    values: [
-      {
-        type: 'SentryError',
-        value: 'something something server connection',
-      },
-    ],
-  },
-};
-
 const SCRIPT_ERROR_EVENT: Event = {
   exception: {
     values: [
@@ -425,22 +414,6 @@ describe.each([
   ['InboundFilters', inboundFiltersIntegration],
   ['EventFilters', eventFiltersIntegration],
 ])('%s', (_, integrationFn) => {
-  describe('_isSentryError', () => {
-    it('should work as expected', () => {
-      const eventProcessor = createEventFiltersEventProcessor(integrationFn);
-      expect(eventProcessor(MESSAGE_EVENT, {})).toBe(MESSAGE_EVENT);
-      expect(eventProcessor(EXCEPTION_EVENT, {})).toBe(EXCEPTION_EVENT);
-      expect(eventProcessor(SENTRY_EVENT, {})).toBe(null);
-    });
-
-    it('should be configurable', () => {
-      const eventProcessor = createEventFiltersEventProcessor(integrationFn, { ignoreInternal: false });
-      expect(eventProcessor(MESSAGE_EVENT, {})).toBe(MESSAGE_EVENT);
-      expect(eventProcessor(EXCEPTION_EVENT, {})).toBe(EXCEPTION_EVENT);
-      expect(eventProcessor(SENTRY_EVENT, {})).toBe(SENTRY_EVENT);
-    });
-  });
-
   describe('ignoreErrors', () => {
     it('string filter with partial match', () => {
       const eventProcessor = createEventFiltersEventProcessor(integrationFn, {
diff --git a/packages/core/test/utils-hoist/is.test.ts b/packages/core/test/utils-hoist/is.test.ts
@@ -14,16 +14,6 @@ import { supportsDOMError, supportsDOMException, supportsErrorEvent } from '../.
 import { resolvedSyncPromise } from '../../src/utils-hoist/syncpromise';
 import { testOnlyIfNodeVersionAtLeast } from './testutils';
 
-class SentryError extends Error {
-  public name: string;
-
-  public constructor(public message: string) {
-    super(message);
-    this.name = new.target.prototype.constructor.name;
-    Object.setPrototypeOf(this, new.target.prototype);
-  }
-}
-
 if (supportsDOMError()) {
   describe('isDOMError()', () => {
     test('should work as advertised', () => {
@@ -47,7 +37,6 @@ describe('isError()', () => {
   test('should work as advertised', () => {
     expect(isError(new Error())).toEqual(true);
     expect(isError(new ReferenceError())).toEqual(true);
-    expect(isError(new SentryError('message'))).toEqual(true);
     expect(isError({})).toEqual(false);
     expect(
       isError({
diff --git a/packages/opentelemetry/test/utils/mapStatus.test.ts b/packages/opentelemetry/test/utils/mapStatus.test.ts
@@ -81,7 +81,7 @@ describe('mapStatus', () => {
     expect(actual).toEqual(expected);
   });
 
-  it('works with string SEMATTRS_HTTP_STATUS_CODE xxx', () => {
+  it('works with string SEMATTRS_HTTP_STATUS_CODE', () => {
     const span = createSpan('test-span');
 
     span.setStatus({ code: 0 }); // UNSET
