diff --git a/.changeset/summary-retry-backoff.md b/.changeset/summary-retry-backoff.md new file mode 100644 index 000000000..bdcd762be --- /dev/null +++ b/.changeset/summary-retry-backoff.md @@ -0,0 +1,5 @@ +--- +"braintrust": patch +--- + +Add exponential backoff between existing `get_json` retry attempts. diff --git a/js/src/logger.test.ts b/js/src/logger.test.ts index 21231d8e0..d9c89ad48 100644 --- a/js/src/logger.test.ts +++ b/js/src/logger.test.ts @@ -1718,6 +1718,73 @@ test("simulateLoginForTests and simulateLogoutForTests", async () => { } }); +describe("HTTPConnection get_json retries", () => { + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + _exportsForTestingOnly.simulateLogoutForTests(); + }); + + test("backs off before retrying", async () => { + vi.useFakeTimers(); + + const state = await _exportsForTestingOnly.simulateLoginForTests(); + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + new Response("timeout", { + status: 504, + statusText: "Gateway Timeout", + }), + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + state.setFetch(fetchMock as unknown as typeof globalThis.fetch); + + const resultPromise = state.apiConn().get_json("/retry-me", undefined, 1); + + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(999); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(1); + await expect(resultPromise).resolves.toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + test("throws after retry exhaustion", async () => { + vi.useFakeTimers(); + + const state = await _exportsForTestingOnly.simulateLoginForTests(); + const fetchMock = vi.fn().mockImplementation(() => + Promise.resolve( + new Response("timeout", { + status: 504, + statusText: "Gateway Timeout", + }), + ), + ); + state.setFetch(fetchMock as unknown as typeof globalThis.fetch); + + const resultPromise = state.apiConn().get_json("/retry-me", undefined, 2); + const expectedFailure = expect(resultPromise).rejects.toThrow( + "504: Gateway Timeout", + ); + + await vi.advanceTimersByTimeAsync(1000); + await vi.advanceTimersByTimeAsync(2000); + + await expectedFailure; + expect(fetchMock).toHaveBeenCalledTimes(3); + }); +}); + test("span.export handles unauthenticated state", async () => { // Create a span without logging in const logger = initLogger({}); diff --git a/js/src/logger.ts b/js/src/logger.ts index 81da807fe..fdd6ee34b 100644 --- a/js/src/logger.ts +++ b/js/src/logger.ts @@ -1297,6 +1297,11 @@ class HTTPConnection { (e as any).status } ${(e as any).text}`, ); + const sleepTimeS = HTTP_RETRY_BASE_SLEEP_TIME_S * 2 ** i; + debugLogger.info(`Sleeping for ${sleepTimeS}s`); + await new Promise((resolve) => + setTimeout(resolve, sleepTimeS * 1000), + ); continue; } throw e; @@ -2765,6 +2770,7 @@ export class TestBackgroundLogger implements BackgroundLogger { } const BACKGROUND_LOGGER_BASE_SLEEP_TIME_S = 1.0; +const HTTP_RETRY_BASE_SLEEP_TIME_S = 1.0; // We should only have one instance of this object per state object in // 'BraintrustState._bgLogger'. Be careful about spawning multiple