Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions packages/core/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -112,11 +112,13 @@ function setupWeightBasedFlushing<
// Track weight and timeout in closure variables
let weight = 0;
let flushTimeout: ReturnType<typeof setTimeout> | 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
Expand All @@ -127,10 +129,15 @@ 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 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);
// Note: isTimerActive is reset by the flushHook handler above, not here,
// to avoid race conditions when new items arrive during the flush.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Stuck Timer Halts Automatic Flushing

The isTimerActive flag can get stuck as true when the timer fires but the buffer is empty. This happens because isTimerActive is only reset in the flushHook handler (line 121), but _INTERNAL_flushLogsBuffer returns early without emitting this hook when the buffer is empty. Once stuck, no new timers can start since !isTimerActive evaluates to false, preventing automatic flushing of subsequent logs/metrics until the weight threshold is exceeded.

Fix in Cursor Fix in Web

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For isTimerActive to be set to true, an item must have come in via afterCaptureHook which in turn starts the timeout. Once the buffer flushes isTimerActive will be set back to false via flushHook.

I don't think this scenario can happen.

}, DEFAULT_FLUSH_INTERVAL);
}
});
Expand Down
42 changes: 34 additions & 8 deletions packages/core/test/lib/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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', () => {
Expand Down
Loading