fix(core): Avoid blocking the process when calling flush on empty buffer (#19062)

This PR fixes a bug when calling `Sentry.flush()` (or `client.flush()`)
and there's no telemetry being processed by the SDK. While previously
we'd wait for the passed-in timeout, we now exit immediately.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Lms24 <8420481+Lms24@users.noreply.github.com>
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
Co-authored-by: Charly Gomez <charly.gomez@sentry.io>

Author: 3 people

.size-limit.js

@@ -255,7 +255,7 @@ module.exports = [
     path: createCDNPath('bundle.tracing.logs.metrics.min.js'),
     gzip: false,
     brotli: false,
-    limit: '130 KB',
+    limit: '131 KB',
   },
   {
     name: 'CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed',

packages/core/src/client.ts

@@ -1150,7 +1150,6 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
   protected async _isClientDoneProcessing(timeout?: number): Promise<boolean> {
     let ticked = 0;
 
-    // if no timeout is provided, we wait "forever" until everything is processed
     while (!timeout || ticked < timeout) {
       await new Promise(resolve => setTimeout(resolve, 1));
 

packages/core/src/transports/offline.ts

@@ -4,6 +4,7 @@ import type { InternalBaseTransportOptions, Transport, TransportMakeRequestRespo
 import { debug } from '../utils/debug-logger';
 import { envelopeContainsItemType } from '../utils/envelope';
 import { parseRetryAfterHeader } from '../utils/ratelimit';
+import { safeUnref } from '../utils/timer';
 
 export const MIN_DELAY = 100; // 100 ms
 export const START_DELAY = 5_000; // 5 seconds
@@ -97,26 +98,24 @@ export function makeOfflineTransport<TO>(
         clearTimeout(flushTimer as ReturnType<typeof setTimeout>);
       }
 
-      flushTimer = setTimeout(async () => {
-        flushTimer = undefined;
-
-        const found = await store.shift();
-        if (found) {
-          log('Attempting to send previously queued event');
+      // We need to unref the timer in node.js, otherwise the node process never exit.
+      flushTimer = safeUnref(
+        setTimeout(async () => {
+          flushTimer = undefined;
 
-          // We should to update the sent_at timestamp to the current time.
-          found[0].sent_at = new Date().toISOString();
+          const found = await store.shift();
+          if (found) {
+            log('Attempting to send previously queued event');
 
-          void send(found, true).catch(e => {
-            log('Failed to retry sending', e);
-          });
-        }
-      }, delay) as Timer;
+            // We should to update the sent_at timestamp to the current time.
+            found[0].sent_at = new Date().toISOString();
 
-      // We need to unref the timer in node.js, otherwise the node process never exit.
-      if (typeof flushTimer !== 'number' && flushTimer.unref) {
-        flushTimer.unref();
-      }
+            void send(found, true).catch(e => {
+              log('Failed to retry sending', e);
+            });
+          }
+        }, delay),
+      ) as Timer;
     }
 
     function flushWithBackOff(): void {

packages/core/src/utils/promisebuffer.ts

@@ -1,4 +1,5 @@
 import { rejectedSyncPromise, resolvedSyncPromise } from './syncpromise';
+import { safeUnref } from './timer';
 
 export interface PromiseBuffer<T> {
   // exposes the internal array so tests can assert on the state of it.
@@ -77,10 +78,11 @@ export function makePromiseBuffer<T>(limit: number = 100): PromiseBuffer<T> {
       return drainPromise;
     }
 
-    const promises = [drainPromise, new Promise<boolean>(resolve => setTimeout(() => resolve(false), timeout))];
+    const promises = [
+      drainPromise,
+      new Promise<boolean>(resolve => safeUnref(setTimeout(() => resolve(false), timeout))),
+    ];
 
-    // Promise.race will resolve to the first promise that resolves or rejects
-    // So if the drainPromise resolves, the timeout promise will be ignored
     return Promise.race(promises);
   }
 

packages/core/src/utils/timer.ts

@@ -0,0 +1,15 @@
+/**
+ * Calls `unref` on a timer, if the method is available on @param timer.
+ *
+ * `unref()` is used to allow processes to exit immediately, even if the timer
+ * is still running and hasn't resolved yet.
+ *
+ * Use this in places where code can run on browser or server, since browsers
+ * do not support `unref`.
+ */
+export function safeUnref(timer: ReturnType<typeof setTimeout>): ReturnType<typeof setTimeout> {
+  if (typeof timer === 'object' && typeof timer.unref === 'function') {
+    timer.unref();
+  }
+  return timer;
+}

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

@@ -2205,6 +2205,53 @@ describe('Client', () => {
         }),
       ]);
     });
+
+    test('flush returns immediately when nothing is processing', async () => {
+      vi.useFakeTimers();
+      expect.assertions(2);
+
+      const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
+      const client = new TestClient(options);
+
+      // just to ensure the client init'd
+      vi.advanceTimersByTime(100);
+
+      const elapsed = Date.now();
+      const done = client.flush(1000).then(result => {
+        expect(result).toBe(true);
+        expect(Date.now() - elapsed).toBeLessThan(2);
+      });
+
+      // ensures that only after 1 ms, we're already done flushing
+      vi.advanceTimersByTime(1);
+      await done;
+    });
+
+    test('flush with early exit when processing completes', async () => {
+      vi.useRealTimers();
+      expect.assertions(3);
+
+      const { makeTransport, getSendCalled, getSentCount } = makeFakeTransport(50);
+
+      const client = new TestClient(
+        getDefaultTestClientOptions({
+          dsn: PUBLIC_DSN,
+          enableSend: true,
+          transport: makeTransport,
+        }),
+      );
+
+      client.captureMessage('test');
+      expect(getSendCalled()).toEqual(1);
+
+      const startTime = Date.now();
+      await client.flush(5000);
+      const elapsed = Date.now() - startTime;
+
+      expect(getSentCount()).toEqual(1);
+      // if this flakes, remove the test
+      expect(elapsed).toBeLessThan(1000);
+    });
   });
 
   describe('sendEvent', () => {

packages/core/test/lib/utils/promisebuffer.test.ts

@@ -252,4 +252,16 @@ describe('PromiseBuffer', () => {
       expect(e).toEqual(new Error('whoops'));
     }
   });
+
+  test('drain returns immediately when buffer is empty', async () => {
+    const buffer = makePromiseBuffer();
+    expect(buffer.$.length).toEqual(0);
+
+    const startTime = Date.now();
+    const result = await buffer.drain(5000);
+    const elapsed = Date.now() - startTime;
+
+    expect(result).toBe(true);
+    expect(elapsed).toBeLessThan(100);
+  });
 });

