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
11 changes: 11 additions & 0 deletions .changeset/fix-retry-transient-status-codes.md
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions packages/platform/src/HttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ export const retryTransient: {
>(
options: {
readonly mode?: Mode | undefined
readonly while?: Predicate.Predicate<NoInfer<Input>>
readonly while?: Predicate.Predicate<NoInfer<E>>
readonly schedule?: Schedule.Schedule<B, NoInfer<Input>, R1>
readonly times?: number
} | Schedule.Schedule<B, NoInfer<Input>, R1>
Expand All @@ -552,7 +552,7 @@ export const retryTransient: {
self: HttpClient.With<E, R>,
options: {
readonly mode?: Mode | undefined
readonly while?: Predicate.Predicate<NoInfer<Input>>
readonly while?: Predicate.Predicate<NoInfer<E>>
readonly schedule?: Schedule.Schedule<B, NoInfer<Input>, R1>
readonly times?: number
} | Schedule.Schedule<B, NoInfer<Input>, R1>
Expand Down
18 changes: 11 additions & 7 deletions packages/platform/src/internal/httpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ export const retryTransient: {
>(
options: {
readonly mode?: Mode | undefined
readonly while?: Predicate.Predicate<NoInfer<Input>>
readonly while?: Predicate.Predicate<NoInfer<E>>
readonly schedule?: Schedule.Schedule<B, NoInfer<Input>, R1>
readonly times?: number
} | Schedule.Schedule<B, NoInfer<Input>, R1>
Expand All @@ -773,7 +773,7 @@ export const retryTransient: {
self: Client.HttpClient.With<E, R>,
options: {
readonly mode?: Mode | undefined
readonly while?: Predicate.Predicate<NoInfer<Input>>
readonly while?: Predicate.Predicate<NoInfer<E>>
readonly schedule?: Schedule.Schedule<B, NoInfer<Input>, R1>
readonly times?: number
} | Schedule.Schedule<B, NoInfer<Input>, R1>
Expand All @@ -784,7 +784,7 @@ export const retryTransient: {
self: Client.HttpClient.With<E, R>,
options: {
readonly mode?: "errors-only" | "response-only" | "both" | undefined
readonly while?: Predicate.Predicate<ClientResponse.HttpClientResponse | NoInfer<E>>
readonly while?: Predicate.Predicate<NoInfer<E>>
readonly schedule?: Schedule.Schedule<B, ClientResponse.HttpClientResponse | NoInfer<E>, R1>
readonly times?: number
} | Schedule.Schedule<B, ClientResponse.HttpClientResponse | NoInfer<E>, R1>
Expand All @@ -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
Expand All @@ -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<
Expand Down
107 changes: 107 additions & 0 deletions packages/platform/test/HttpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}))
})
})