From 6c98ea2adf915a22e8434a6a96143e8cb5f11512 Mon Sep 17 00:00:00 2001 From: pacocartones Date: Mon, 24 Aug 2026 04:27:52 +0000 Subject: [PATCH] fix(media): release artifact response body on early download failures GatewayMediaExecutor.download() left the fetch Response body un-read and un-cancelled on several early-exit paths: a terminal non-ok HTTP status, an oversized declared content-length (declaredLength > maxApiArtifactBytes), and a mid-stream writeSync failure in the download loop. In each case the underlying undici connection/socket is leaked, so repeated failed downloads accumulate open connections and exhaust the pool. Cancel the response body before throwing on those paths, and cancel the reader in the write-error handler, matching the cancellation already done on the retry branch and the in-loop size-limit branch. --- packages/core/src/media/executors.ts | 13 +++- .../media/media-executor-download.test.mjs | 76 +++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 packages/core/test/unit/media/media-executor-download.test.mjs diff --git a/packages/core/src/media/executors.ts b/packages/core/src/media/executors.ts index 690c7a5f..0fcd865a 100644 --- a/packages/core/src/media/executors.ts +++ b/packages/core/src/media/executors.ts @@ -115,15 +115,21 @@ export class GatewayMediaExecutor { if (attempt < 3) await delay(attempt * 500, undefined, { signal }); } if (!response) throw lastError ?? mediaError("artifact_download_failed", "Failed to download generated artifact.", true); - if (!response.ok) throw mediaError("artifact_download_failed", `Failed to download generated artifact: HTTP ${response.status}.`, true); + if (!response.ok) { + await response.body?.cancel(); + throw mediaError("artifact_download_failed", `Failed to download generated artifact: HTTP ${response.status}.`, true); + } const declaredLength = Number(response.headers.get("content-length") ?? 0); - if (declaredLength > maxApiArtifactBytes) throw mediaError("artifact_too_large", "Generated artifact exceeds the 250 MB limit.", false); + if (declaredLength > maxApiArtifactBytes) { + await response.body?.cancel(); + throw mediaError("artifact_too_large", "Generated artifact exceeds the 250 MB limit.", false); + } if (!response.body) throw mediaError("artifact_download_failed", "Generated artifact response has no body.", true); const temporary = path.join(os.tmpdir(), `ccr-media-${randomUUID()}.download`); const file = openSync(temporary, "wx", 0o600); + const reader = response.body.getReader(); let size = 0; try { - const reader = response.body.getReader(); while (true) { const chunk = await reader.read(); if (chunk.done) break; @@ -136,6 +142,7 @@ export class GatewayMediaExecutor { writeSync(file, buffer); } } catch (error) { + await reader.cancel().catch(() => undefined); closeSync(file); rmSync(temporary, { force: true }); throw error; diff --git a/packages/core/test/unit/media/media-executor-download.test.mjs b/packages/core/test/unit/media/media-executor-download.test.mjs new file mode 100644 index 00000000..34eb302b --- /dev/null +++ b/packages/core/test/unit/media/media-executor-download.test.mjs @@ -0,0 +1,76 @@ +import assert from "node:assert/strict"; +import http from "node:http"; +import test from "node:test"; +import { GatewayMediaExecutor } from "@ccr/core/media/executors.ts"; + +// Starts a loopback server that reports a large content-length but never ends +// the body, so the client keeps the connection open unless it explicitly +// cancels the response body. `closed.fired` flips once the upstream socket is +// torn down, which only happens when `download()` releases the body. +function stalledArtifactServer(statusCode) { + return new Promise((resolve) => { + const closed = { fired: false }; + const server = http.createServer((request, response) => { + request.on("close", () => { + closed.fired = true; + }); + response.writeHead(statusCode, { + "content-length": String(300 * 1024 * 1024), + "content-type": "application/octet-stream" + }); + response.write(Buffer.alloc(16)); + }); + server.listen(0, "127.0.0.1", () => { + resolve({ closed, port: server.address().port, server }); + }); + }); +} + +function loopbackExecutor(port) { + return new GatewayMediaExecutor( + { + model: "test-model", + protocol: "openai", + providerBaseUrl: `http://127.0.0.1:${port}`, + providerName: "test-provider" + }, + {} + ); +} + +async function connectionClosedWithin(closed, timeoutMs) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (closed.fired) return true; + await new Promise((resolve) => setTimeout(resolve, 25)); + } + return closed.fired; +} + +test("download releases the response body when the declared artifact is too large", async () => { + const { closed, port, server } = await stalledArtifactServer(200); + const executor = loopbackExecutor(port); + try { + await assert.rejects( + executor.download({ fileName: "artifact.bin", remoteUrl: `http://127.0.0.1:${port}/artifact` }, new AbortController().signal), + /exceeds the 250 MB limit/ + ); + assert.equal(await connectionClosedWithin(closed, 2000), true, "expected the upstream response body to be cancelled"); + } finally { + server.close(); + } +}); + +test("download releases the response body on a non-ok status", async () => { + const { closed, port, server } = await stalledArtifactServer(404); + const executor = loopbackExecutor(port); + try { + await assert.rejects( + executor.download({ fileName: "artifact.bin", remoteUrl: `http://127.0.0.1:${port}/artifact` }, new AbortController().signal), + /HTTP 404/ + ); + assert.equal(await connectionClosedWithin(closed, 2000), true, "expected the upstream response body to be cancelled"); + } finally { + server.close(); + } +});