mirror of
https://github.com/musistudio/claude-code-router.git
synced 2026-08-29 03:12:10 +08:00
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.
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user