packages/core/test/lib/utils/timer.test.ts

@@ -0,0 +1,32 @@
+import { describe, expect, it, vi } from 'vitest';
+import { safeUnref } from '../../../src/utils/timer';
+
+describe('safeUnref', () => {
+  it('calls unref on a NodeJS timer', () => {
+    const timeout = setTimeout(() => {}, 1000);
+    const unrefSpy = vi.spyOn(timeout, 'unref');
+    safeUnref(timeout);
+    expect(unrefSpy).toHaveBeenCalledOnce();
+  });
+
+  it('returns the timer', () => {
+    const timeout = setTimeout(() => {}, 1000);
+    const result = safeUnref(timeout);
+    expect(result).toBe(timeout);
+  });
+
+  it('handles multiple unref calls', () => {
+    const timeout = setTimeout(() => {}, 1000);
+    const unrefSpy = vi.spyOn(timeout, 'unref');
+
+    const result = safeUnref(timeout);
+    result.unref();
+
+    expect(unrefSpy).toHaveBeenCalledTimes(2);
+  });
+
+  it("doesn't throw for a browser timer", () => {
+    const timer = safeUnref(385 as unknown as ReturnType<typeof setTimeout>);
+    expect(timer).toBe(385);
+  });
+});

packages/node-core/test/sdk/client.test.ts

@@ -386,4 +386,26 @@ describe('NodeClient', () => {
       expect(processOffSpy).toHaveBeenNthCalledWith(1, 'beforeExit', expect.any(Function));
     });
   });
+
+  describe('flush', () => {
+    it('flush returns immediately when nothing is processing', async () => {
+      const options = getDefaultNodeClientOptions();
+      const client = new NodeClient(options);
+
+      const startTime = Date.now();
+      const result = await client.flush(1000);
+      const elapsed = Date.now() - startTime;
+
+      expect(result).toBe(true);
+      expect(elapsed).toBeLessThan(100);
+    });
+
+    it('flush does not block process exit with unref timers', async () => {
+      const options = getDefaultNodeClientOptions();
+      const client = new NodeClient(options);
+
+      const result = await client.flush(5000);
+      expect(result).toBe(true);
+    });
+  });
 });