From 8e3526f09966ee72766cd3f055d18e9cf6c5f56b Mon Sep 17 00:00:00 2001 From: Danilo Alonso Date: Sat, 11 Jul 2026 15:36:11 -0400 Subject: [PATCH] feat(update): resume binary download with range requests and retry A stall or dropped connection now retries (up to 5 attempts with backoff) and resumes from the bytes on disk via an HTTP range request, guarded by If-Range/ETag so a changed asset restarts cleanly. Permanent errors like 404 fail fast; the CLI shows a resuming notice between attempts. --- .changeset/update-resumable-download.md | 5 + src/cli/update.ts | 13 ++ src/core/update/types.ts | 1 + src/core/update/updater.ts | 213 ++++++++++++++++++++---- tests/core/update/updater.test.ts | 133 ++++++++++++++- 5 files changed, 329 insertions(+), 36 deletions(-) create mode 100644 .changeset/update-resumable-download.md diff --git a/.changeset/update-resumable-download.md b/.changeset/update-resumable-download.md new file mode 100644 index 0000000..00d1434 --- /dev/null +++ b/.changeset/update-resumable-download.md @@ -0,0 +1,5 @@ +--- +"@noormdev/cli": patch +--- + +`feat(update):` the binary self-update now resumes instead of restarting. When a download stalls or the connection drops, `noorm update` retries (up to 5 attempts with backoff) and resumes from the bytes already on disk via an HTTP range request — validated with `If-Range`/`ETag` so a changed asset restarts cleanly rather than stitching a stale prefix. A flaky connection now retries the tail, not the whole ~70MB. Permanent failures (e.g. `404`) still fail fast without retrying, and the CLI prints a `resuming (attempt N/M)` notice between attempts. diff --git a/src/cli/update.ts b/src/cli/update.ts index d87bb96..e1bb30d 100644 --- a/src/cli/update.ts +++ b/src/cli/update.ts @@ -98,6 +98,17 @@ const updateCommand = defineCommand({ }; + const onRetry = ({ attempt: n, maxAttempts, error }: { attempt: number; maxAttempts: number; error: string }) => { + + if (args.json) return; + + // End the in-place progress line before printing the notice. + const prefix = isTty ? '\n' : ''; + + process.stdout.write(`${prefix}${error} — resuming (attempt ${n + 1}/${maxAttempts})...\n`); + + }; + if (!args.json) { process.stdout.write(isTty ? 'Installing...\n' : 'Installing (downloading binary)...\n'); @@ -105,10 +116,12 @@ const updateCommand = defineCommand({ } observer.on('update:progress', onProgress); + observer.on('update:retry', onRetry); const [result, installErr] = await attempt(() => installUpdate(checkResult.latestVersion)); observer.off('update:progress', onProgress); + observer.off('update:retry', onRetry); if (isTty) { diff --git a/src/core/update/types.ts b/src/core/update/types.ts index 9ed1425..c2e5c62 100644 --- a/src/core/update/types.ts +++ b/src/core/update/types.ts @@ -118,6 +118,7 @@ export interface UpdateEvents { // Update installation 'update:installing': { version: string }; 'update:progress': { version: string; received: number; total: number }; + 'update:retry': { version: string; attempt: number; maxAttempts: number; error: string }; 'update:complete': { previousVersion: string; newVersion: string }; 'update:failed': { version: string; error: string }; 'update:skipped': { version: string; reason: 'user-denied' | 'user-deferred' }; diff --git a/src/core/update/updater.ts b/src/core/update/updater.ts index d8bf505..4018c88 100644 --- a/src/core/update/updater.ts +++ b/src/core/update/updater.ts @@ -14,7 +14,7 @@ * ``` */ import { spawn } from 'child_process'; -import { open, rename, unlink, chmod } from 'fs/promises'; +import { open, rename, unlink, chmod, stat } from 'fs/promises'; import { attempt } from '@logosdx/utils'; @@ -31,12 +31,23 @@ import type { UpdateResult } from './types.js'; const PACKAGE_NAME = '@noormdev/cli'; /** - * Abort the download if no bytes arrive for this long. A bare `fetch()` has no - * timeout, so a stalled connection would otherwise hang the process forever - * with no error to surface — this converts a silent hang into a real failure. + * Abort a download attempt if no bytes arrive for this long. A bare `fetch()` + * has no timeout, so a stalled connection would otherwise hang the process + * forever with no error to surface — this converts a silent hang into a real + * failure that the retry loop can act on. */ const DOWNLOAD_STALL_MS = 30_000; +/** + * How many times to (re)start a download before giving up. A stall or network + * error resumes from the bytes already on disk via an HTTP range request, so + * these are cheap — a flaky connection retries the tail, not the whole ~70MB. + */ +const DOWNLOAD_MAX_ATTEMPTS = 5; + +/** Base backoff between attempts; scaled by attempt number. */ +const RETRY_BACKOFF_MS = 500; + /** Emit a progress event at most once per this many bytes, to avoid spamming. */ const PROGRESS_EMIT_BYTES = 512 * 1024; @@ -131,27 +142,139 @@ function installViaNpm(version: string, previousVersion: string): Promise new Promise((resolve) => setTimeout(resolve, ms)); + +/** Current size of a file, or 0 if it doesn't exist — the resume offset. */ +async function fileSizeOrZero(path: string): Promise { + + const [info] = await attempt(() => stat(path)); + + return info?.size ?? 0; + +} + +/** Parse the total from a `Content-Range: bytes 200-1023/1024` header. */ +function parseContentRangeTotal(header: string | null): number { + + const total = header?.split('/')[1]; + + return total ? Number(total) || 0 : 0; + +} + +/** + * Download `url` to `destPath`, resuming from bytes already on disk and + * retrying transient failures, emitting `update:progress` throughout. * - * Streaming instead of buffering the whole ~70MB into memory lets the caller - * show real progress, and the stall-abort guarantees the download either - * completes, errors, or times out — never hangs indefinitely on a dead socket. + * Streaming (not buffering ~70MB into memory) lets the caller show progress; + * the per-attempt stall-abort guarantees no indefinite hang; and the range-based + * resume means a flaky connection retries the tail rather than the whole file. * - * @param stallMs - Abort if no chunk arrives within this window. Overridable - * for tests; production uses `DOWNLOAD_STALL_MS`. - * @throws Error on a non-OK response, an empty body, a stall, or a write failure. + * @throws DownloadError/Error when the retry budget is exhausted or the failure + * is non-retriable (e.g. HTTP 404). On throw, the partial file is left in place + * for the caller to clean up. */ export async function downloadToFile( url: string, destPath: string, version: string, - stallMs: number = DOWNLOAD_STALL_MS, + options: DownloadOptions = {}, ): Promise { - const controller = new AbortController(); + const stallMs = options.stallMs ?? DOWNLOAD_STALL_MS; + const maxAttempts = options.maxAttempts ?? DOWNLOAD_MAX_ATTEMPTS; + const backoffMs = options.backoffMs ?? RETRY_BACKOFF_MS; + + const state: DownloadState = { total: 0 }; + + for (let attemptNo = 1; attemptNo <= maxAttempts; attemptNo++) { + + const offset = await fileSizeOrZero(destPath); + + // A prior attempt already pulled the whole asset — nothing left to do. + if (state.total > 0 && offset >= state.total) break; + + const [, err] = await attempt(() => downloadAttempt(url, destPath, version, offset, state, stallMs)); + + if (!err) break; + + const retriable = !(err instanceof DownloadError) || err.retriable; + + if (!retriable || attemptNo >= maxAttempts) throw err; + observer.emit('update:retry', { version, attempt: attemptNo, maxAttempts, error: err.message }); + + await sleep(backoffMs * attemptNo); + + } + + await chmod(destPath, 0o755); + +} + +/** + * One download attempt: fetch (resuming from `offset` when possible), stream the + * body to `destPath`, and emit progress. Mutates `state.etag`/`state.total` as + * soon as headers arrive so the orchestrator can resume even if this attempt + * later throws. + * + * @throws DownloadError on an HTTP error, or the underlying stall/stream error. + */ +async function downloadAttempt( + url: string, + destPath: string, + version: string, + offset: number, + state: DownloadState, + stallMs: number, +): Promise { + + // Resume only when there are bytes on disk AND an ETag to guard against the + // asset changing under us; otherwise start clean. + const resuming = offset > 0 && state.etag !== undefined; + const headers: Record = resuming + ? { Range: `bytes=${offset}-`, 'If-Range': state.etag! } + : {}; + + const controller = new AbortController(); let stallTimer: ReturnType | undefined; const armStall = () => { @@ -164,24 +287,53 @@ export async function downloadToFile( }; - const response = await fetch(url, { signal: controller.signal }); + const response = await fetch(url, { headers, signal: controller.signal }); + + const etag = response.headers.get('etag'); + if (etag) state.etag = etag; + + // Decide where the write starts and in which mode. 206 = server honored the + // range → append. 200 = server sent the whole asset (fresh, or If-Range + // rejected because it changed) → truncate and start over. + let received: number; + let writeMode: 'a' | 'w'; - if (!response.ok || !response.body) { + if (response.status === 206) { + + state.total = parseContentRangeTotal(response.headers.get('content-range')) || state.total; + received = offset; + writeMode = 'a'; + + } + else if (response.ok) { + + state.total = Number(response.headers.get('content-length')) || 0; + received = 0; + writeMode = 'w'; + + } + else { clearTimeout(stallTimer); - throw new Error(`HTTP ${response.status} downloading binary`); + // 408/429/5xx are transient; other 4xx (404/403/…) are permanent. + const retriable = response.status === 408 || response.status === 429 || response.status >= 500; + + throw new DownloadError(`HTTP ${response.status} downloading binary`, retriable); } - const total = Number(response.headers.get('content-length')) || 0; - let received = 0; - let sinceLastEmit = 0; + if (!response.body) { + + clearTimeout(stallTimer); - const handle = await open(destPath, 'w'); + throw new DownloadError('empty response body', true); + + } + + const handle = await open(destPath, writeMode); + let sinceLastEmit = 0; - // A stall or write failure must still close the handle and clear the timer, - // so wrap the streaming loop and clean up in both outcomes. const [, streamErr] = await attempt(async () => { armStall(); @@ -197,15 +349,12 @@ export async function downloadToFile( if (sinceLastEmit >= PROGRESS_EMIT_BYTES) { sinceLastEmit = 0; - observer.emit('update:progress', { version, received, total }); + observer.emit('update:progress', { version, received, total: state.total }); } } - // Final tick so consumers see 100% even when the last chunk was small. - observer.emit('update:progress', { version, received, total: total || received }); - }); clearTimeout(stallTimer); @@ -221,7 +370,15 @@ export async function downloadToFile( } - await chmod(destPath, 0o755); + // A clean end that's short of the advertised total means the connection + // dropped without throwing — treat as retriable so the loop resumes. + if (state.total > 0 && received < state.total) { + + throw new DownloadError(`connection closed early (${received}/${state.total} bytes)`, true); + + } + + observer.emit('update:progress', { version, received, total: state.total || received }); } diff --git a/tests/core/update/updater.test.ts b/tests/core/update/updater.test.ts index 34903f7..0290adf 100644 --- a/tests/core/update/updater.test.ts +++ b/tests/core/update/updater.test.ts @@ -3,10 +3,11 @@ * * These exercise the real streaming download (`downloadToFile`) against a live * local HTTP server — no fetch/fs mocks — because the regressions we care about - * are behavioral: does it stream to disk, does it report progress, and does it - * fail (rather than hang forever) when the connection stalls? The `installUpdate` - * swap step is deliberately not tested here: in a test process `process.execPath` - * is the test runner's own binary, and swapping it would be catastrophic. + * are behavioral: does it stream to disk, report progress, resume from a stall + * via a range request, and fail (rather than hang) once retries run out? The + * `installUpdate` swap step is deliberately not tested here: in a test process + * `process.execPath` is the test runner's own binary, and swapping it would be + * catastrophic. */ import { describe, it, expect, beforeAll, afterAll } from 'bun:test'; import { readFile, stat, mkdtemp, rm } from 'fs/promises'; @@ -26,6 +27,14 @@ let workDir: string; // times, so we can assert on real chunked progress rather than a single tick. const PAYLOAD = new Uint8Array(1_500_000).fill(65); // 1.5 MB of 'A' +// How much /resume streams before it stalls on the first request. +const RESUME_PARTIAL = 700_000; +const RESUME_ETAG = '"asset-v1"'; + +// Range headers seen by /resume, so a test can prove the second attempt resumed +// (sent a byte range) rather than restarting from scratch. +let resumeRanges: Array = []; + beforeAll(async () => { workDir = await mkdtemp(join(tmpdir(), 'noorm-updater-test-')); @@ -65,6 +74,47 @@ beforeAll(async () => { } + // Resumable asset. First request (no Range): 200 with an ETag, + // streams a partial prefix, then stalls. A subsequent Range request: + // 206 with the remainder, so the download completes on resume. + if (url.pathname === '/resume') { + + const range = req.headers.get('range'); + resumeRanges.push(range); + + if (!range) { + + const stream = new ReadableStream({ + start(controller) { + + controller.enqueue(PAYLOAD.slice(0, RESUME_PARTIAL)); + // stall — never enqueue the rest, never close + + }, + }); + + return new Response(stream, { + headers: { + 'content-length': String(PAYLOAD.byteLength), + etag: RESUME_ETAG, + }, + }); + + } + + const startByte = Number(/bytes=(\d+)-/.exec(range)?.[1] ?? 0); + + return new Response(PAYLOAD.slice(startByte), { + status: 206, + headers: { + etag: RESUME_ETAG, + 'content-range': `bytes ${startByte}-${PAYLOAD.byteLength - 1}/${PAYLOAD.byteLength}`, + 'content-length': String(PAYLOAD.byteLength - startByte), + }, + }); + + } + if (url.pathname === '/notfound') { return new Response('nope', { status: 404 }); @@ -87,7 +137,7 @@ afterAll(async () => { }); -describe('downloadToFile', () => { +describe('updater: downloadToFile', () => { it('streams the full body to disk and makes it executable', async () => { @@ -136,29 +186,96 @@ describe('downloadToFile', () => { }); + it('resumes from the partial via a range request after a stall', async () => { + + resumeRanges = []; + const dest = join(workDir, 'resume.bin'); + const retries: Array<{ attempt: number; maxAttempts: number }> = []; + + const onRetry = (r: { attempt: number; maxAttempts: number }) => { + + retries.push({ attempt: r.attempt, maxAttempts: r.maxAttempts }); + + }; + + observer.on('update:retry', onRetry); + await downloadToFile(`${baseUrl}/resume`, dest, '1.0.0-test', { stallMs: 300, backoffMs: 20 }); + observer.off('update:retry', onRetry); + + // The file is whole and correct despite the mid-stream stall. + const written = await readFile(dest); + expect(written.byteLength).toBe(PAYLOAD.byteLength); + expect(written.every((b) => b === 65)).toBe(true); + + // It retried once... + expect(retries.length).toBe(1); + + // ...and the retry was a RESUME: first request had no Range, the second + // asked for exactly the bytes already on disk — not a fresh restart. + expect(resumeRanges[0]).toBeNull(); + expect(resumeRanges[1]).toBe(`bytes=${RESUME_PARTIAL}-`); + + }); + it('aborts with a "stalled" error instead of hanging when the stream stops', async () => { const dest = join(workDir, 'stall.bin'); const start = performance.now(); - const [, err] = await attempt(() => downloadToFile(`${baseUrl}/stall`, dest, '1.0.0-test', 300)); + const [, err] = await attempt(() => + downloadToFile(`${baseUrl}/stall`, dest, '1.0.0-test', { stallMs: 200, maxAttempts: 1 }), + ); const elapsed = performance.now() - start; expect(err).toBeTruthy(); expect(err!.message).toContain('stalled'); - // proves it did not hang: bounded by the injected 300ms stall window + // proves it did not hang: one attempt, bounded by the 200ms stall window expect(elapsed).toBeLessThan(3000); }); - it('rejects a non-OK response with the status code', async () => { + it('gives up after exhausting the retry budget on a persistent stall', async () => { + + const dest = join(workDir, 'give-up.bin'); + let retryCount = 0; + + const onRetry = () => { + + retryCount++; + + }; + + observer.on('update:retry', onRetry); + const [, err] = await attempt(() => + downloadToFile(`${baseUrl}/stall`, dest, '1.0.0-test', { stallMs: 150, maxAttempts: 3, backoffMs: 10 }), + ); + observer.off('update:retry', onRetry); + + expect(err).toBeTruthy(); + expect(err!.message).toContain('stalled'); + // 3 attempts → 2 retry notices between them + expect(retryCount).toBe(2); + + }); + + it('does not retry a non-retriable 404', async () => { const dest = join(workDir, 'nf.bin'); + let retryCount = 0; + + const onRetry = () => { + + retryCount++; + + }; + observer.on('update:retry', onRetry); const [, err] = await attempt(() => downloadToFile(`${baseUrl}/notfound`, dest, '1.0.0-test')); + observer.off('update:retry', onRetry); expect(err).toBeTruthy(); expect(err!.message).toContain('404'); + expect(retryCount).toBe(0); // 4xx is permanent — fail fast, no retries });