fix(llm): remove package retry policy (#35003)

This commit is contained in:
Aiden Cline 2026-07-02 15:51:12 -05:00 committed by GitHub
commit 460cdc5aec
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 77 additions and 244 deletions

View file

@ -592,7 +592,7 @@ const step = (state: ParserState, event: BedrockEvent) =>
event.modelStreamErrorException?.message ??
event.serviceUnavailableException?.message ??
"Bedrock Converse stream error"
return [state, [LLMEvent.providerError({ message, retryable: true })]] as const
return [state, [LLMEvent.providerError({ message })]] as const
}
if (event.validationException || event.throttlingException) {
@ -604,7 +604,6 @@ const step = (state: ParserState, event: BedrockEvent) =>
LLMEvent.providerError({
message,
classification: event.validationException && isContextOverflow(message) ? "context-overflow" : undefined,
retryable: event.throttlingException !== undefined,
}),
],
] as const

View file

@ -1,4 +1,4 @@
import { Cause, Context, Effect, Layer, Random } from "effect"
import { Cause, Context, Effect, Layer } from "effect"
import {
FetchHttpClient,
Headers,
@ -33,9 +33,6 @@ export interface Interface {
export class Service extends Context.Service<Service, Interface>()("@opencode/LLM/RequestExecutor") {}
const BODY_LIMIT = 16_384
const MAX_RETRIES = 2
const BASE_DELAY_MS = 500
const MAX_DELAY_MS = 10_000
const REDACTED = "<redacted>"
// One source of truth for what counts as a sensitive name across headers,
@ -88,7 +85,7 @@ const requestId = (headers: Record<string, string>) => {
)
}
const retryableStatus = (status: number) => status === 429 || status === 503 || status === 504 || status === 529
const providerInternalStatus = (status: number) => status === 429 || status === 503 || status === 504 || status === 529
const retryAfterMs = (headers: Record<string, string>) => {
const millis = Number(headers["retry-after-ms"])
@ -263,7 +260,7 @@ const statusReason = (input: {
http: input.http,
})
}
if (input.status >= 500 || retryableStatus(input.status)) {
if (input.status >= 500 || providerInternalStatus(input.status)) {
return new ProviderInternalReason({
message: input.message,
status: input.status,
@ -342,27 +339,6 @@ const toHttpError = (redactedNames: ReadonlyArray<string | RegExp>) => (error: u
})
}
const retryDelay = (error: LLMError, attempt: number) => {
if (error.retryAfterMs !== undefined) return Effect.succeed(Math.min(error.retryAfterMs, MAX_DELAY_MS))
return Random.nextBetween(
Math.min(BASE_DELAY_MS * 2 ** attempt * 0.8, MAX_DELAY_MS),
Math.min(BASE_DELAY_MS * 2 ** attempt * 1.2, MAX_DELAY_MS),
).pipe(Effect.map((delay) => Math.round(delay)))
}
const retryStatusFailures = <A, R>(
effect: Effect.Effect<A, LLMError, R>,
retries = MAX_RETRIES,
attempt = 0,
): Effect.Effect<A, LLMError, R> =>
Effect.catchTag(effect, "LLM.Error", (error): Effect.Effect<A, LLMError, R> => {
if (!error.retryable || retries <= 0) return Effect.fail(error)
return retryDelay(error, attempt).pipe(
Effect.flatMap((delay) => Effect.sleep(delay)),
Effect.flatMap(() => retryStatusFailures(effect, retries - 1, attempt + 1)),
)
})
export const layer: Layer.Layer<Service, never, HttpClient.HttpClient> = Layer.effect(
Service,
Effect.gen(function* () {
@ -375,7 +351,7 @@ export const layer: Layer.Layer<Service, never, HttpClient.HttpClient> = Layer.e
.pipe(Effect.mapError(toHttpError(redactedNames)), Effect.flatMap(statusError(request, redactedNames)))
})
return Service.of({
execute: (request) => retryStatusFailures(executeOnce(request)),
execute: executeOnce,
})
}),
)

View file

@ -38,11 +38,7 @@ export class InvalidRequestReason extends Schema.Class<InvalidRequestReason>("LL
classification: Schema.optional(ProviderFailureClassification),
providerMetadata: Schema.optional(ProviderMetadata),
http: Schema.optional(HttpContext),
}) {
get retryable() {
return false
}
}
}) {}
export class NoRouteReason extends Schema.Class<NoRouteReason>("LLM.Error.NoRoute")({
_tag: Schema.tag("NoRoute"),
@ -50,10 +46,6 @@ export class NoRouteReason extends Schema.Class<NoRouteReason>("LLM.Error.NoRout
provider: ProviderID,
model: ModelID,
}) {
get retryable() {
return false
}
get message() {
return `No LLM route for ${this.provider}/${this.model} using ${this.route}`
}
@ -65,11 +57,7 @@ export class AuthenticationReason extends Schema.Class<AuthenticationReason>("LL
kind: Schema.Literals(["missing", "invalid", "expired", "insufficient-permissions", "unknown"]),
providerMetadata: Schema.optional(ProviderMetadata),
http: Schema.optional(HttpContext),
}) {
get retryable() {
return false
}
}
}) {}
export class RateLimitReason extends Schema.Class<RateLimitReason>("LLM.Error.RateLimit")({
_tag: Schema.tag("RateLimit"),
@ -78,33 +66,21 @@ export class RateLimitReason extends Schema.Class<RateLimitReason>("LLM.Error.Ra
rateLimit: Schema.optional(HttpRateLimitDetails),
providerMetadata: Schema.optional(ProviderMetadata),
http: Schema.optional(HttpContext),
}) {
get retryable() {
return true
}
}
}) {}
export class QuotaExceededReason extends Schema.Class<QuotaExceededReason>("LLM.Error.QuotaExceeded")({
_tag: Schema.tag("QuotaExceeded"),
message: Schema.String,
providerMetadata: Schema.optional(ProviderMetadata),
http: Schema.optional(HttpContext),
}) {
get retryable() {
return false
}
}
}) {}
export class ContentPolicyReason extends Schema.Class<ContentPolicyReason>("LLM.Error.ContentPolicy")({
_tag: Schema.tag("ContentPolicy"),
message: Schema.String,
providerMetadata: Schema.optional(ProviderMetadata),
http: Schema.optional(HttpContext),
}) {
get retryable() {
return false
}
}
}) {}
export class ProviderInternalReason extends Schema.Class<ProviderInternalReason>("LLM.Error.ProviderInternal")({
_tag: Schema.tag("ProviderInternal"),
@ -113,11 +89,7 @@ export class ProviderInternalReason extends Schema.Class<ProviderInternalReason>
retryAfterMs: Schema.optional(Schema.Number),
providerMetadata: Schema.optional(ProviderMetadata),
http: Schema.optional(HttpContext),
}) {
get retryable() {
return true
}
}
}) {}
export class TransportReason extends Schema.Class<TransportReason>("LLM.Error.Transport")({
_tag: Schema.tag("Transport"),
@ -125,11 +97,7 @@ export class TransportReason extends Schema.Class<TransportReason>("LLM.Error.Tr
kind: Schema.optional(Schema.String),
url: Schema.optional(Schema.String),
http: Schema.optional(HttpContext),
}) {
get retryable() {
return false
}
}
}) {}
export class InvalidProviderOutputReason extends Schema.Class<InvalidProviderOutputReason>(
"LLM.Error.InvalidProviderOutput",
@ -139,11 +107,7 @@ export class InvalidProviderOutputReason extends Schema.Class<InvalidProviderOut
route: Schema.optional(Schema.String),
raw: Schema.optional(Schema.String),
providerMetadata: Schema.optional(ProviderMetadata),
}) {
get retryable() {
return false
}
}
}) {}
export class UnknownProviderReason extends Schema.Class<UnknownProviderReason>("LLM.Error.UnknownProvider")({
_tag: Schema.tag("UnknownProvider"),
@ -151,11 +115,7 @@ export class UnknownProviderReason extends Schema.Class<UnknownProviderReason>("
status: Schema.optional(Schema.Number),
providerMetadata: Schema.optional(ProviderMetadata),
http: Schema.optional(HttpContext),
}) {
get retryable() {
return false
}
}
}) {}
export const LLMErrorReason = Schema.Union([
InvalidRequestReason,
@ -178,14 +138,6 @@ export class LLMError extends Schema.TaggedErrorClass<LLMError>()("LLM.Error", {
}) {
override readonly cause = this.reason
get retryable() {
return this.reason.retryable
}
get retryAfterMs() {
return "retryAfterMs" in this.reason ? this.reason.retryAfterMs : undefined
}
override get message() {
return `${this.module}.${this.method}: ${this.reason.message}`
}

View file

@ -201,7 +201,6 @@ export const ProviderErrorEvent = Schema.Struct({
type: Schema.tag("provider-error"),
message: Schema.String,
classification: Schema.optional(ProviderFailureClassification),
retryable: Schema.optional(Schema.Boolean),
providerMetadata: Schema.optional(ProviderMetadata),
}).annotate({ identifier: "LLM.Event.ProviderError" })
export type ProviderErrorEvent = Schema.Schema.Type<typeof ProviderErrorEvent>

View file

@ -1,6 +1,5 @@
import { describe, expect } from "bun:test"
import { Effect, Fiber, Layer, Random, Ref } from "effect"
import * as TestClock from "effect/testing/TestClock"
import { Effect, Layer, Ref } from "effect"
import { Headers, HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http"
import { LLM, LLMError } from "../src"
import { LLMClient, RequestExecutor } from "../src/route"
@ -59,11 +58,6 @@ const countedResponsesLayer = (attempts: Ref.Ref<number>, responses: ReadonlyArr
),
)
const randomMidpoint = {
nextDoubleUnsafe: () => 0.5,
nextIntUnsafe: () => 0,
}
const expectLLMError = (error: unknown) => {
expect(error).toBeInstanceOf(LLMError)
if (!(error instanceof LLMError)) throw new Error("expected LLMError")
@ -113,17 +107,16 @@ describe("RequestExecutor", () => {
}).pipe(Effect.provide(responsesLayer([new Response("invalid parameter", { status: 400 })]))),
)
it.effect("returns redacted diagnostics for retryable rate limits", () =>
it.effect("returns redacted diagnostics for rate limits", () =>
Effect.gen(function* () {
const executor = yield* RequestExecutor.Service
const error = yield* executor.execute(request).pipe(Effect.flip)
expectLLMError(error)
expect(error).toMatchObject({
retryable: true,
retryAfterMs: 0,
reason: {
_tag: "RateLimit",
retryAfterMs: 0,
rateLimit: { retryAfterMs: 0 },
http: {
requestId: "req_123",
@ -146,16 +139,12 @@ describe("RequestExecutor", () => {
expect(errorHttp(error)?.body).toBe("rate limited")
}).pipe(
Effect.provide(
responsesLayer(
Array.from(
{ length: 3 },
() =>
new Response("rate limited", {
status: 429,
headers: { "retry-after-ms": "0", "x-request-id": "req_123", "x-api-key": "secret" },
}),
),
),
responsesLayer([
new Response("rate limited", {
status: 429,
headers: { "retry-after-ms": "0", "x-request-id": "req_123", "x-api-key": "secret" },
}),
]),
),
),
)
@ -189,24 +178,20 @@ describe("RequestExecutor", () => {
})
}).pipe(
Effect.provide(
responsesLayer(
Array.from(
{ length: 3 },
() =>
new Response("rate limited", {
status: 429,
headers: {
"retry-after-ms": "0",
"x-ratelimit-limit-requests": "500",
"x-ratelimit-limit-tokens": "30000",
"x-ratelimit-remaining-requests": "499",
"x-ratelimit-remaining-tokens": "29900",
"x-ratelimit-reset-requests": "1s",
"x-ratelimit-reset-tokens": "10s",
},
}),
),
),
responsesLayer([
new Response("rate limited", {
status: 429,
headers: {
"retry-after-ms": "0",
"x-ratelimit-limit-requests": "500",
"x-ratelimit-limit-tokens": "30000",
"x-ratelimit-remaining-requests": "499",
"x-ratelimit-remaining-tokens": "29900",
"x-ratelimit-reset-requests": "1s",
"x-ratelimit-reset-tokens": "10s",
},
}),
]),
),
),
)
@ -224,48 +209,48 @@ describe("RequestExecutor", () => {
remaining: { requests: "12", "input-tokens": "9000" },
reset: { requests: "2026-05-06T12:00:00Z", "input-tokens": "2026-05-06T12:00:10Z" },
})
}).pipe(
Effect.provide(
responsesLayer(
Array.from(
{ length: 3 },
() =>
new Response("overloaded", {
status: 529,
headers: {
"retry-after-ms": "0",
"anthropic-ratelimit-requests-limit": "100",
"anthropic-ratelimit-requests-remaining": "12",
"anthropic-ratelimit-requests-reset": "2026-05-06T12:00:00Z",
"anthropic-ratelimit-input-tokens-limit": "10000",
"anthropic-ratelimit-input-tokens-remaining": "9000",
"anthropic-ratelimit-input-tokens-reset": "2026-05-06T12:00:10Z",
},
}),
),
),
),
),
)
it.effect("retries retryable status responses before returning the stream", () =>
Effect.gen(function* () {
const executor = yield* RequestExecutor.Service
const response = yield* executor.execute(request)
expect(response.status).toBe(200)
expect(yield* response.text).toBe("ok")
}).pipe(
Effect.provide(
responsesLayer([
new Response("busy", { status: 503, headers: { "retry-after-ms": "0" } }),
new Response("ok", { status: 200 }),
new Response("overloaded", {
status: 529,
headers: {
"retry-after-ms": "0",
"anthropic-ratelimit-requests-limit": "100",
"anthropic-ratelimit-requests-remaining": "12",
"anthropic-ratelimit-requests-reset": "2026-05-06T12:00:00Z",
"anthropic-ratelimit-input-tokens-limit": "10000",
"anthropic-ratelimit-input-tokens-remaining": "9000",
"anthropic-ratelimit-input-tokens-reset": "2026-05-06T12:00:10Z",
},
}),
]),
),
),
)
it.effect("marks 504 and 529 status responses retryable", () =>
it.effect("returns provider status failures without retrying", () =>
Effect.gen(function* () {
const attempts = yield* Ref.make(0)
const error = yield* Effect.gen(function* () {
const executor = yield* RequestExecutor.Service
return yield* executor.execute(request).pipe(Effect.flip)
}).pipe(
Effect.provide(
countedResponsesLayer(attempts, [
new Response("busy", { status: 503, headers: { "retry-after-ms": "0" } }),
new Response("ok", { status: 200 }),
]),
),
)
expectLLMError(error)
expect(error.reason).toMatchObject({ _tag: "ProviderInternal", status: 503 })
expect(yield* Ref.get(attempts)).toBe(1)
}),
)
it.effect("marks 504 and 529 status responses as provider-internal", () =>
Effect.gen(function* () {
const failWith = (status: number) =>
Effect.gen(function* () {
@ -274,19 +259,14 @@ describe("RequestExecutor", () => {
expectLLMError(error)
expect(error.reason).toMatchObject({ _tag: "ProviderInternal", status })
expect(error.retryable).toBe(true)
}).pipe(
Effect.provide(
responsesLayer(
Array.from(
{ length: 3 },
() =>
new Response("retry", {
status,
headers: { "retry-after-ms": "0" },
}),
),
),
responsesLayer([
new Response("provider failure", {
status,
headers: { "retry-after-ms": "0" },
}),
]),
),
)
@ -295,14 +275,13 @@ describe("RequestExecutor", () => {
}),
)
it.effect("does not retry non-retryable status responses and truncates large bodies", () =>
it.effect("truncates large authentication error bodies", () =>
Effect.gen(function* () {
const executor = yield* RequestExecutor.Service
const error = yield* executor.execute(request).pipe(Effect.flip)
expectLLMError(error)
expect(error.reason).toMatchObject({ _tag: "Authentication" })
expect(error.retryable).toBe(false)
expect(errorHttp(error)?.bodyTruncated).toBe(true)
expect(errorHttp(error)?.body).toHaveLength(16_384)
}).pipe(
@ -355,77 +334,7 @@ describe("RequestExecutor", () => {
),
)
it.effect("honors Retry-After delta seconds before retrying", () =>
Effect.gen(function* () {
const attempts = yield* Ref.make(0)
return yield* Effect.gen(function* () {
const executor = yield* RequestExecutor.Service
const fiber = yield* executor.execute(request).pipe(Effect.forkChild)
yield* Effect.yieldNow
expect(yield* Ref.get(attempts)).toBe(1)
yield* TestClock.adjust(1_999)
yield* Effect.yieldNow
expect(yield* Ref.get(attempts)).toBe(1)
yield* TestClock.adjust(1)
const response = yield* Fiber.join(fiber)
expect(response.status).toBe(200)
expect(yield* Ref.get(attempts)).toBe(2)
}).pipe(
Effect.provide(
countedResponsesLayer(attempts, [
new Response("busy", { status: 503, headers: { "retry-after": "2" } }),
new Response("ok", { status: 200 }),
]),
),
)
}),
)
it.effect("uses exponential jittered delay when retry-after is absent", () =>
Effect.gen(function* () {
const attempts = yield* Ref.make(0)
return yield* Effect.gen(function* () {
const executor = yield* RequestExecutor.Service
const fiber = yield* executor.execute(request).pipe(Effect.flip, Effect.forkChild)
yield* Effect.yieldNow
expect(yield* Ref.get(attempts)).toBe(1)
yield* TestClock.adjust(499)
yield* Effect.yieldNow
expect(yield* Ref.get(attempts)).toBe(1)
yield* TestClock.adjust(1)
yield* Effect.yieldNow
expect(yield* Ref.get(attempts)).toBe(2)
yield* TestClock.adjust(999)
yield* Effect.yieldNow
expect(yield* Ref.get(attempts)).toBe(2)
yield* TestClock.adjust(1)
const error = yield* Fiber.join(fiber)
expectLLMError(error)
expect(error.reason).toMatchObject({ _tag: "ProviderInternal" })
expect(yield* Ref.get(attempts)).toBe(3)
}).pipe(
Effect.provide(
countedResponsesLayer(attempts, [
new Response("busy", { status: 503 }),
new Response("still busy", { status: 503 }),
new Response("done retrying", { status: 503 }),
]),
),
)
}).pipe(Effect.provideService(Random.Random, randomMidpoint)),
)
it.effect("does not retry after a successful response reaches stream parsing", () =>
it.effect("does not re-execute after a successful response reaches stream parsing", () =>
Effect.gen(function* () {
const attempts = yield* Ref.make(0)
const model = OpenAIChat.route

View file

@ -366,7 +366,6 @@ describe("Bedrock Converse route", () => {
expect(response.events.find((event) => event.type === "provider-error")).toEqual({
type: "provider-error",
message: "Slow down",
retryable: true,
})
}),
)
@ -383,7 +382,6 @@ describe("Bedrock Converse route", () => {
type: "provider-error",
message: "Input is too long for requested model",
classification: "context-overflow",
retryable: false,
})
}),
)