Skip to content

Downloader: surface download failures, add timeouts, share client, stream to disk #51

Description

@lmorchard

Found during a code-review pass (2026-07-23). Related to #14.

The media downloader silently ignores failures and lacks basic hardening, so fetch/build can report success while media is missing or corrupt.

  • src/downloader.rs run() harvests finished workers with workers.join_next() inside tokio::select! and discards the result entirely — both JoinError (panic) and the inner Result. A failed download is never observed, logged, or surfaced; run() returns Ok(()) regardless.
  • (Fixed already: HTTP status is now checked via error_for_status() before writing the body.)
  • No request/connect timeout on any client, so a stalled server hangs a worker slot indefinitely and can prevent the run loop from exiting.
  • A new reqwest::Client is built per download task, defeating connection pooling / TLS reuse.
  • The whole response body is buffered into memory (response.bytes() + Cursor) and written with blocking std::io::copy on a tokio worker thread — large files stall the runtime and spike RAM.

Suggested fix

  • Inspect each join_next() result; log/collect errors and return a failure summary (or error) from run().
  • Build one Client with .timeout(..)/.connect_timeout(..) and clone it into workers.
  • Stream the body to disk (tokio::fs + bytes_stream()), or wrap the blocking write in spawn_blocking.

Severity: high for the silent-failure part; medium for the perf/robustness parts.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingenhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions