diff --git a/.size-limit.js b/.size-limit.js index 2a14a40df88b..361e2026f2c7 100644 --- a/.size-limit.js +++ b/.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', diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 56b382a2860e..1ed447ab802d 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -1150,7 +1150,6 @@ export abstract class Client { protected async _isClientDoneProcessing(timeout?: number): Promise { 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)); diff --git a/packages/core/src/transports/offline.ts b/packages/core/src/transports/offline.ts index 88211a4378e7..b799bf691efb 100644 --- a/packages/core/src/transports/offline.ts +++ b/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( clearTimeout(flushTimer as ReturnType); } - 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 { diff --git a/packages/core/src/utils/promisebuffer.ts b/packages/core/src/utils/promisebuffer.ts index f66077a76fd5..4868e30f14b8 100644 --- a/packages/core/src/utils/promisebuffer.ts +++ b/packages/core/src/utils/promisebuffer.ts @@ -1,4 +1,5 @@ import { rejectedSyncPromise, resolvedSyncPromise } from './syncpromise'; +import { safeUnref } from './timer'; export interface PromiseBuffer { // exposes the internal array so tests can assert on the state of it. @@ -77,10 +78,11 @@ export function makePromiseBuffer(limit: number = 100): PromiseBuffer { return drainPromise; } - const promises = [drainPromise, new Promise(resolve => setTimeout(() => resolve(false), timeout))]; + const promises = [ + drainPromise, + new Promise(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); } diff --git a/packages/core/src/utils/timer.ts b/packages/core/src/utils/timer.ts new file mode 100644 index 000000000000..f08b698a8e67 --- /dev/null +++ b/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): ReturnType { + if (typeof timer === 'object' && typeof timer.unref === 'function') { + timer.unref(); + } + return timer; +} diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index 0945af5f019f..09ec34bf4fcc 100644 --- a/packages/core/test/lib/client.test.ts +++ b/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', () => { diff --git a/packages/core/test/lib/utils/promisebuffer.test.ts b/packages/core/test/lib/utils/promisebuffer.test.ts index d1b4b9abc48d..8d45d426b1a4 100644 --- a/packages/core/test/lib/utils/promisebuffer.test.ts +++ b/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); + }); }); diff --git a/packages/core/test/lib/utils/timer.test.ts b/packages/core/test/lib/utils/timer.test.ts new file mode 100644 index 000000000000..cd3b99fa4246 --- /dev/null +++ b/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); + expect(timer).toBe(385); + }); +}); diff --git a/packages/node-core/test/sdk/client.test.ts b/packages/node-core/test/sdk/client.test.ts index 0bcef2669095..cf33049c1ffa 100644 --- a/packages/node-core/test/sdk/client.test.ts +++ b/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); + }); + }); });