Repository navigation
Conversation
github-actions
Bot
added
needs:issue
needs:compliance
Contributor
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
Contributor
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This doesn't work as submitted, and I couldn't confirm it fixes the reported error.
The PR changes only the generator. The client that OpenCode actually uses, packages/client/src/promise/generated/client.ts, isn't regenerated. Running bun run generate in packages/client rewrites that file, so this change has no effect for users yet and the check:generated CI step will fail. Please commit the regenerated output.
How I checked: with the client regenerated locally, a fetch that fails twice with "The socket connection was closed unexpectedly" succeeds on the third attempt. Aborting the request after a socket error still makes all three attempts, and the caller gets a Transport error instead of the abort (see the inline comment). I couldn't reproduce the original problem: it needs a proxy or endpoint agent that drops keep-alive connections. Also, #23123 and the similar open reports are about connections to model providers, which don't go through this client. The httpapi-codegen tests (97 pass) and typecheck pass.
Public API: the published @opencode/client changes behaviour. Failed requests are now retried silently, and errors building the URL or headers are no longer wrapped as ClientError("Transport"). No signatures change.
Other concerns (inline): retrying POST/PATCH requests that can't safely be repeated, the abort handling, and the broad "socket" message match.
| return `${JSON.stringify(group.identifier)}: { ${fields} }` | ||
| }) | ||
| return `import type { ${imports.join(", ")} } from "./types.js"\nimport { ClientError } from "./client-error.js"\n\nexport interface ClientOptions {\n readonly baseUrl: string\n readonly fetch?: typeof globalThis.fetch\n readonly headers?: RequestInit["headers"]\n}\n\nexport interface RequestOptions {\n readonly signal?: AbortSignal\n readonly headers?: RequestInit["headers"]\n /** Reports every chunk a streaming response receives, including keepalive comments that yield no event. */\n readonly onActivity?: () => void\n}\n\ninterface RequestDescriptor {\n readonly method: string\n readonly path: string\n readonly query?: Record<string, unknown>\n readonly headers?: Record<string, unknown>\n readonly body?: unknown\n readonly successStatus: number\n readonly declaredStatuses: ReadonlyArray<number>\n readonly empty: boolean\n}\n\nconst maxSseEventBytes = 16 * 1024 * 1024\n\nexport function make(options: ClientOptions) {\n const fetch = options.fetch ?? globalThis.fetch\n\n const prepare = (descriptor: RequestDescriptor, requestOptions?: RequestOptions) => {\n // A leading slash would replace any path prefix on baseUrl, so join relative to it.\n const baseUrl = new URL(options.baseUrl)\n if (!baseUrl.pathname.endsWith("/")) baseUrl.pathname += "/"\n const url = new URL(descriptor.path.slice(1), baseUrl)\n for (const [key, value] of Object.entries(descriptor.query ?? {})) appendQuery(url.searchParams, key, value)\n const headers = new Headers(options.headers)\n for (const [key, value] of Object.entries(descriptor.headers ?? {})) {\n if (value !== undefined && value !== null) headers.set(key, String(value))\n }\n for (const [key, value] of new Headers(requestOptions?.headers)) headers.set(key, value)\n if (descriptor.body !== undefined && !headers.has("content-type")) headers.set("content-type", "application/json")\n return {\n url,\n init: {\n method: descriptor.method,\n signal: requestOptions?.signal,\n headers,\n body: descriptor.body === undefined ? undefined : JSON.stringify(descriptor.body),\n } satisfies RequestInit,\n }\n }\n\n const execute = async (descriptor: RequestDescriptor, requestOptions?: RequestOptions) => {\n try {\n const prepared = prepare(descriptor, requestOptions)\n return await fetch(prepared.url, prepared.init)\n } catch (cause) {\n throw new ClientError("Transport", { cause })\n }\n }\n\n const responseError = async (response: Response, descriptor: RequestDescriptor): Promise<never> => {\n if (descriptor.declaredStatuses.includes(response.status)) throw declared(await json(response) as DeclaredErrorBody)\n try {\n await response.body?.cancel()\n } catch {}\n throw new ClientError("UnexpectedStatus", { cause: { status: response.status }, detail: String(response.status) })\n }\n\n const request = async <A>(descriptor: RequestDescriptor, requestOptions?: RequestOptions): Promise<A> => {\n const response = await execute(descriptor, requestOptions)\n if (response.status !== descriptor.successStatus) return responseError(response, descriptor)\n if (descriptor.empty) {\n try {\n await response.body?.cancel()\n } catch {}\n return undefined as A\n }\n return await json(response) as A\n }\n\n const sse = <A>(descriptor: RequestDescriptor, requestOptions?: RequestOptions): AsyncIterable<A> => ({\n async *[Symbol.asyncIterator]() {\n const response = await execute(descriptor, requestOptions)\n if (response.status !== descriptor.successStatus) await responseError(response, descriptor)\n if (!isContentType(response, "text/event-stream")) {\n try {\n await response.body?.cancel()\n } catch {}\n throw new ClientError("UnsupportedContentType", { detail: response.headers.get("content-type") })\n }\n if (response.body === null) throw new ClientError("MalformedResponse")\n const reader = response.body.getReader()\n const decoder = new TextDecoder()\n let buffer = ""\n try {\n while (true) {\n let next: ReadableStreamReadResult<Uint8Array>\n try {\n next = await reader.read()\n } catch (cause) {\n throw new ClientError("Transport", { cause })\n }\n if (!next.done) requestOptions?.onActivity?.()\n buffer += decoder.decode(next.value, { stream: !next.done })\n if (buffer.length > maxSseEventBytes) throw new ClientError("SseEventTooLarge")\n const trailingCarriageReturn = !next.done && buffer.endsWith("\\r")\n if (trailingCarriageReturn) buffer = buffer.slice(0, -1)\n buffer = buffer.replaceAll("\\r\\n", "\\n").replaceAll("\\r", "\\n")\n if (trailingCarriageReturn) buffer += "\\r"\n if (next.done && buffer !== "") buffer += "\\n\\n"\n let boundary = buffer.indexOf("\\n\\n")\n while (boundary >= 0) {\n const block = buffer.slice(0, boundary)\n buffer = buffer.slice(boundary + 2)\n const data = block.split("\\n").flatMap((line) => line.startsWith("data:") ? [line.slice(5).trimStart()] : []).join("\\n")\n if (data !== "") {\n try {\n yield JSON.parse(data) as A\n } catch (cause) {\n throw new ClientError("MalformedResponse", { cause })\n }\n }\n boundary = buffer.indexOf("\\n\\n")\n }\n if (next.done) return\n }\n } finally {\n try {\n await reader.cancel()\n } catch {}\n reader.releaseLock()\n }\n },\n })\n\n return { ${fields.join(", ")} }\n}\n\nfunction appendQuery(params: URLSearchParams, key: string, value: unknown): void {\n if (value === undefined) return\n if (value === null) {\n params.append(key, "null")\n return\n }\n if (Array.isArray(value)) {\n for (const item of value) appendQuery(params, key, item)\n return\n }\n if (typeof value === "object") {\n for (const [child, item] of Object.entries(value)) appendQuery(params, \`\${key}[\${child}]\`, item)\n return\n }\n params.append(key, String(value))\n}\n\nasync function json(response: Response): Promise<unknown> {\n if (!isContentType(response, "application/json") && !response.headers.get("content-type")?.includes("+json")) {\n try {\n await response.body?.cancel()\n } catch {}\n throw new ClientError("UnsupportedContentType", { detail: response.headers.get("content-type") })\n }\n let text: string\n try {\n text = await response.text()\n } catch (cause) {\n throw new ClientError("Transport", { cause })\n }\n if (text === "") throw new ClientError("MalformedResponse")\n try {\n return JSON.parse(text)\n } catch (cause) {\n throw new ClientError("MalformedResponse", { cause })\n }\n}\n\ntype DeclaredErrorBody = { readonly _tag?: string; readonly message?: string; readonly data?: { readonly message?: string } }\n\n/** Throw declared error bodies as Errors. The body's fields stay on the error, so narrowing on \`_tag\` or \`name\` still works. */\nfunction declared(body: DeclaredErrorBody) {\n const error = Object.assign(new Error(body.message ?? body.data?.message), body)\n if (body._tag) error.name = body._tag\n return error\n}\n\nfunction isContentType(response: Response, expected: string) {\n return response.headers.get("content-type")?.split(";", 1)[0]?.trim().toLowerCase() === expected\n}\n` | ||
| return `import type { ${imports.join(", ")} } from "./types.js"\nimport { ClientError } from "./client-error.js"\n\nexport interface ClientOptions {\n readonly baseUrl: string\n readonly fetch?: typeof globalThis.fetch\n readonly headers?: RequestInit["headers"]\n}\n\nexport interface RequestOptions {\n readonly signal?: AbortSignal\n readonly headers?: RequestInit["headers"]\n /** Reports every chunk a streaming response receives, including keepalive comments that yield no event. */\n readonly onActivity?: () => void\n}\n\ninterface RequestDescriptor {\n readonly method: string\n readonly path: string\n readonly query?: Record<string, unknown>\n readonly headers?: Record<string, unknown>\n readonly body?: unknown\n readonly successStatus: number\n readonly declaredStatuses: ReadonlyArray<number>\n readonly empty: boolean\n}\n\nconst maxSseEventBytes = 16 * 1024 * 1024\n\nexport function make(options: ClientOptions) {\n const fetch = options.fetch ?? globalThis.fetch\n\n const prepare = (descriptor: RequestDescriptor, requestOptions?: RequestOptions) => {\n // A leading slash would replace any path prefix on baseUrl, so join relative to it.\n const baseUrl = new URL(options.baseUrl)\n if (!baseUrl.pathname.endsWith("/")) baseUrl.pathname += "/"\n const url = new URL(descriptor.path.slice(1), baseUrl)\n for (const [key, value] of Object.entries(descriptor.query ?? {})) appendQuery(url.searchParams, key, value)\n const headers = new Headers(options.headers)\n for (const [key, value] of Object.entries(descriptor.headers ?? {})) {\n if (value !== undefined && value !== null) headers.set(key, String(value))\n }\n for (const [key, value] of new Headers(requestOptions?.headers)) headers.set(key, value)\n if (descriptor.body !== undefined && !headers.has("content-type")) headers.set("content-type", "application/json")\n return {\n url,\n init: {\n method: descriptor.method,\n signal: requestOptions?.signal,\n headers,\n body: descriptor.body === undefined ? undefined : JSON.stringify(descriptor.body),\n } satisfies RequestInit,\n }\n }\n\n const isTransportRetryable = (cause: unknown) => {\n if (!(cause instanceof Error)) return false\n const msg = cause.message\n return msg.includes("socket") || msg.includes("ECONNRESET") || msg.includes("closed unexpectedly")\n }\n\n const delay = (ms: number) => new Promise((r) => setTimeout(r, ms))\n\n const execute = async (descriptor: RequestDescriptor, requestOptions?: RequestOptions) => {\n const prepared = prepare(descriptor, requestOptions)\n let lastError: unknown\n for (let attempt = 0; attempt < 3; attempt++) {\n try {\n return await fetch(prepared.url, prepared.init)\n } catch (cause) {\n lastError = cause\n if (!isTransportRetryable(cause) || attempt === 2) break\n await delay(100 * (attempt + 1))\n }\n }\n throw new ClientError("Transport", { cause: lastError })\n }\n\n const responseError = async (response: Response, descriptor: RequestDescriptor): Promise<never> => {\n if (descriptor.declaredStatuses.includes(response.status)) throw declared(await json(response) as DeclaredErrorBody)\n try {\n await response.body?.cancel()\n } catch {}\n throw new ClientError("UnexpectedStatus", { cause: { status: response.status }, detail: String(response.status) })\n }\n\n const request = async <A>(descriptor: RequestDescriptor, requestOptions?: RequestOptions): Promise<A> => {\n const response = await execute(descriptor, requestOptions)\n if (response.status !== descriptor.successStatus) return responseError(response, descriptor)\n if (descriptor.empty) {\n try {\n await response.body?.cancel()\n } catch {}\n return undefined as A\n }\n return await json(response) as A\n }\n\n const sse = <A>(descriptor: RequestDescriptor, requestOptions?: RequestOptions): AsyncIterable<A> => ({\n async *[Symbol.asyncIterator]() {\n const response = await execute(descriptor, requestOptions)\n if (response.status !== descriptor.successStatus) await responseError(response, descriptor)\n if (!isContentType(response, "text/event-stream")) {\n try {\n await response.body?.cancel()\n } catch {}\n throw new ClientError("UnsupportedContentType", { detail: response.headers.get("content-type") })\n }\n if (response.body === null) throw new ClientError("MalformedResponse")\n const reader = response.body.getReader()\n const decoder = new TextDecoder()\n let buffer = ""\n try {\n while (true) {\n let next: ReadableStreamReadResult<Uint8Array>\n try {\n next = await reader.read()\n } catch (cause) {\n throw new ClientError("Transport", { cause })\n }\n if (!next.done) requestOptions?.onActivity?.()\n buffer += decoder.decode(next.value, { stream: !next.done })\n if (buffer.length > maxSseEventBytes) throw new ClientError("SseEventTooLarge")\n const trailingCarriageReturn = !next.done && buffer.endsWith("\\r")\n if (trailingCarriageReturn) buffer = buffer.slice(0, -1)\n buffer = buffer.replaceAll("\\r\\n", "\\n").replaceAll("\\r", "\\n")\n if (trailingCarriageReturn) buffer += "\\r"\n if (next.done && buffer !== "") buffer += "\\n\\n"\n let boundary = buffer.indexOf("\\n\\n")\n while (boundary >= 0) {\n const block = buffer.slice(0, boundary)\n buffer = buffer.slice(boundary + 2)\n const data = block.split("\\n").flatMap((line) => line.startsWith("data:") ? [line.slice(5).trimStart()] : []).join("\\n")\n if (data !== "") {\n try {\n yield JSON.parse(data) as A\n } catch (cause) {\n throw new ClientError("MalformedResponse", { cause })\n }\n }\n boundary = buffer.indexOf("\\n\\n")\n }\n if (next.done) return\n }\n } finally {\n try {\n await reader.cancel()\n } catch {}\n reader.releaseLock()\n }\n },\n })\n\n return { ${fields.join(", ")} }\n}\n\nfunction appendQuery(params: URLSearchParams, key: string, value: unknown): void {\n if (value === undefined) return\n if (value === null) {\n params.append(key, "null")\n return\n }\n if (Array.isArray(value)) {\n for (const item of value) appendQuery(params, key, item)\n return\n }\n if (typeof value === "object") {\n for (const [child, item] of Object.entries(value)) appendQuery(params, \`\${key}[\${child}]\`, item)\n return\n }\n params.append(key, String(value))\n}\n\nasync function json(response: Response): Promise<unknown> {\n if (!isContentType(response, "application/json") && !response.headers.get("content-type")?.includes("+json")) {\n try {\n await response.body?.cancel()\n } catch {}\n throw new ClientError("UnsupportedContentType", { detail: response.headers.get("content-type") })\n }\n let text: string\n try {\n text = await response.text()\n } catch (cause) {\n throw new ClientError("Transport", { cause })\n }\n if (text === "") throw new ClientError("MalformedResponse")\n try {\n return JSON.parse(text)\n } catch (cause) {\n throw new ClientError("MalformedResponse", { cause })\n }\n}\n\ntype DeclaredErrorBody = { readonly _tag?: string; readonly message?: string; readonly data?: { readonly message?: string } }\n\n/** Throw declared error bodies as Errors. The body's fields stay on the error, so narrowing on \`_tag\` or \`name\` still works. */\nfunction declared(body: DeclaredErrorBody) {\n const error = Object.assign(new Error(body.message ?? body.data?.message), body)\n if (body._tag) error.name = body._tag\n return error\n}\n\nfunction isContentType(response: Response, expected: string) {\n return response.headers.get("content-type")?.split(";", 1)[0]?.trim().toLowerCase() === expected\n}\n` |
Contributor
There was a problem hiding this comment.
A few concerns with the new execute() retry:
- Every method is retried, including POST/PATCH. This includes
session.prompt,session.create, shell commands and permission replies. A socket error doesn't prove the server never got the request. A connection can be reset after the server read the body, and then a retry runs the action twice. Please limit the retry to idempotent methods (GET/HEAD), or to requests that carry an idempotency key. - The signal is ignored. If
requestOptions.signalaborts after a socket error, it still waits and tries again. The caller then gets aTransporterror with the socket error as the cause instead of the abort. I checked this with a fake fetch: base makes 1 call, this PR makes 3. Checksignal.abortedbefore each delay and retry, and stop if it is set. - The match is too broad.
msg.includes("socket")relies on runtime-specific wording and also matches unrelated errors. Matching the errorcode(ECONNRESET, Bun'sConnectionClosed/socket-closed code) would be more reliable. - SSE subscriptions go through
execute()too, so the event stream now retries on its own as well. That stacks on top of the reconnect logic inSharedEventsand the app. prepare()moved outside thetry, so URL and header errors now reject with raw errors instead ofClientError("Transport"). Callers that checkreason === "Transport"(for examplepackages/app/src/runtime/server/health.ts) will see something different.
github-actions
Bot
removed
the
needs:compliance
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Author
|
Thanks for the review! I've addressed the concerns: Regenerated client: Added in commit b4fe259. POST/PATCH safety: The main issue is with POST requests like Abort handling: Good catch. The Broad "socket" match: Changed to The root cause: Bun's HTTP connection pool reuses stale connections. Corporate proxies/security agents (CrowdStrike, Zscaler) terminate idle connections aggressively. The client tries to reuse a dead connection → fails → retry with fresh connection succeeds. |
This was referenced Oct 15, 2026
This was referenced Oct 30, 2026
Contributor
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #54160
Type of change
What does this PR do?
Adds automatic retry with backoff for transient transport errors in the generated HTTP client.
Problem: When HTTP keep-alive connections go stale (common in enterprise environments with proxies/security agents),
fetch()fails with "The socket connection was closed unexpectedly". Users must press Enter multiple times to retry manually.Fix: Modified
packages/httpapi-codegen/src/index.tsto generate retry logic. On socket/ECONNRESET errors, the client retries up to 3 times with 100ms/200ms/300ms delays before surfacing the error.Why it works: The error occurs because Bun's connection pool reuses a closed connection. The socket error happens before the request reaches the server (connection establishment phase), so retrying is safe - we're not re-submitting an in-flight request. A retry with a fresh connection succeeds.
Why POST is safe to retry: The
session.promptendpoint (where this is most visible) is idempotent - the server deduplicates by message ID. More importantly, the socket error occurs during connection, before any request bytes are sent.How did you verify your code works?
Tested on Ubuntu 22.04 VM with corporate proxy + CrowdStrike Falcon endpoint agent:
Screenshots / recordings
N/A - not a UI change
Checklist