diff --git a/packages/llm/src/protocols/bedrock-converse.ts b/packages/llm/src/protocols/bedrock-converse.ts index c447a1a39d..0c6b0598ff 100644 --- a/packages/llm/src/protocols/bedrock-converse.ts +++ b/packages/llm/src/protocols/bedrock-converse.ts @@ -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 diff --git a/packages/llm/src/route/executor.ts b/packages/llm/src/route/executor.ts index b2f679c683..0f7a949168 100644 --- a/packages/llm/src/route/executor.ts +++ b/packages/llm/src/route/executor.ts @@ -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()("@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 = "" // One source of truth for what counts as a sensitive name across headers, @@ -88,7 +85,7 @@ const requestId = (headers: Record) => { ) } -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) => { 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) => (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 = ( - effect: Effect.Effect, - retries = MAX_RETRIES, - attempt = 0, -): Effect.Effect => - Effect.catchTag(effect, "LLM.Error", (error): Effect.Effect => { - 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 = Layer.effect( Service, Effect.gen(function* () { @@ -375,7 +351,7 @@ export const layer: Layer.Layer = Layer.e .pipe(Effect.mapError(toHttpError(redactedNames)), Effect.flatMap(statusError(request, redactedNames))) }) return Service.of({ - execute: (request) => retryStatusFailures(executeOnce(request)), + execute: executeOnce, }) }), ) diff --git a/packages/llm/src/schema/errors.ts b/packages/llm/src/schema/errors.ts index 072e4e8389..3592ea729f 100644 --- a/packages/llm/src/schema/errors.ts +++ b/packages/llm/src/schema/errors.ts @@ -38,11 +38,7 @@ export class InvalidRequestReason extends Schema.Class("LL classification: Schema.optional(ProviderFailureClassification), providerMetadata: Schema.optional(ProviderMetadata), http: Schema.optional(HttpContext), -}) { - get retryable() { - return false - } -} +}) {} export class NoRouteReason extends Schema.Class("LLM.Error.NoRoute")({ _tag: Schema.tag("NoRoute"), @@ -50,10 +46,6 @@ export class NoRouteReason extends Schema.Class("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("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("LLM.Error.RateLimit")({ _tag: Schema.tag("RateLimit"), @@ -78,33 +66,21 @@ export class RateLimitReason extends Schema.Class("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("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("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("LLM.Error.ProviderInternal")({ _tag: Schema.tag("ProviderInternal"), @@ -113,11 +89,7 @@ export class ProviderInternalReason extends Schema.Class retryAfterMs: Schema.optional(Schema.Number), providerMetadata: Schema.optional(ProviderMetadata), http: Schema.optional(HttpContext), -}) { - get retryable() { - return true - } -} +}) {} export class TransportReason extends Schema.Class("LLM.Error.Transport")({ _tag: Schema.tag("Transport"), @@ -125,11 +97,7 @@ export class TransportReason extends Schema.Class("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( "LLM.Error.InvalidProviderOutput", @@ -139,11 +107,7 @@ export class InvalidProviderOutputReason extends Schema.Class("LLM.Error.UnknownProvider")({ _tag: Schema.tag("UnknownProvider"), @@ -151,11 +115,7 @@ export class UnknownProviderReason extends Schema.Class(" 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()("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}` } diff --git a/packages/llm/src/schema/events.ts b/packages/llm/src/schema/events.ts index 98fcc9a24d..f43251f670 100644 --- a/packages/llm/src/schema/events.ts +++ b/packages/llm/src/schema/events.ts @@ -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 diff --git a/packages/llm/test/executor.test.ts b/packages/llm/test/executor.test.ts index 811f7a9ffe..1227dbbf96 100644 --- a/packages/llm/test/executor.test.ts +++ b/packages/llm/test/executor.test.ts @@ -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, 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 diff --git a/packages/llm/test/provider/bedrock-converse.test.ts b/packages/llm/test/provider/bedrock-converse.test.ts index 46657331a3..87186949ae 100644 --- a/packages/llm/test/provider/bedrock-converse.test.ts +++ b/packages/llm/test/provider/bedrock-converse.test.ts @@ -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, }) }), )