feat(download): resume downloads via Range on transient failure

[rubenhensen]

Background

#47 added retry-with-backoff for chunk PUTs and download GETs. Chunk uploads benefit fully thanks to cryptify's idempotent-retry contract (encryption4all/cryptify#145): a retry replays the cached response without re-writing or double-counting. Downloads only retry the GET handshake — once the response stream starts and a transient failure interrupts it mid-read, the SDK currently has to abandon everything received and re-issue the request from byte zero.

cryptify's `FileServer` already supports HTTP Range requests out of the box, so the server side is essentially free. This issue tracks the SDK-side work to actually use it.

Proposed design

downloadFileWithRetry (src/api/cryptify.ts) is wrapped around withRetry, which today re-issues the GET on transient failure with no offset awareness. Extend it to:

1. Track received bytes. Tee the returned ReadableStream<Uint8Array> into a counter that the retry layer can read on each chunk delivered to the consumer.
2. Retry from received on stream-level failures. When the stream errors after some bytes have been read (network drop, idle timeout, etc.), the next attempt issues:

   GET /filedownload/{uuid}
   Range: bytes={received}-
   

   The server returns 206 Partial Content from the offset, and the SDK splices the new stream onto the consumer where the old one ended.
3. Re-classify mid-stream errors as retriable (today the withRetry loop only sees errors thrown from fetch()/response.body; mid-stream ReadableStream errors propagate to the caller without ever entering the loop). Pipe the stream through a transform that surfaces stream errors to the retry classifier.

Out of scope

• Cross-refresh download resume (persisting the partial-blob in IndexedDB so a refresh can pick up where the previous session left off). Could be a follow-up if it turns out useful — most downloads are short enough that in-session retry covers the common case.
• Cryptify-side changes — `FileServer` already does the right thing on Range. Verify in passing that 206s + content-range echo back as expected, but no server work needed.

Acceptance criteria

• Mid-stream network failure on a 100 MB download retries from the byte offset reached, not from zero
• Consumer sees a single contiguous stream (no duplicates, no gaps) regardless of how many internal retries happened
• Retry budget still bounded by the existing RetryOptions from feat(upload): retry chunks and downloads with exponential backoff #47
• Caller-driven AbortSignal aborts the whole operation, including any in-flight retry GET
• New mocked-network tests in tests/api.test.ts cover: clean download (1 attempt), mid-stream failure → resume from offset → success, persistent failure → exhausts budget then throws
• Manual verification: against a local cryptify, kill the server mid-download → SDK resumes when cryptify comes back

Refs

• Upload fails with 404 on chunk PUT — no retry, entire upload lost postguard-website#117 (root robustness issue)
• Upload and download sessions are not robust cryptify#136 (resume protocol design discussion — the comment also notes Range is already supported)
• feat(upload): retry chunks and downloads with exponential backoff #47 (this repo, retry/backoff foundation)
• feat(upload): status endpoint for cross-refresh resume cryptify#146 (sibling: cross-refresh upload resume)

[rubenhensen]

/dobby please review this design proposal — particularly the mid-stream-error retry classification and the splicing-on-resume idea. Is there a cleaner way to thread Range-based resume through the existing withRetry loop?

[dobby-coder]

Dobby sees the design proposal! This agent will dig into src/api/cryptify.ts, the existing withRetry loop, and the mid-stream error path, then come back with a focused review on (1) error classification across the fetch/stream boundary and (2) whether splicing-on-resume can be threaded more cleanly. Be back shortly.

[dobby-coder]

Design review

Read through src/api/cryptify.ts and src/util/retry.ts. The proposal lines up with what the code can do, but threading resume through the existing withRetry is the wrong shape — that's the core thing worth pushing back on, plus a few corner cases.

1. withRetry is the wrong abstraction for mid-stream resume

withRetry is (attempt) => Promise<T> and it returns once the promise resolves. For downloads, T is ReadableStream<Uint8Array> — by the time the loop has returned, the stream hasn't been read yet. Mid-stream failures happen after withRetry is gone. There is no clean way to re-enter the loop from the consumer's read() site without inverting control: either the consumer has to call back into the retry layer ("hey, I died at offset N, give me a new stream"), or the retry layer has to drive consumption itself. Both are uglier than not using withRetry at all.

The cleaner shape is to return a single outer ReadableStream whose source owns the retry loop, and treat each underlying GET as one attempt:

