From 53f0acbd39b351fe57a9d53fd2be7fd7e200d9d3 Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Fri, 14 Nov 2025 18:00:16 +0100 Subject: [PATCH 1/2] fix(core): Fix log flush timeout starvation with continuous logging The flush timeout was being reset on every incoming log, preventing flushes when logs arrived continuously. Now, the timer starts on the first log won't get reset, ensuring logs flush within the configured interval. Fixes #18204, getsentry/sentry-react-native#5378 --- packages/core/src/client.ts | 12 ++++++-- packages/core/test/lib/client.test.ts | 42 ++++++++++++++++++++++----- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 53e0328965a4..309edbda3161 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -94,7 +94,7 @@ function _isDoNotSendEventError(error: unknown): error is DoNotSendEventError { * This helper function encapsulates the common pattern of: * 1. Tracking accumulated weight of items * 2. Flushing when weight exceeds threshold (800KB) - * 3. Flushing after idle timeout if no new items arrive + * 3. Flushing after timeout period from the first item * * Uses closure variables to track weight and timeout state. */ @@ -112,11 +112,13 @@ function setupWeightBasedFlushing< // Track weight and timeout in closure variables let weight = 0; let flushTimeout: ReturnType | undefined; + let isTimerActive = false; // @ts-expect-error - TypeScript can't narrow generic hook types to match specific overloads, but we know this is type-safe client.on(flushHook, () => { weight = 0; clearTimeout(flushTimeout); + isTimerActive = false; }); // @ts-expect-error - TypeScript can't narrow generic hook types to match specific overloads, but we know this is type-safe @@ -127,10 +129,14 @@ function setupWeightBasedFlushing< // The weight is a rough estimate, so we flush way before the payload gets too big. if (weight >= 800_000) { flushFn(client); - } else { - clearTimeout(flushTimeout); + } else if (!isTimerActive) { + // Only start timer if one isn't already running. + // This prevents flushing being delayed by logs that arrive close to the timeout limit + // and thus resetting the flushing timeout and delaying logs being flushed. + isTimerActive = true; flushTimeout = setTimeout(() => { flushFn(client); + isTimerActive = false; }, DEFAULT_FLUSH_INTERVAL); } }); diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index acb4197cf4cf..c009d0e0c2a8 100644 --- a/packages/core/test/lib/client.test.ts +++ b/packages/core/test/lib/client.test.ts @@ -2772,7 +2772,7 @@ describe('Client', () => { expect(sendEnvelopeSpy).toHaveBeenCalledTimes(1); }); - it('resets idle timeout when new logs are captured', () => { + it('does not reset idle timeout when new logs are captured', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, enableLogs: true, @@ -2783,26 +2783,52 @@ describe('Client', () => { const sendEnvelopeSpy = vi.spyOn(client, 'sendEnvelope'); - // Add initial log + // Add initial log (starts the timer) _INTERNAL_captureLog({ message: 'test log 1', level: 'info' }, scope); // Fast forward part of the idle timeout vi.advanceTimersByTime(2500); - // Add another log which should reset the timeout + // Add another log which should NOT reset the timeout _INTERNAL_captureLog({ message: 'test log 2', level: 'info' }, scope); - // Fast forward the remaining time + // Fast forward the remaining time to reach the full timeout from the first log vi.advanceTimersByTime(2500); - // Should not have flushed yet since timeout was reset - expect(sendEnvelopeSpy).not.toHaveBeenCalled(); + // Should have flushed both logs since timeout was not reset + expect(sendEnvelopeSpy).toHaveBeenCalledTimes(1); + }); + + it('starts new timer after timeout completes and flushes', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + enableLogs: true, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setClient(client); - // Fast forward the full timeout + const sendEnvelopeSpy = vi.spyOn(client, 'sendEnvelope'); + + // First batch: Add a log and let it flush + _INTERNAL_captureLog({ message: 'test log 1', level: 'info' }, scope); + + // Fast forward to trigger the first flush vi.advanceTimersByTime(5000); - // Now should have flushed both logs expect(sendEnvelopeSpy).toHaveBeenCalledTimes(1); + + // Second batch: Add another log after the first flush completed + _INTERNAL_captureLog({ message: 'test log 2', level: 'info' }, scope); + + // Should not have flushed yet + expect(sendEnvelopeSpy).toHaveBeenCalledTimes(1); + + // Fast forward to trigger the second flush + vi.advanceTimersByTime(5000); + + // Should have flushed the second log + expect(sendEnvelopeSpy).toHaveBeenCalledTimes(2); }); it('flushes logs on flush event', () => { From c96a65d088ace35ca1df6aca475e093ffee2de3f Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Fri, 14 Nov 2025 18:19:50 +0100 Subject: [PATCH 2/2] Ensure timer is only marked inactive after flushHook executes --- packages/core/src/client.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 309edbda3161..f363e61becd7 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -131,12 +131,13 @@ function setupWeightBasedFlushing< flushFn(client); } else if (!isTimerActive) { // Only start timer if one isn't already running. - // This prevents flushing being delayed by logs that arrive close to the timeout limit - // and thus resetting the flushing timeout and delaying logs being flushed. + // This prevents flushing being delayed by items that arrive close to the timeout limit + // and thus resetting the flushing timeout and delaying items being flushed. isTimerActive = true; flushTimeout = setTimeout(() => { flushFn(client); - isTimerActive = false; + // Note: isTimerActive is reset by the flushHook handler above, not here, + // to avoid race conditions when new items arrive during the flush. }, DEFAULT_FLUSH_INTERVAL); } });