Repository navigation
fix(connect): bound the catalog download by inactivity, not total duration #3310
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { MAX_REMOTE_CATALOG_BYTES } from "../server/catalog-download"; | ||
| import { readBoundedResponseBytes } from "../lib/bounded-body"; | ||
| import { clearableDeadline } from "../lib/abort"; | ||
|
|
||
| /** | ||
| * A pairing grant may cross loopback or authenticated HTTPS, and nothing else. | ||
|
|
@@ -84,29 +85,42 @@ async function fetchBounded( | |
| url: string, | ||
| init: RequestInit, | ||
| timeoutMs: number | undefined, | ||
| timeoutScope: "request" | "headers" = "request", | ||
| ): Promise<Response> { | ||
| const timeout = safeTimeout(timeoutMs); | ||
| const headerDeadline = timeoutScope === "headers" ? clearableDeadline(timeout) : null; | ||
| try { | ||
| const response = await fetchImpl(url, { | ||
| ...init, | ||
| redirect: "manual", | ||
| signal: headerDeadline?.signal ?? AbortSignal.timeout(timeout), | ||
| }); | ||
| headerDeadline?.clear(); | ||
| if (response.status >= 300 && response.status < 400 && response.status !== 304) { | ||
| throw new HubClientError("redirect_refused", "Hub request redirect was refused", response.status); | ||
| } | ||
| return response; | ||
| } catch (error) { | ||
| if (error instanceof HubClientError) throw error; | ||
| throw new HubClientError("unreachable", "Hub request did not complete", undefined, { cause: error }); | ||
| } finally { | ||
| headerDeadline?.clear(); | ||
| } | ||
| } | ||
|
|
||
| async function boundedText( | ||
| response: Response, | ||
| maxBytes: number, | ||
| options: { inactivityTimeoutMs?: number } = {}, | ||
| ): Promise<string> { | ||
| const declared = Number(response.headers.get("content-length") ?? "0"); | ||
| if (Number.isFinite(declared) && declared > maxBytes) { | ||
| throw new HubClientError("body_too_large", "Hub response exceeded the allowed size", response.status); | ||
| } | ||
| const result = await readBoundedResponseBytes(response, { | ||
| maxBytes, | ||
| ...(options.inactivityTimeoutMs === undefined ? {} : { inactivityTimeoutMs: options.inactivityTimeoutMs }), | ||
| }); | ||
| if (result.oversized) { | ||
| throw new HubClientError("body_too_large", "Hub response exceeded the allowed size", response.status); | ||
| } | ||
|
|
@@ -422,7 +436,7 @@ export async function downloadClientCatalog( | |
| const response = await fetchBounded(options.fetchImpl ?? fetch, `${origin}/v1/catalog`, { | ||
| method: "GET", | ||
| headers, | ||
| }, options.timeoutMs, "headers"); | ||
|
There was a problem hiding this comment.
When AGENTS.md reference: src/AGENTS.md:L15-L17 Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,220p' /tmp/coderabbit-repo-knowledge/lidge-jun-opencodex-7afea732/*/*.md
printf '\n--- target outline ---\n'
ast-grep outline src/client/hub-client.ts
printf '\n--- target implementation ---\n'
sed -n '60,125p' src/client/hub-client.ts
sed -n '380,465p' src/client/hub-client.tsRepository: lidge-jun/opencodex Length of output: 26288 🏁 Script executed: printf '%s\n' '--- imports and deadline/reader definitions ---'
sed -n '1,75p' src/client/hub-client.ts
rg -n --glob '*.ts' 'function clearableDeadline|clearableDeadline|function readBoundedResponseBytes|readBoundedResponseBytes' src tests
printf '%s\n' '--- catalog-related tests ---'
rg -n -C 4 --glob '*.ts' 'downloadClientCatalog|catalog_unexpected_304|catalog_http_|catalog_content_type_invalid' tests srcRepository: lidge-jun/opencodex Length of output: 21460 🌐 Web query:
💡 Result: In Bun, handling unconsumed fetch response bodies and managing their associated connection resources is a critical area that has undergone significant optimization and fixes. By default, simply discarding a Response object without consuming its body or explicitly canceling it can lead to memory leaks and connection resource retention [1][2]. Bun's architecture uses reference counting for these resources; if a response body is not consumed, the underlying connection may be held open for potential reuse or because the system is waiting for the body to be drained [3][4]. Key practices for managing response body cleanup in Bun: 1. Explicit Cancellation: To immediately stop receiving data and release connection resources, you should call Citations:
Cancel catalog error response bodies after the header deadline is cleared. At 🤖 Prompt for AI Agents |
||
| if (response.status === 304) { | ||
| throw new HubClientError("catalog_unexpected_304", "Hub answered 304 to an unconditional catalog request", 304); | ||
| } | ||
|
|
@@ -434,7 +448,17 @@ export async function downloadClientCatalog( | |
| try { await response.body?.cancel(); } catch { /* best effort */ } | ||
| throw new HubClientError("catalog_content_type_invalid", "Hub catalog response was not JSON", response.status); | ||
| } | ||
| let body: string; | ||
| try { | ||
| body = await boundedText(response, options.maxBytes ?? MAX_REMOTE_CATALOG_BYTES, { | ||
| inactivityTimeoutMs: safeTimeout(options.timeoutMs), | ||
| }); | ||
| } catch (error) { | ||
| if (error instanceof DOMException && error.name === "TimeoutError") { | ||
| throw new HubClientError("unreachable", "Hub catalog download stalled", undefined, { cause: error }); | ||
| } | ||
| throw error; | ||
| } | ||
| const parsed = parseJson(body, "catalog_invalid"); | ||
| validateRemoteCatalog(parsed); | ||
| const keyId = response.headers.get("x-opencodex-key-id")?.trim() || undefined; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 21280
Synchronize the localized lifecycle pages.
Add
--catalog-timeout <seconds>and the five-second inactivity-timeout behavior todocs-site/src/content/docs/{ja,ko,ru,zh-cn}/reference/cli/lifecycle.md. These pages currently omit the option, so localized users cannot discover the supported timeout override.🤖 Prompt for AI Agents
Sources: Path instructions, Learnings