Skip to content

Commit 2925bd5

Browse files
committed
fix(HttpClient): backport retryTransient fixes from v4
1 parent e71889f commit 2925bd5

File tree

3 files changed

+126
-4
lines changed

3 files changed

+126
-4
lines changed
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@effect/platform": patch
3+
---
4+
5+
Fix `retryTransient` to use correct transient status codes
6+
7+
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.
8+
9+
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.
10+
11+
Fixes #5995

packages/platform/src/internal/httpClient.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -800,9 +800,7 @@ export const retryTransient: {
800800
mode === "errors-only" ? identity : Effect.repeat({
801801
schedule: passthroughSchedule,
802802
times,
803-
while: isOnlySchedule || options.while === undefined
804-
? isTransientResponse
805-
: Predicate.and(isTransientResponse, options.while)
803+
while: isTransientResponse
806804
}),
807805
mode === "response-only" ? identity : Effect.retry({
808806
while: isOnlySchedule || options.while === undefined
@@ -824,7 +822,13 @@ const isTransientHttpError = (error: unknown) =>
824822
((error._tag === "RequestError" && error.reason === "Transport") ||
825823
(error._tag === "ResponseError" && isTransientResponse(error.response)))
826824

827-
const isTransientResponse = (response: ClientResponse.HttpClientResponse) => response.status >= 429
825+
const isTransientResponse = (response: ClientResponse.HttpClientResponse) =>
826+
response.status === 408 ||
827+
response.status === 429 ||
828+
response.status === 500 ||
829+
response.status === 502 ||
830+
response.status === 503 ||
831+
response.status === 504
828832

829833
/** @internal */
830834
export const tap = dual<

packages/platform/test/HttpClient.test.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,4 +305,111 @@ describe("HttpClient", () => {
305305
Effect.provide(FetchHttpClient.layer),
306306
Effect.provideService(FetchHttpClient.RequestInit, { redirect: "manual" })
307307
), 30000)
308+
309+
describe("retryTransient", () => {
310+
const makeTestClient = (status: number) => {
311+
const attemptsRef = Ref.unsafeMake(0)
312+
const client = HttpClient.make((request) =>
313+
Effect.gen(function*() {
314+
yield* Ref.update(attemptsRef, (n) => n + 1)
315+
return HttpClientResponse.fromWeb(request, new Response(null, { status }))
316+
})
317+
)
318+
return { client, attemptsRef }
319+
}
320+
321+
it.effect("retries 408 Request Timeout", () =>
322+
Effect.gen(function*() {
323+
const { client, attemptsRef } = makeTestClient(408)
324+
const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 }))
325+
yield* retryClient.get("http://test/").pipe(Effect.ignore)
326+
const attempts = yield* Ref.get(attemptsRef)
327+
strictEqual(attempts, 3)
328+
}))
329+
330+
it.effect("retries 429 Too Many Requests", () =>
331+
Effect.gen(function*() {
332+
const { client, attemptsRef } = makeTestClient(429)
333+
const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 }))
334+
yield* retryClient.get("http://test/").pipe(Effect.ignore)
335+
const attempts = yield* Ref.get(attemptsRef)
336+
strictEqual(attempts, 3)
337+
}))
338+
339+
it.effect("retries 500 Internal Server Error", () =>
340+
Effect.gen(function*() {
341+
const { client, attemptsRef } = makeTestClient(500)
342+
const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 }))
343+
yield* retryClient.get("http://test/").pipe(Effect.ignore)
344+
const attempts = yield* Ref.get(attemptsRef)
345+
strictEqual(attempts, 3)
346+
}))
347+
348+
it.effect("retries 502 Bad Gateway", () =>
349+
Effect.gen(function*() {
350+
const { client, attemptsRef } = makeTestClient(502)
351+
const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 }))
352+
yield* retryClient.get("http://test/").pipe(Effect.ignore)
353+
const attempts = yield* Ref.get(attemptsRef)
354+
strictEqual(attempts, 3)
355+
}))
356+
357+
it.effect("retries 503 Service Unavailable", () =>
358+
Effect.gen(function*() {
359+
const { client, attemptsRef } = makeTestClient(503)
360+
const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 }))
361+
yield* retryClient.get("http://test/").pipe(Effect.ignore)
362+
const attempts = yield* Ref.get(attemptsRef)
363+
strictEqual(attempts, 3)
364+
}))
365+
366+
it.effect("retries 504 Gateway Timeout", () =>
367+
Effect.gen(function*() {
368+
const { client, attemptsRef } = makeTestClient(504)
369+
const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 }))
370+
yield* retryClient.get("http://test/").pipe(Effect.ignore)
371+
const attempts = yield* Ref.get(attemptsRef)
372+
strictEqual(attempts, 3)
373+
}))
374+
375+
it.effect("does NOT retry 200 OK", () =>
376+
Effect.gen(function*() {
377+
const { client, attemptsRef } = makeTestClient(200)
378+
const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 }))
379+
yield* retryClient.get("http://test/").pipe(Effect.ignore)
380+
const attempts = yield* Ref.get(attemptsRef)
381+
strictEqual(attempts, 1)
382+
}))
383+
384+
it.effect("does NOT retry 501 Not Implemented", () =>
385+
Effect.gen(function*() {
386+
const { client, attemptsRef } = makeTestClient(501)
387+
const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 }))
388+
yield* retryClient.get("http://test/").pipe(Effect.ignore)
389+
const attempts = yield* Ref.get(attemptsRef)
390+
strictEqual(attempts, 1)
391+
}))
392+
393+
it.effect("does NOT retry 505 HTTP Version Not Supported", () =>
394+
Effect.gen(function*() {
395+
const { client, attemptsRef } = makeTestClient(505)
396+
const retryClient = client.pipe(HttpClient.retryTransient({ times: 2 }))
397+
yield* retryClient.get("http://test/").pipe(Effect.ignore)
398+
const attempts = yield* Ref.get(attemptsRef)
399+
strictEqual(attempts, 1)
400+
}))
401+
402+
it.effect("while predicate only applies to error retries, not response retries", () =>
403+
Effect.gen(function*() {
404+
const { client, attemptsRef } = makeTestClient(503)
405+
const errorOnlyPredicate = (e: unknown) => e !== null && typeof e === "object" && "_tag" in e
406+
const retryClient = client.pipe(HttpClient.retryTransient({
407+
times: 2,
408+
while: errorOnlyPredicate
409+
}))
410+
yield* retryClient.get("http://test/").pipe(Effect.ignore)
411+
const attempts = yield* Ref.get(attemptsRef)
412+
strictEqual(attempts, 3)
413+
}))
414+
})
308415
})

0 commit comments

Comments
 (0)