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
45 changes: 44 additions & 1 deletion code/core/src/telemetry/telemetry.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { beforeEach, expect, it, vi } from 'vitest';
import { afterEach, beforeEach, expect, it, vi } from 'vitest';

import { fetch } from './fetch.ts';
import { sendTelemetry } from './telemetry.ts';
Expand All @@ -22,6 +22,10 @@ beforeEach(() => {
fetchMock.mockResolvedValue({ status: 200 } as any);
});

afterEach(() => {
vi.restoreAllMocks();
});

it('makes a fetch request with name and data', async () => {
await sendTelemetry({ eventType: 'dev', payload: { foo: 'bar' } });

Expand All @@ -33,6 +37,45 @@ it('makes a fetch request with name and data', async () => {
});
});

it('abandons a request that never responds, instead of hanging the process', async () => {
const controller = new AbortController();
const timeoutSpy = vi.spyOn(AbortSignal, 'timeout').mockReturnValue(controller.signal);

// fetch that never settles on its own - it only rejects once its signal aborts, which we control via the timeoutSpy above.
fetchMock.mockImplementation(
(_url, init) =>
new Promise((_resolve, reject) => {
const signal = (init as RequestInit | undefined)?.signal;
if (signal?.aborted) {
reject(signal.reason);
} else {
signal?.addEventListener('abort', () => reject(signal.reason));
}
})
);

let settled = false;
const promise = sendTelemetry({ eventType: 'dev', payload: { foo: 'bar' } }).finally(() => {
settled = true;
});

// Let the request reach its in-flight state.
await vi.waitFor(() => expect(fetchMock).toHaveBeenCalled());
expect(timeoutSpy).toHaveBeenCalledWith(30_000);
expect(settled).toBe(false);

// Abort the request, simulating the timeout expiring. This should cause the promise to reject and the sendTelemetry
// call to settle, instead of hanging indefinitely.
controller.abort();

await promise;

expect(settled).toBe(true);

// Ensure the aborted request is not retried
expect(fetchMock).toHaveBeenCalledTimes(1);
});
Comment thread
badams marked this conversation as resolved.

it('retries if fetch fails with a 503', async () => {
fetchMock.mockResolvedValueOnce({ status: 503 } as any);
await sendTelemetry(
Expand Down
14 changes: 12 additions & 2 deletions code/core/src/telemetry/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,28 @@ const prepareRequest = async (data: TelemetryData, context: Record<string, any>,
const sessionId = await getSessionId();
const eventId = nanoid();
const body = { ...rest, eventType, eventId, sessionId, metadata, payload, context };
const signal = AbortSignal.timeout(30_000);
const maxRetries = 3;

return retryingFetch(URL, {
method: 'post',
body: JSON.stringify(body),
headers: { 'Content-Type': 'application/json' },
retries: 3,
retryOn: [503, 504],
retryDelay: (attempt: number) =>
2 ** attempt *
(typeof options?.retryDelay === 'number' && !Number.isNaN(options?.retryDelay)
? options.retryDelay
: 1000),
retryOn: (attempt, error, response) => {
// If explicitly aborted (e.g. by the timeout above), or if we've exhausted our retries, give up.
if (signal.aborted || attempt >= maxRetries) {
return false;
}

// Retry transient network errors and server-overload responses.
return Boolean(error) || response?.status === 503 || response?.status === 504;
},
signal,
});
};

Expand Down
Loading