Skip to content

Commit 82ae81f

Browse files
vincentkocvincentkoc
committed
fix(e2e): time out package url response bodies
1 parent 2db37c2 commit 82ae81f

2 files changed

Lines changed: 118 additions & 19 deletions

File tree

‎scripts/resolve-openclaw-package-candidate.mjs‎

Lines changed: 84 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -987,6 +987,15 @@ function responseHeader(response, name) {
987987
return response.headers?.get?.(name) ?? null;
988988
}
989989

990+
function createPackageDownloadTimeoutError(parsed, timeoutMs) {
991+
return Object.assign(
992+
new Error(`package_url download timed out after ${timeoutMs}ms: ${parsed.toString()}`),
993+
{
994+
code: "ETIMEDOUT",
995+
},
996+
);
997+
}
998+
990999
async function closeResponseBody(body) {
9911000
if (!body) {
9921001
return;
@@ -1002,8 +1011,16 @@ async function closeResponseBody(body) {
10021011

10031012
async function openFetchPackageDownloadResponse(parsed, options) {
10041013
const controller = new AbortController();
1005-
const timeout = setTimeout(() => controller.abort(), options.timeoutMs);
1006-
timeout.unref?.();
1014+
const timeoutError = createPackageDownloadTimeoutError(parsed, options.timeoutMs);
1015+
let timeout;
1016+
const timeoutPromise = new Promise((_, reject) => {
1017+
timeout = setTimeout(() => {
1018+
controller.abort(timeoutError);
1019+
reject(timeoutError);
1020+
}, options.timeoutMs);
1021+
timeout.unref?.();
1022+
});
1023+
timeoutPromise.catch(() => {});
10071024
const response = await options
10081025
.fetchImpl(parsed, {
10091026
headers: options.headers,
@@ -1013,27 +1030,31 @@ async function openFetchPackageDownloadResponse(parsed, options) {
10131030
.catch((error) => {
10141031
clearTimeout(timeout);
10151032
if (error?.name === "AbortError") {
1016-
throw new Error(
1017-
`package_url download timed out after ${options.timeoutMs}ms: ${parsed.toString()}`,
1018-
{
1019-
cause: error,
1020-
},
1021-
);
1033+
throw Object.assign(timeoutError, { cause: error });
10221034
}
10231035
throw error;
10241036
});
10251037
return {
10261038
close: async () => closeResponseBody(response.body),
10271039
response,
10281040
timeout,
1041+
timeoutPromise,
10291042
timeoutMs: options.timeoutMs,
10301043
};
10311044
}
10321045

10331046
async function openHttpsPackageDownloadResponse(parsed, options) {
10341047
const controller = new AbortController();
1035-
const timeout = setTimeout(() => controller.abort(), options.timeoutMs);
1036-
timeout.unref?.();
1048+
const timeoutError = createPackageDownloadTimeoutError(parsed, options.timeoutMs);
1049+
let timeout;
1050+
const timeoutPromise = new Promise((_, reject) => {
1051+
timeout = setTimeout(() => {
1052+
controller.abort(timeoutError);
1053+
reject(timeoutError);
1054+
}, options.timeoutMs);
1055+
timeout.unref?.();
1056+
});
1057+
timeoutPromise.catch(() => {});
10371058
const lookup = createPinnedLookup(parsed.hostname, options.addresses);
10381059
const response = await new Promise((resolve, reject) => {
10391060
const request = httpsRequest(
@@ -1065,12 +1086,7 @@ async function openHttpsPackageDownloadResponse(parsed, options) {
10651086
/** @param {unknown} error */ (error) => {
10661087
clearTimeout(timeout);
10671088
if (error?.name === "AbortError" || error?.code === "ABORT_ERR") {
1068-
throw new Error(
1069-
`package_url download timed out after ${options.timeoutMs}ms: ${parsed.toString()}`,
1070-
{
1071-
cause: error,
1072-
},
1073-
);
1089+
throw Object.assign(timeoutError, { cause: error });
10741090
}
10751091
throw error;
10761092
},
@@ -1079,6 +1095,7 @@ async function openHttpsPackageDownloadResponse(parsed, options) {
10791095
close: async () => closeResponseBody(response.body),
10801096
response,
10811097
timeout,
1098+
timeoutPromise,
10821099
timeoutMs: options.timeoutMs,
10831100
};
10841101
}
@@ -1124,7 +1141,46 @@ async function openPackageDownloadResponse(url, options) {
11241141
throw new Error(`package_url exceeded ${maxRedirects} redirects: ${url}`);
11251142
}
11261143

1127-
async function* limitResponseBody(body, maxBytes) {
1144+
async function* limitWebResponseBody(body, maxBytes, timeoutPromise) {
1145+
let downloaded = 0;
1146+
const reader = body.getReader();
1147+
let timedOut = false;
1148+
let timeoutFailure;
1149+
const timeoutRead = timeoutPromise?.catch((error) => {
1150+
timedOut = true;
1151+
timeoutFailure = error;
1152+
void reader.cancel().catch(() => {});
1153+
throw error;
1154+
});
1155+
try {
1156+
for (;;) {
1157+
const next = reader.read();
1158+
const { done, value } = timeoutRead ? await Promise.race([next, timeoutRead]) : await next;
1159+
if (timedOut) {
1160+
throw toLintErrorObject(timeoutFailure, "package_url download timed out");
1161+
}
1162+
if (done) {
1163+
return;
1164+
}
1165+
const size = typeof value === "string" ? Buffer.byteLength(value) : value.byteLength;
1166+
downloaded += size;
1167+
if (downloaded > maxBytes) {
1168+
throw new Error(`package_url exceeds maximum download size of ${maxBytes} bytes`);
1169+
}
1170+
yield value;
1171+
}
1172+
} finally {
1173+
if (!timedOut) {
1174+
reader.releaseLock();
1175+
}
1176+
}
1177+
}
1178+
1179+
async function* limitResponseBody(body, maxBytes, timeoutPromise) {
1180+
if (typeof body.getReader === "function") {
1181+
yield* limitWebResponseBody(body, maxBytes, timeoutPromise);
1182+
return;
1183+
}
11281184
let downloaded = 0;
11291185
for await (const chunk of body) {
11301186
const size = typeof chunk === "string" ? Buffer.byteLength(chunk) : chunk.byteLength;
@@ -1138,7 +1194,10 @@ async function* limitResponseBody(body, maxBytes) {
11381194

11391195
export async function downloadUrl(url, target, options = {}) {
11401196
const maxBytes = options.maxBytes ?? PACKAGE_URL_MAX_BYTES;
1141-
const { close, response, timeout, timeoutMs } = await openPackageDownloadResponse(url, options);
1197+
const { close, response, timeout, timeoutMs, timeoutPromise } = await openPackageDownloadResponse(
1198+
url,
1199+
options,
1200+
);
11421201
const tempTarget = `${target}.tmp`;
11431202
try {
11441203
if (!responseOk(response) || !response.body) {
@@ -1151,9 +1210,15 @@ export async function downloadUrl(url, target, options = {}) {
11511210
throw new Error(`package_url exceeds maximum download size of ${maxBytes} bytes`);
11521211
}
11531212
await fs.rm(tempTarget, { force: true });
1154-
await pipeline(limitResponseBody(response.body, maxBytes), createWriteStream(tempTarget));
1213+
await pipeline(
1214+
limitResponseBody(response.body, maxBytes, timeoutPromise),
1215+
createWriteStream(tempTarget),
1216+
);
11551217
await fs.rename(tempTarget, target);
11561218
} catch (error) {
1219+
if (error?.code === "ETIMEDOUT") {
1220+
throw error;
1221+
}
11571222
if (error?.name === "AbortError") {
11581223
throw new Error(`package_url download timed out after ${timeoutMs}ms: ${url}`, {
11591224
cause: error,

‎test/scripts/resolve-openclaw-package-candidate.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,40 @@ describe("resolve-openclaw-package-candidate", () => {
758758
await expect(missing(`${target}.tmp`)).resolves.toBe(true);
759759
});
760760

761+
it("times out stalled package_url response bodies", async () => {
762+
const dir = await mkdtemp(path.join(tmpdir(), "openclaw-package-download-timeout-"));
763+
tempDirs.push(dir);
764+
const target = path.join(dir, "openclaw.tgz");
765+
let bodyCancelled = false;
766+
const startedAt = Date.now();
767+
768+
await expect(
769+
downloadUrl("https://packages.example/openclaw.tgz", target, {
770+
fetchImpl: async () =>
771+
new Response(
772+
new ReadableStream({
773+
pull() {
774+
return new Promise(() => {});
775+
},
776+
cancel() {
777+
bodyCancelled = true;
778+
},
779+
}),
780+
{ status: 200 },
781+
),
782+
lookupHost: lookupAddresses([{ address: "93.184.216.34", family: 4 }]),
783+
timeoutMs: 25,
784+
}),
785+
).rejects.toThrow(
786+
"package_url download timed out after 25ms: https://packages.example/openclaw.tgz",
787+
);
788+
789+
expect(Date.now() - startedAt).toBeLessThan(1_000);
790+
expect(bodyCancelled).toBe(true);
791+
await expect(missing(target)).resolves.toBe(true);
792+
await expect(missing(`${target}.tmp`)).resolves.toBe(true);
793+
});
794+
761795
it("streams non-decimal package_url content-length values through the download cap", async () => {
762796
const dir = await mkdtemp(path.join(tmpdir(), "openclaw-package-download-"));
763797
tempDirs.push(dir);

0 commit comments

Comments
 (0)