return new ReadableStream<Uint8Array>({
  async start(controller) {
    let received = 0;
    let attempt = 1;
    while (true) {
      try {
        const stream = attempt === 1
          ? await downloadFile(cryptifyUrl, uuid, signal)
          : await downloadRange(cryptifyUrl, uuid, received, signal);
        const reader = stream.getReader();
        for (;;) {
          const { value, done } = await reader.read();
          if (done) { controller.close(); return; }
          received += value.byteLength;
          controller.enqueue(value);
        }
      } catch (err) {
        if (signal?.aborted) { controller.error(err); return; }
        if (attempt >= retry.maxAttempts) { controller.error(err); return; }
        if (classifyCryptifyError(err, signal) === 'fail') { controller.error(err); return; }
        retry.onRetry?.({ attempt, maxAttempts: retry.maxAttempts, error: err, nextDelayMs });
        await sleep(nextDelayMs, signal);
        attempt++;
      }
    }
  },
});

This reuses classifyCryptifyError, sleep, and the backoff helper unchanged — so it's not a parallel implementation, just a different driver. withRetry itself can stay exactly as it is for chunk PUTs and finalize, where the existing shape fits.

2. tee() for the byte counter is overkill

Step 1 of the proposal teeing the consumer-facing stream is more machinery than needed. ReadableStream.tee() produces two backpressure-coupled streams that both yield every chunk — to drive a byte counter you'd have to actively drain one branch. With the source-owned retry loop above, the bytes flow through the source's own read() → enqueue() path, so the counter is just received += value.byteLength at the single point where data is forwarded. No tee, no transform stream, one source of truth for received.

3. Mid-stream error classification — likely already works

Step 3's "stream errors propagate to the caller without ever entering the loop" is true today because withRetry has already returned. Once the retry loop sits inside the source (per §1), errors thrown from reader.read() land in the same catch and go through classifyCryptifyError directly. Empirically, fetch's mid-stream failures surface as TypeError ("network error") or AbortError — both already classified correctly. I'd expect the existing classifier to need zero changes; worth confirming with a mocked-network test before adding any new branches.

4. 206 vs 200 on the resume request — the silent-rewind trap

Range: bytes={received}- is a request hint. A server (or intermediary that strips Range) is allowed to ignore it and return 200 OK with the full body from byte 0. If the SDK splices that onto an existing offset-N consumer, the file is corrupted with no error.

Hard requirement on the resume path: assert response.status === 206 and that Content-Range's start byte equals received. Anything else → throw, classify as fail (not retry — the server isn't going to start honouring Range on the next try). Cryptify's FileServer does the right thing today; the assertion is for misconfigured intermediaries (caching CDNs, corporate proxies).

Optional belt-and-braces: send If-Range: <ETag> from the first response. If the upstream object changed between attempts, the server returns 200 instead of 206 and the assertion above catches it. Cryptify objects are immutable post-finalize so this is theoretical, but it's a one-header cost.

5. Retry budget must persist across resumes

Acceptance criterion 3 ("retry budget bounded by RetryOptions") needs the attempt counter to be shared across resume attempts, not reset on each successful resume. Otherwise a flapping connection that delivers one chunk per attempt resumes forever. The pseudocode in §1 has attempt loop-scoped, which is the right behaviour — worth calling out explicitly in the design so the test for "persistent failure → exhausts budget" actually exercises it (e.g., let each attempt deliver some bytes, then fail).

6. downloadTimeoutMs is handshake-only — stalls are a separate problem

retry.ts:18-25 is explicit that downloadTimeoutMs only bounds the GET handshake. With resume in place, a stalled stream (TCP open, no bytes flowing) still hangs forever. If you want stall detection, that's an idle watchdog timer that resets on each enqueue — separate axis from downloadTimeoutMs. I'd leave it out of this issue and track it separately; otherwise the scope balloons.

7. Misc nits

• downloadFile reads response.body synchronously after !response.ok. The resume helper should mirror that and also surface Content-Range parsing errors as NetworkError (consistent with the rest of the file) so the classifier sees it correctly.
• The single existing caller (src/crypto/decrypt.ts:45) consumes the stream via WASM StreamUnsealer.new. Worth a manual smoke test that the unsealer doesn't choke on an internally-spliced stream — it shouldn't (chunks are chunks), but it's the only consumer so it's cheap to verify.
• AbortSignal: each underlying fetch needs the caller's signal directly (the source-owned loop in §1 already passes signal through). The current withRetry + withTimeout composition is unnecessary on the resume path because there's no per-handshake timeout to layer in (and adding one mid-stream would conflict with the stall-watchdog discussion above).

Summary

• Don't try to thread resume through withRetry; put the retry loop inside the outer ReadableStream source. Same primitives, right shape.
• Skip tee() — count bytes at the enqueue site.
• Classifier almost certainly works as-is for stream errors; verify with a mocked test rather than pre-emptively changing it.
• Strict 206 + Content-Range check on resume responses, fail-not-retry on 200.
• Share the attempt counter across resumes so the budget is real.
• Leave idle-stall detection for a separate issue.

Happy to prep the implementation against this shape if it lands well — say the word.

[github-actions]

🎉 This issue has been resolved in version 1.5.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