fix(core): Drop sampled-out errors before they take a promise buffer slot

packages/core/src/client.ts

@@ -52,6 +52,7 @@ import { makePromiseBuffer, type PromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from
 import { safeMathRandom } from './utils/randomSafeContext';
 import { safeCallback } from './utils/safeCallback';
 import { reparentChildSpans, shouldIgnoreSpan } from './utils/should-ignore-span';
+import { rejectedSyncPromise } from './utils/syncpromise';
 import { safeUnref } from './utils/timer';
 import { convertSpanJsonToTransactionEvent, convertTransactionEventToSpanJson } from './utils/transactionEvent';
 import { maybeWarnAboutIgnoredTransactionOptions } from './utils/warnAboutIgnoredTransactionOptions';
@@ -105,6 +106,12 @@ function _makeDoNotSendEventError(message: string): DoNotSendEventError {
   };
 }
 
+function _makeSampledOutError(sampleRate: ClientOptions['sampleRate']): DoNotSendEventError {
+  return _makeDoNotSendEventError(
+    `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`,
+  );
+}
+
 function _isInternalError(error: unknown): error is InternalError {
   return isObjectLike(error) && INTERNAL_ERROR_SYMBOL in error;
 }
@@ -1550,11 +1557,20 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
     // 0.0 === 0% events are sent
     // Sampling for transaction happens somewhere else
     const parsedSampleRate = typeof sampleRate === 'undefined' ? undefined : parseSampleRate(sampleRate);
+    const isSampledOut = isError && typeof parsedSampleRate === 'number' && safeMathRandom() > parsedSampleRate;
     const dataCategory = getDataCategoryByType(event.type);
     // Spans that event processors removed are already recorded in `prepareEvent`, so all later
     // span outcomes are relative to the span count of the prepared event.
     let preparedSpanCount = 0;
 
+    const session = currentScope.getSession() || isolationScope.getSession();
+    // A sampled-out error only runs the pipeline to update the session. Without a session it could update,
+    // drop it right away so it doesn't hold a promise buffer slot while event processors run.
+    if (isSampledOut && session?.status !== 'ok') {
+      this.recordDroppedEvent('sample_rate', 'error');
+      return rejectedSyncPromise(_makeSampledOutError(sampleRate));
+    }
+
     return this._prepareEvent(event, hint, currentScope, isolationScope)
       .then(prepared => {
         if (prepared === null) {
@@ -1602,16 +1618,13 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
           throw _makeDoNotSendEventError(`${dropMessage}, will not send event.`);
         }
 
-        const session = currentScope.getSession() || isolationScope.getSession();
         if (isError && session) {
           this._updateSessionFromEvent(session, processedEvent);
         }
 
-        if (isError && typeof parsedSampleRate === 'number' && safeMathRandom() > parsedSampleRate) {
+        if (isSampledOut) {
           this.recordDroppedEvent('sample_rate', 'error');
-          throw _makeDoNotSendEventError(
-            `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`,
-          );
+          throw _makeSampledOutError(sampleRate);
         }
 
         if (isTransaction) {

packages/core/test/lib/client.test.ts

@@ -552,20 +552,21 @@ describe('Client', () => {
       expect(eventId).toEqual(lastEventId());
     });
 
-    test('sets lastEventId when an error is sampled out', () => {
+    test('does not set lastEventId when an error is sampled out without a session', () => {
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, sampleRate: 0 });
       const client = new TestClient(options);
 
-      const eventId = client.captureException(new Error('sampled-out exception'));
+      client.captureException(new Error('sampled-out exception'));
 
-      expect(eventId).toEqual(lastEventId());
+      expect(lastEventId()).toBeUndefined();
       expect(TestClient.instance!.event).toBeUndefined();
     });
 
-    test('(known limitation) replaces lastEventId with a sampled-out error ID', () => {
+    test('(known limitation) replaces lastEventId with a sampled-out error ID when a session exists', () => {
       // After a successfully sent error, a subsequent sampled-out error replaces lastEventId() even though that new ID has no corresponding event in Sentry.
-      // The `setLastEventId` call in `_prepareEvent` now executes before the `sampleRate` check
+      // With a session, the `setLastEventId` call in `_prepareEvent` executes before the `sampleRate` check
       const client = new TestClient(getDefaultTestClientOptions({ dsn: PUBLIC_DSN }));
+      getCurrentScope().setSession(makeSession());
 
       client.captureException(new Error('sent exception'), { event_id: 'sent-event-id' });
       client.getOptions().sampleRate = 0;
@@ -4531,6 +4532,41 @@ describe('Client', () => {
       ]);
     });
 
+    it('drops sampled-out errors without a session before they take a buffer slot', async () => {
+      const options = getDefaultTestClientOptions({
+        dsn: PUBLIC_DSN,
+        sampleRate: 0,
+        transportOptions: { bufferSize: 1 },
+      });
+      const client = new TestClient(options);
+
+      client.addIntegration(new AsyncTestIntegration());
+
+      client.captureException(new Error('first'));
+      client.captureException(new Error('second'));
+      client.captureException(new Error('third'));
+
+      expect(client._clearOutcomes()).toEqual([{ reason: 'sample_rate', category: 'error', quantity: 3 }]);
+    });
+
+    it('keeps processing sampled-out errors that can update the session', async () => {
+      const options = getDefaultTestClientOptions({
+        dsn: PUBLIC_DSN,
+        sampleRate: 0,
+        transportOptions: { bufferSize: 1 },
+      });
+      const client = new TestClient(options);
+      setCurrentClient(client);
+      getCurrentScope().setSession(makeSession());
+
+      client.addIntegration(new AsyncTestIntegration());
+
+      client.captureException(new Error('first'));
+      client.captureException(new Error('second'));
+
+      expect(client._clearOutcomes()).toEqual([{ reason: 'queue_overflow', category: 'error', quantity: 1 }]);
+    });
+
     it('should skip the promise buffer with sync integrations', async () => {
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, transportOptions: { bufferSize: 1 } });
       const client = new TestClient(options);