diff --git a/src/lib/retry.ts b/src/lib/retry.ts index 3540ff73e4..7af5d2f911 100644 --- a/src/lib/retry.ts +++ b/src/lib/retry.ts @@ -1,3 +1,5 @@ +import { TimeoutError } from "./errors.js"; + const MAX_REQUEST_RETRY_JITTER = 250; const MAX_REQUEST_RETRY_DELAY = 10000; const DEFAULT_NUMBER_RETRIES = 3; @@ -27,6 +29,12 @@ async function pause(delay: number) { return new Promise((resolve) => setTimeout(resolve, delay)); } +function calculateWait(nrOfTries: number): number { + let wait = BASE_DELAY * Math.pow(2, nrOfTries - 1); + wait = getRandomInt(wait + 1, wait + MAX_REQUEST_RETRY_JITTER); + return Math.min(wait, MAX_REQUEST_RETRY_DELAY); +} + /** * Configure the retry logic for http calls. * By default, this retries any request that returns a 429 3 times. @@ -43,7 +51,9 @@ export interface RetryConfiguration { */ maxRetries?: number; /** - * Status Codes on which the SDK should trigger retries. + * HTTP Status Codes on which the SDK should trigger retries. + * Note: network-level errors (e.g. ECONNRESET) are always retried up to maxRetries, + * regardless of this setting. Use `enabled: false` to disable all retries. * Defaults to [429]. */ retryWhen?: number[]; @@ -57,20 +67,23 @@ export function retry(action: () => Promise, { maxRetries, retryWhen } const nrOfTriesToAttempt = Math.min(MAX_NUMBER_RETRIES, maxRetries ?? DEFAULT_NUMBER_RETRIES); let nrOfTries = 0; - const retryAndWait = async () => { + const retryAndWait = async (): Promise => { let result: Response; - result = await action(); + try { + result = await action(); + } catch (e: unknown) { + if (!(e instanceof TimeoutError) && nrOfTries < nrOfTriesToAttempt) { + nrOfTries++; + await pause(calculateWait(nrOfTries)); + return retryAndWait(); + } + throw e; + } if ((retryWhen || [429]).includes(result.status) && nrOfTries < nrOfTriesToAttempt) { nrOfTries++; - - let wait = BASE_DELAY * Math.pow(2, nrOfTries - 1); - wait = getRandomInt(wait + 1, wait + MAX_REQUEST_RETRY_JITTER); - wait = Math.min(wait, MAX_REQUEST_RETRY_DELAY); - - await pause(wait); - + await pause(calculateWait(nrOfTries)); result = await retryAndWait(); } diff --git a/tests/lib/runtime.test.ts b/tests/lib/runtime.test.ts index 29ad35aee7..3437f72ac3 100644 --- a/tests/lib/runtime.test.ts +++ b/tests/lib/runtime.test.ts @@ -268,6 +268,100 @@ describe("Runtime", () => { ).rejects.toThrowError(expect.objectContaining({ statusCode: 429 })); }); + it("should retry on socket hang up (ECONNRESET) when retry is enabled", async () => { + // issue #1020 — network errors were not retried, only HTTP status codes were + const request = nock(URL, { encodedQueryParams: true }) + .get("/clients") + .times(2) + .replyWithError({ code: "ECONNRESET", message: "socket hang up" }) + .get("/clients") + .reply(200, [{ client_id: "123" }]); + + const client = new TestClient({ + baseUrl: URL, + parseError, + }); + + const response = await client.testRequest({ + path: `/clients`, + method: "GET", + }); + + const data = (await response.json()) as Array<{ client_id: string }>; + + expect(data[0].client_id).toBe("123"); + expect(request.isDone()).toBe(true); + }); + + it("should throw after exhausting retries on repeated socket hang up", async () => { + // issue #1020 — should give up after maxRetries attempts + nock(URL, { encodedQueryParams: true }) + .get("/clients") + .times(4) + .replyWithError({ code: "ECONNRESET", message: "socket hang up" }); + + const client = new TestClient({ + baseUrl: URL, + parseError, + }); + + await expect( + client.testRequest({ + path: `/clients`, + method: "GET", + }), + ).rejects.toThrowError( + expect.objectContaining({ cause: expect.objectContaining({ message: "socket hang up" }) }), + ); + }); + + it("should not retry on socket hang up when retry is disabled", async () => { + nock(URL, { encodedQueryParams: true }) + .get("/clients") + .replyWithError({ code: "ECONNRESET", message: "socket hang up" }) + .get("/clients") + .reply(200, [{ client_id: "123" }]); + + const client = new TestClient({ + baseUrl: URL, + parseError, + retry: { enabled: false }, + }); + + await expect( + client.testRequest({ + path: `/clients`, + method: "GET", + }), + ).rejects.toThrow(); + }); + + it("should not retry on timeout errors", async () => { + nock(URL) + .get("/clients") + .delayConnection(100) + .reply(200, [{ client_id: "123" }]) + .get("/clients") + .reply(200, [{ client_id: "123" }]); + + const client = new TestClient({ + baseUrl: URL, + parseError, + timeoutDuration: 50, + }); + + await expect( + client.testRequest({ + path: `/clients`, + method: "GET", + }), + ).rejects.toThrowError(expect.objectContaining({ cause: expect.objectContaining({ name: "TimeoutError" }) })); + + // Second nock was never consumed — confirms no retry occurred + expect(nock.pendingMocks().length).toBe(1); + nock.abortPendingRequests(); + }); + it("should timeout after default time", async () => { nock(URL).get("/clients").delayConnection(10000).reply(200, []);