mirror of
https://github.com/Routstr/routstrd.git
synced 2026-10-05 20:38:22 +00:00
fix(update): bound downloads with a two-tier deadline
`routstrd update` gave the whole release download the same 30 s budget as the connection: `AbortSignal.timeout(FETCH_TIMEOUT_MS)` was attached to the fetch, then the 38 MB archive was read with `arrayBuffer()` under that same signal. Anything slower than ~1.3 MB/s failed with "The operation timed out." — including the 540 KB/s / 70 s connection this was reproduced on. Split the deadline in two: - connect + response headers: 30 s, unchanged. The releases API and SHA256SUMS are a few KB, and a hang there is a real failure worth surfacing quickly. - release archive body: 300 s. That is 38 MB / 300 s ≈ 127 KB/s ≈ 1 Mbit/s, about 4x headroom over the reproduction, while still bounding the silent failure (the command prints no progress) at five minutes. `fetchOrThrow` now owns an AbortController and reschedules the deadline once headers arrive, taking a `consume` callback so the body read is covered by the transfer budget. Aborts report the phase and URL instead of a bare "The operation timed out." Also bound `getLatestNpmVersion`, which called `fetch` with no timeout at all on the same `routstrd update` path; it already returns null on error, so a stalled registry now degrades to "latest version unknown" instead of hanging. Adds regression tests: a body slower than the connect budget must still install, and a body overrunning the transfer budget must fail with the phase-aware message.
This commit is contained in:
@@ -13,11 +13,19 @@ import { tmpdir } from "os";
|
|||||||
import { basename, join } from "path";
|
import { basename, join } from "path";
|
||||||
|
|
||||||
const RELEASES_API = "https://api.github.com/repos/Routstr/routstrd/releases/latest";
|
const RELEASES_API = "https://api.github.com/repos/Routstr/routstrd/releases/latest";
|
||||||
const FETCH_TIMEOUT_MS = 30_000;
|
/** Time allowed to connect and receive response headers. */
|
||||||
|
const CONNECT_TIMEOUT_MS = 30_000;
|
||||||
|
/** Time allowed to download a release archive once headers have arrived. */
|
||||||
|
const TRANSFER_TIMEOUT_MS = 300_000;
|
||||||
const PROCESS_TIMEOUT_MS = 30_000;
|
const PROCESS_TIMEOUT_MS = 30_000;
|
||||||
const MAX_ARCHIVE_BYTES = 250 * 1024 * 1024;
|
const MAX_ARCHIVE_BYTES = 250 * 1024 * 1024;
|
||||||
const MAX_CHECKSUM_BYTES = 1024 * 1024;
|
const MAX_CHECKSUM_BYTES = 1024 * 1024;
|
||||||
|
|
||||||
|
export type UpdateTimeouts = {
|
||||||
|
connectMs?: number;
|
||||||
|
transferMs?: number;
|
||||||
|
};
|
||||||
|
|
||||||
type ReleaseAsset = {
|
type ReleaseAsset = {
|
||||||
name: string;
|
name: string;
|
||||||
browser_download_url: string;
|
browser_download_url: string;
|
||||||
@@ -52,18 +60,47 @@ export function releaseArchiveName(
|
|||||||
return `routstrd-v${normalizeVersion(version)}-${platform}-${arch}.tar.gz`;
|
return `routstrd-v${normalizeVersion(version)}-${platform}-${arch}.tar.gz`;
|
||||||
}
|
}
|
||||||
|
|
||||||
async function fetchOrThrow(
|
type FetchOptions = {
|
||||||
|
maxBytes?: number;
|
||||||
|
connectTimeoutMs?: number;
|
||||||
|
transferTimeoutMs?: number;
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Fetch `url` and hand the response to `consume` while it is still covered by a
|
||||||
|
* deadline. The connect+header phase gets the short `CONNECT_TIMEOUT_MS`; once
|
||||||
|
* headers are in, the deadline is relaxed to the (much longer)
|
||||||
|
* `transferTimeoutMs` so that a slow-but-progressing download is not killed for
|
||||||
|
* failing to fit a 38 MB archive into the connect budget.
|
||||||
|
*/
|
||||||
|
async function fetchOrThrow<T>(
|
||||||
url: string,
|
url: string,
|
||||||
fetchImpl: typeof fetch,
|
fetchImpl: typeof fetch,
|
||||||
|
{
|
||||||
maxBytes = MAX_CHECKSUM_BYTES,
|
maxBytes = MAX_CHECKSUM_BYTES,
|
||||||
): Promise<Response> {
|
connectTimeoutMs = CONNECT_TIMEOUT_MS,
|
||||||
|
transferTimeoutMs = CONNECT_TIMEOUT_MS,
|
||||||
|
}: FetchOptions,
|
||||||
|
consume: (response: Response) => Promise<T>,
|
||||||
|
): Promise<T> {
|
||||||
|
const controller = new AbortController();
|
||||||
|
let deadlineHit = false;
|
||||||
|
let phase: "connect" | "transfer" = "connect";
|
||||||
|
const startDeadline = (ms: number) =>
|
||||||
|
setTimeout(() => {
|
||||||
|
deadlineHit = true;
|
||||||
|
controller.abort();
|
||||||
|
}, ms);
|
||||||
|
let timer = startDeadline(connectTimeoutMs);
|
||||||
|
|
||||||
|
try {
|
||||||
const response = await fetchImpl(url, {
|
const response = await fetchImpl(url, {
|
||||||
headers: {
|
headers: {
|
||||||
Accept: "application/vnd.github+json",
|
Accept: "application/vnd.github+json",
|
||||||
"User-Agent": "routstrd",
|
"User-Agent": "routstrd",
|
||||||
},
|
},
|
||||||
redirect: "follow",
|
redirect: "follow",
|
||||||
signal: AbortSignal.timeout(FETCH_TIMEOUT_MS),
|
signal: controller.signal,
|
||||||
});
|
});
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
throw new Error(`Download failed (${response.status}) for ${url}`);
|
throw new Error(`Download failed (${response.status}) for ${url}`);
|
||||||
@@ -72,7 +109,25 @@ async function fetchOrThrow(
|
|||||||
if (Number.isFinite(contentLength) && contentLength > maxBytes) {
|
if (Number.isFinite(contentLength) && contentLength > maxBytes) {
|
||||||
throw new Error(`Download is too large (${contentLength} bytes) for ${url}`);
|
throw new Error(`Download is too large (${contentLength} bytes) for ${url}`);
|
||||||
}
|
}
|
||||||
return response;
|
|
||||||
|
// Headers are in: the body gets its own, larger budget.
|
||||||
|
phase = "transfer";
|
||||||
|
clearTimeout(timer);
|
||||||
|
timer = startDeadline(transferTimeoutMs);
|
||||||
|
|
||||||
|
return await consume(response);
|
||||||
|
} catch (error) {
|
||||||
|
if (deadlineHit) {
|
||||||
|
const seconds = Math.round(
|
||||||
|
(phase === "connect" ? connectTimeoutMs : transferTimeoutMs) / 1000,
|
||||||
|
);
|
||||||
|
const what = phase === "connect" ? "Connecting to" : "Downloading from";
|
||||||
|
throw new Error(`${what} ${url} timed out after ${seconds}s.`, { cause: error });
|
||||||
|
}
|
||||||
|
throw error;
|
||||||
|
} finally {
|
||||||
|
clearTimeout(timer);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async function waitForExit(
|
async function waitForExit(
|
||||||
@@ -100,8 +155,12 @@ export async function getLatestStandaloneRelease(
|
|||||||
arch: string = process.arch,
|
arch: string = process.arch,
|
||||||
fetchImpl: typeof fetch = fetch,
|
fetchImpl: typeof fetch = fetch,
|
||||||
): Promise<StandaloneRelease> {
|
): Promise<StandaloneRelease> {
|
||||||
const response = await fetchOrThrow(RELEASES_API, fetchImpl);
|
const release = await fetchOrThrow(
|
||||||
const release = (await response.json()) as GithubRelease;
|
RELEASES_API,
|
||||||
|
fetchImpl,
|
||||||
|
{},
|
||||||
|
async (response) => (await response.json()) as GithubRelease,
|
||||||
|
);
|
||||||
const version = normalizeVersion(release.tag_name || "");
|
const version = normalizeVersion(release.tag_name || "");
|
||||||
if (!/^\d+\.\d+\.\d+/.test(version)) {
|
if (!/^\d+\.\d+\.\d+/.test(version)) {
|
||||||
throw new Error("The latest GitHub Release has an invalid version tag.");
|
throw new Error("The latest GitHub Release has an invalid version tag.");
|
||||||
@@ -151,28 +210,43 @@ export async function installStandaloneRelease(
|
|||||||
release: StandaloneRelease,
|
release: StandaloneRelease,
|
||||||
executablePath: string = process.execPath,
|
executablePath: string = process.execPath,
|
||||||
fetchImpl: typeof fetch = fetch,
|
fetchImpl: typeof fetch = fetch,
|
||||||
|
timeouts: UpdateTimeouts = {},
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
const connectMs = timeouts.connectMs ?? CONNECT_TIMEOUT_MS;
|
||||||
|
const transferMs = timeouts.transferMs ?? TRANSFER_TIMEOUT_MS;
|
||||||
|
/** SHA256SUMS is a few hundred bytes, so it gets the short budget, not the archive's. */
|
||||||
|
const checksumMs = connectMs;
|
||||||
const tempDir = mkdtempSync(join(tmpdir(), "routstrd-update-"));
|
const tempDir = mkdtempSync(join(tmpdir(), "routstrd-update-"));
|
||||||
const archivePath = join(tempDir, release.archive.name);
|
const archivePath = join(tempDir, release.archive.name);
|
||||||
const stagedPath = `${executablePath}.update-${randomBytes(12).toString("hex")}`;
|
const stagedPath = `${executablePath}.update-${randomBytes(12).toString("hex")}`;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const [archiveResponse, checksumsResponse] = await Promise.all([
|
const [archiveBytes, checksums] = await Promise.all([
|
||||||
fetchOrThrow(
|
fetchOrThrow(
|
||||||
release.archive.browser_download_url,
|
release.archive.browser_download_url,
|
||||||
fetchImpl,
|
fetchImpl,
|
||||||
MAX_ARCHIVE_BYTES,
|
{ maxBytes: MAX_ARCHIVE_BYTES, connectTimeoutMs: connectMs, transferTimeoutMs: transferMs },
|
||||||
),
|
async (response) => {
|
||||||
fetchOrThrow(release.checksums.browser_download_url, fetchImpl),
|
const bytes = await response.arrayBuffer();
|
||||||
]);
|
if (bytes.byteLength > MAX_ARCHIVE_BYTES) {
|
||||||
const archiveBytes = await archiveResponse.arrayBuffer();
|
throw new Error(`Download is too large (${bytes.byteLength} bytes).`);
|
||||||
if (archiveBytes.byteLength > MAX_ARCHIVE_BYTES) {
|
|
||||||
throw new Error(`Download is too large (${archiveBytes.byteLength} bytes).`);
|
|
||||||
}
|
}
|
||||||
const checksums = await checksumsResponse.text();
|
return bytes;
|
||||||
if (Buffer.byteLength(checksums) > MAX_CHECKSUM_BYTES) {
|
},
|
||||||
|
),
|
||||||
|
fetchOrThrow(
|
||||||
|
release.checksums.browser_download_url,
|
||||||
|
fetchImpl,
|
||||||
|
{ connectTimeoutMs: connectMs, transferTimeoutMs: checksumMs },
|
||||||
|
async (response) => {
|
||||||
|
const contents = await response.text();
|
||||||
|
if (Buffer.byteLength(contents) > MAX_CHECKSUM_BYTES) {
|
||||||
throw new Error("SHA256SUMS is too large.");
|
throw new Error("SHA256SUMS is too large.");
|
||||||
}
|
}
|
||||||
|
return contents;
|
||||||
|
},
|
||||||
|
),
|
||||||
|
]);
|
||||||
const expected = expectedChecksum(checksums, release.archive.name);
|
const expected = expectedChecksum(checksums, release.archive.name);
|
||||||
const actual = sha256(archiveBytes);
|
const actual = sha256(archiveBytes);
|
||||||
if (actual !== expected) {
|
if (actual !== expected) {
|
||||||
|
|||||||
@@ -3,6 +3,8 @@ import { VERSION } from "../version";
|
|||||||
import { getLatestStandaloneRelease } from "./standalone-update";
|
import { getLatestStandaloneRelease } from "./standalone-update";
|
||||||
|
|
||||||
const NPM_REGISTRY = "https://registry.npmjs.org";
|
const NPM_REGISTRY = "https://registry.npmjs.org";
|
||||||
|
/** Bounds the update check so a stalled registry cannot hang `routstrd update`. */
|
||||||
|
const NPM_TIMEOUT_MS = 30_000;
|
||||||
|
|
||||||
/** Packages that `routstrd update` manages. */
|
/** Packages that `routstrd update` manages. */
|
||||||
export const UPDATE_PACKAGES = [
|
export const UPDATE_PACKAGES = [
|
||||||
@@ -20,6 +22,7 @@ export async function getLatestNpmVersion(
|
|||||||
try {
|
try {
|
||||||
const response = await fetch(
|
const response = await fetch(
|
||||||
`${NPM_REGISTRY}/${encodeURIComponent(packageName)}/latest`,
|
`${NPM_REGISTRY}/${encodeURIComponent(packageName)}/latest`,
|
||||||
|
{ signal: AbortSignal.timeout(NPM_TIMEOUT_MS) },
|
||||||
);
|
);
|
||||||
if (!response.ok) return null;
|
if (!response.ok) return null;
|
||||||
const data = (await response.json()) as { version?: string };
|
const data = (await response.json()) as { version?: string };
|
||||||
|
|||||||
@@ -179,3 +179,118 @@ describe("installStandaloneRelease", () => {
|
|||||||
expect(readFileSync(current, "utf8")).toBe(original);
|
expect(readFileSync(current, "utf8")).toBe(original);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("download deadlines", () => {
|
||||||
|
async function makeFixture() {
|
||||||
|
const dir = mkdtempSync(join(tmpdir(), "routstrd-updater-timeout-"));
|
||||||
|
tempDirs.push(dir);
|
||||||
|
const current = join(dir, "installed-routstrd");
|
||||||
|
const archiveRoot = join(dir, "archive");
|
||||||
|
const candidate = join(archiveRoot, "routstrd");
|
||||||
|
const archivePath = join(dir, "routstrd-v0.5.0-linux-x64.tar.gz");
|
||||||
|
mkdirSync(archiveRoot);
|
||||||
|
writeFileSync(current, "#!/bin/sh\necho 0.4.4\n");
|
||||||
|
writeFileSync(candidate, "#!/bin/sh\necho 0.5.0\n");
|
||||||
|
chmodSync(current, 0o755);
|
||||||
|
chmodSync(candidate, 0o755);
|
||||||
|
const tar = Bun.spawnSync([
|
||||||
|
"tar",
|
||||||
|
"-C",
|
||||||
|
archiveRoot,
|
||||||
|
"-czf",
|
||||||
|
archivePath,
|
||||||
|
"routstrd",
|
||||||
|
]);
|
||||||
|
expect(tar.exitCode).toBe(0);
|
||||||
|
const archiveBytes = readFileSync(archivePath);
|
||||||
|
return {
|
||||||
|
current,
|
||||||
|
archiveBytes,
|
||||||
|
releaseName: "routstrd-v0.5.0-linux-x64.tar.gz",
|
||||||
|
checksum: sha256(archiveBytes),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Headers land immediately; the body arrives after `delayMs`, or errors on abort. */
|
||||||
|
function delayedResponse(
|
||||||
|
bytes: Uint8Array,
|
||||||
|
delayMs: number,
|
||||||
|
signal: AbortSignal | null | undefined,
|
||||||
|
): Response {
|
||||||
|
const stream = new ReadableStream<Uint8Array>({
|
||||||
|
start(controller) {
|
||||||
|
const timer = setTimeout(() => {
|
||||||
|
controller.enqueue(bytes);
|
||||||
|
controller.close();
|
||||||
|
}, delayMs);
|
||||||
|
signal?.addEventListener(
|
||||||
|
"abort",
|
||||||
|
() => {
|
||||||
|
clearTimeout(timer);
|
||||||
|
controller.error(new DOMException("Aborted", "AbortError"));
|
||||||
|
},
|
||||||
|
{ once: true },
|
||||||
|
);
|
||||||
|
},
|
||||||
|
});
|
||||||
|
return new Response(stream);
|
||||||
|
}
|
||||||
|
|
||||||
|
function slowArchiveFetch(
|
||||||
|
archiveBytes: Uint8Array,
|
||||||
|
checksum: string,
|
||||||
|
releaseName: string,
|
||||||
|
bodyDelayMs: number,
|
||||||
|
): typeof fetch {
|
||||||
|
return (async (input: string | URL | Request, init?: RequestInit) =>
|
||||||
|
String(input).endsWith("SHA256SUMS")
|
||||||
|
? new Response(`${checksum} ${releaseName}\n`)
|
||||||
|
: delayedResponse(archiveBytes, bodyDelayMs, init?.signal)) as typeof fetch;
|
||||||
|
}
|
||||||
|
|
||||||
|
test("does not kill a slow archive on the connect budget", async () => {
|
||||||
|
const { current, archiveBytes, releaseName, checksum } = await makeFixture();
|
||||||
|
// Body takes 80ms; the connect budget is 20ms. Regression guard for the flat
|
||||||
|
// 30s deadline that failed real 70s downloads.
|
||||||
|
const fetchImpl = slowArchiveFetch(archiveBytes, checksum, releaseName, 80);
|
||||||
|
|
||||||
|
await installStandaloneRelease(
|
||||||
|
{
|
||||||
|
version: "0.5.0",
|
||||||
|
archive: { name: releaseName, browser_download_url: "https://example/archive" },
|
||||||
|
checksums: { name: "SHA256SUMS", browser_download_url: "https://example/SHA256SUMS" },
|
||||||
|
},
|
||||||
|
current,
|
||||||
|
fetchImpl,
|
||||||
|
{ connectMs: 20, transferMs: 2_000 },
|
||||||
|
);
|
||||||
|
|
||||||
|
const version = Bun.spawnSync([current, "--version"]);
|
||||||
|
expect(version.exitCode).toBe(0);
|
||||||
|
expect(version.stdout.toString().trim()).toBe("0.5.0");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("gives up when the transfer deadline is exceeded", async () => {
|
||||||
|
const { current, archiveBytes, releaseName, checksum } = await makeFixture();
|
||||||
|
const fetchImpl = slowArchiveFetch(archiveBytes, checksum, releaseName, 300);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
installStandaloneRelease(
|
||||||
|
{
|
||||||
|
version: "0.5.0",
|
||||||
|
archive: {
|
||||||
|
name: releaseName,
|
||||||
|
browser_download_url: "https://example/archive",
|
||||||
|
},
|
||||||
|
checksums: {
|
||||||
|
name: "SHA256SUMS",
|
||||||
|
browser_download_url: "https://example/SHA256SUMS",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
current,
|
||||||
|
fetchImpl,
|
||||||
|
{ connectMs: 1_000, transferMs: 50 },
|
||||||
|
),
|
||||||
|
).rejects.toThrow(/Downloading from .* timed out after/);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user