diff --git a/.changeset/fix-retry-transient-status-codes.md b/.changeset/fix-retry-transient-status-codes.md new file mode 100644 index 00000000000..3cb4980c52d --- /dev/null +++ b/.changeset/fix-retry-transient-status-codes.md @@ -0,0 +1,11 @@ +--- +"@effect/platform": patch +--- + +Fix `retryTransient` to use correct transient status codes + +Changed `isTransientResponse` from `status >= 429` to an explicit allowlist (408, 429, 500, 502, 503, 504). This correctly excludes 501 (Not Implemented) and 505+ permanent errors, while including 408 (Request Timeout) which was previously missed. + +Also aligned response retry behavior with v4: the `while` predicate now only applies to error retries, not response retries. Response retries are determined solely by `isTransientResponse`. This matches the semantic intent since `while` is typed for errors, not responses. + +Fixes #5995 diff --git a/packages/platform/src/HttpClient.ts b/packages/platform/src/HttpClient.ts index 0a8a2f58817..628e9e52296 100644 --- a/packages/platform/src/HttpClient.ts +++ b/packages/platform/src/HttpClient.ts @@ -534,7 +534,7 @@ export const retryTransient: { >( options: { readonly mode?: Mode | undefined - readonly while?: Predicate.Predicate> + readonly while?: Predicate.Predicate> readonly schedule?: Schedule.Schedule, R1> readonly times?: number } | Schedule.Schedule, R1> @@ -552,7 +552,7 @@ export const retryTransient: { self: HttpClient.With, options: { readonly mode?: Mode | undefined - readonly while?: Predicate.Predicate> + readonly while?: Predicate.Predicate> readonly schedule?: Schedule.Schedule, R1> readonly times?: number } | Schedule.Schedule, R1> diff --git a/packages/platform/src/internal/httpClient.ts b/packages/platform/src/internal/httpClient.ts index 19b9276f2ab..503016040f5 100644 --- a/packages/platform/src/internal/httpClient.ts +++ b/packages/platform/src/internal/httpClient.ts @@ -755,7 +755,7 @@ export const retryTransient: { >( options: { readonly mode?: Mode | undefined - readonly while?: Predicate.Predicate> + readonly while?: Predicate.Predicate> readonly schedule?: Schedule.Schedule, R1> readonly times?: number } | Schedule.Schedule, R1> @@ -773,7 +773,7 @@ export const retryTransient: { self: Client.HttpClient.With, options: { readonly mode?: Mode | undefined - readonly while?: Predicate.Predicate> + readonly while?: Predicate.Predicate> readonly schedule?: Schedule.Schedule, R1> readonly times?: number } | Schedule.Schedule, R1> @@ -784,7 +784,7 @@ export const retryTransient: { self: Client.HttpClient.With, options: { readonly mode?: "errors-only" | "response-only" | "both" | undefined - readonly while?: Predicate.Predicate> + readonly while?: Predicate.Predicate> readonly schedule?: Schedule.Schedule, R1> readonly times?: number } | Schedule.Schedule, R1> @@ -800,9 +800,7 @@ export const retryTransient: { mode === "errors-only" ? identity : Effect.repeat({ schedule: passthroughSchedule, times, - while: isOnlySchedule || options.while === undefined - ? isTransientResponse - : Predicate.and(isTransientResponse, options.while) + while: isTransientResponse }), mode === "response-only" ? identity : Effect.retry({ while: isOnlySchedule || options.while === undefined @@ -824,7 +822,13 @@ const isTransientHttpError = (error: unknown) => ((error._tag === "RequestError" && error.reason === "Transport") || (error._tag === "ResponseError" && isTransientResponse(error.response))) -const isTransientResponse = (response: ClientResponse.HttpClientResponse) => response.status >= 429 +const isTransientResponse = (response: ClientResponse.HttpClientResponse) => + response.status === 408 || + response.status === 429 || + response.status === 500 || + response.status === 502 || + response.status === 503 || + response.status === 504 /** @internal */ export const tap = dual< diff --git a/packages/platform/test/HttpClient.test.ts b/packages/platform/test/HttpClient.test.ts index f46433ddf56..ca1c6067343 100644 --- a/packages/platform/test/HttpClient.test.ts +++ b/packages/platform/test/HttpClient.test.ts @@ -305,4 +305,111 @@ describe("HttpClient", () => { Effect.provide(FetchHttpClient.layer), Effect.provideService(FetchHttpClient.RequestInit, { redirect: "manual" }) ), 30000) + + describe("retryTransient", () => { + const makeTestClient = (status: number) => { + const attemptsRef = Ref.unsafeMake(0) + const client = HttpClient.make((request) => + Effect.gen(function*() { + yield* Ref.update(attemptsRef, (n) => n + 1) + return HttpClientResponse.fromWeb(request, new Response(null, { status })) + }) + ) + return { attemptsRef, client } + } + + it.effect("retries 408 Request Timeout", () => + Effect.gen(function*() { + const { attemptsRef, client } = makeTestClient(408) + const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 })) + yield* retryClient.get("http://test/").pipe(Effect.ignore) + const attempts = yield* Ref.get(attemptsRef) + strictEqual(attempts, 3) + })) + + it.effect("retries 429 Too Many Requests", () => + Effect.gen(function*() { + const { attemptsRef, client } = makeTestClient(429) + const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 })) + yield* retryClient.get("http://test/").pipe(Effect.ignore) + const attempts = yield* Ref.get(attemptsRef) + strictEqual(attempts, 3) + })) + + it.effect("retries 500 Internal Server Error", () => + Effect.gen(function*() { + const { attemptsRef, client } = makeTestClient(500) + const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 })) + yield* retryClient.get("http://test/").pipe(Effect.ignore) + const attempts = yield* Ref.get(attemptsRef) + strictEqual(attempts, 3) + })) + + it.effect("retries 502 Bad Gateway", () => + Effect.gen(function*() { + const { attemptsRef, client } = makeTestClient(502) + const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 })) + yield* retryClient.get("http://test/").pipe(Effect.ignore) + const attempts = yield* Ref.get(attemptsRef) + strictEqual(attempts, 3) + })) + + it.effect("retries 503 Service Unavailable", () => + Effect.gen(function*() { + const { attemptsRef, client } = makeTestClient(503) + const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 })) + yield* retryClient.get("http://test/").pipe(Effect.ignore) + const attempts = yield* Ref.get(attemptsRef) + strictEqual(attempts, 3) + })) + + it.effect("retries 504 Gateway Timeout", () => + Effect.gen(function*() { + const { attemptsRef, client } = makeTestClient(504) + const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 })) + yield* retryClient.get("http://test/").pipe(Effect.ignore) + const attempts = yield* Ref.get(attemptsRef) + strictEqual(attempts, 3) + })) + + it.effect("does NOT retry 200 OK", () => + Effect.gen(function*() { + const { attemptsRef, client } = makeTestClient(200) + const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 })) + yield* retryClient.get("http://test/").pipe(Effect.ignore) + const attempts = yield* Ref.get(attemptsRef) + strictEqual(attempts, 1) + })) + + it.effect("does NOT retry 501 Not Implemented", () => + Effect.gen(function*() { + const { attemptsRef, client } = makeTestClient(501) + const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 })) + yield* retryClient.get("http://test/").pipe(Effect.ignore) + const attempts = yield* Ref.get(attemptsRef) + strictEqual(attempts, 1) + })) + + it.effect("does NOT retry 505 HTTP Version Not Supported", () => + Effect.gen(function*() { + const { attemptsRef, client } = makeTestClient(505) + const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 })) + yield* retryClient.get("http://test/").pipe(Effect.ignore) + const attempts = yield* Ref.get(attemptsRef) + strictEqual(attempts, 1) + })) + + it.effect("while predicate only applies to error retries, not response retries", () => + Effect.gen(function*() { + const { attemptsRef, client } = makeTestClient(503) + const errorOnlyPredicate = (e: unknown) => e !== null && typeof e === "object" && "_tag" in e + const retryClient = client.pipe(HttpClient.retryTransient({ + times: 2, + while: errorOnlyPredicate + })) + yield* retryClient.get("http://test/").pipe(Effect.ignore) + const attempts = yield* Ref.get(attemptsRef) + strictEqual(attempts, 3) + })) + }) })