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

Author: mydea

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.
  *
@@ -963,10 +997,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);
           }
@@ -1010,9 +1044,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',
         ),
       );
     }
@@ -1023,7 +1056,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;
@@ -1043,7 +1076,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();
@@ -1077,7 +1110,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;
         }
 
@@ -1087,7 +1120,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}`,
         );
       });
@@ -1192,17 +1225,17 @@ function _validateBeforeSendResult(
   if (isThenable(beforeSendResult)) {
     return beforeSendResult.then(
       event => {
-        if (!isPlainObject(event) && event !== null) {
-          throw new SentryError(invalidValueError);
+        if (!isPlainObject(event) && event) {
+          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);
+  } else if (!isPlainObject(beforeSendResult) && beforeSendResult) {
+    throw makeInternalError(invalidValueError);
   }
   return beforeSendResult;
 }

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];

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, {