fix(upgrade): stream prebuilt-download with progress so it can't silent-hang (#284.3)#301
Merged
Merged
Conversation
5 tasks
093c023 to
206a3c2
Compare
Deploying wireup-landing with
|
| Latest commit: |
250beb0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5923e661.wireup-landing.pages.dev |
| Branch Preview URL: | https://fix-284-3-wire-upgrade-prebu.wireup-landing.pages.dev |
119f3fb to
3af2a2f
Compare
…nt-hang (#284.3) Issue #284 part 3 (from Willard's Windows report). With no `cargo` on PATH, `wire upgrade` correctly falls through to its prebuilt-release download path. But the download itself called `resp.bytes()`, which blocks until the entire body lands — silently. On Willard's Windows host that looked like a hard hang: no stderr, no progress, the binary's mtime unchanged, and yet `crates.io` + `github.com` were both reachable. The operator had no way to tell whether the request was making forward progress, blocked on TLS, or wedged on something else, and bypassed the upgrade entirely with a manual `curl` + SHA verify. The download now streams in 64 KiB chunks: - 500ms-throttled `\rwire upgrade: downloaded N / TOTAL bytes (PCT%)` line on stderr while bytes arrive — operator sees progress (or sees it stall) instead of a frozen terminal. - Falls back to `downloaded N bytes (unknown size)` when the server omits Content-Length, or when it advertises `Content-Length: 0` (degenerate but a real response shape — guards against divide-by-zero in the percent calc). - On overshoot (Content-Length advisory, server sent more) the percent clamps at 100% instead of rendering `120%`. - Final line is replaced with `downloaded N bytes in <duration>` so the operator has a clean record of how long the download took. - Existing 120s wall-clock timeout on the blocking reqwest client stays as the hard backstop. Pure-logic `format_download_progress(got, expected)` is extracted so the format is unit-testable without touching reqwest. Tests: 4 new in upgrade::tests, all green on `x86_64-pc-windows-msvc` (rustc 1.96.0): - format_download_progress_includes_percent_when_total_known - format_download_progress_clamps_percent_at_100_when_overshoot - format_download_progress_falls_back_to_unknown_size_when_total_missing - format_download_progress_falls_back_to_unknown_size_when_total_zero Full lib suite: 490 passed; 0 failed; 7 ignored. Out of scope: switching to a hard total-wall-clock timeout enforced by a watchdog thread (so a wedged TLS read can't ride past the reqwest timeout in pathological cases). Today's 120s reqwest timeout + visible progress is the smallest fix that turns "silent hang" into "observable progress or loud failure". A watchdog-thread variant would be a follow-up if Windows reqwest 120s turns out to leak in practice. Stacks on top of #294 (Windows test/clippy hygiene); rebase onto main once #294 lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: laul.pogan <paul@zaibatsuheavy.industries>
3af2a2f to
250beb0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 3 of #284 (Willard's Windows report).
Summary
With no `cargo` on PATH, `wire upgrade` correctly falls through to its prebuilt-release download path. But the download itself called `resp.bytes()`, which blocks until the entire body lands — silently. On Willard's Windows host that looked like a hard hang: zero stderr, no progress, the binary's mtime unchanged, and yet `crates.io` + `github.com` were both reachable. The operator had no way to tell whether the request was making forward progress, blocked on TLS, or wedged on something else, and bypassed the upgrade entirely with a manual `curl` + SHA verify (which is exactly what this code path was supposed to remove the need for).
Fix
Stream the download in 64 KiB chunks and emit progress to stderr at most every 500ms:
```
wire upgrade: downloaded 6291456 / 12308480 bytes (51%)
…
wire upgrade: downloaded 12308480 / 12308480 bytes (100%) in 4.218s
```
Edge cases covered:
The existing 120s wall-clock timeout on the blocking reqwest client stays as the hard backstop. No new dep.
Pure-logic
`format_download_progress(got: u64, expected: Option) -> String` is split out so the format is unit-testable without touching reqwest.
Tests
4 new in `upgrade::tests`, all green on `x86_64-pc-windows-msvc` (rustc 1.96.0):
Full lib suite: 490 passed; 0 failed; 7 ignored.
Out of scope
Switching to a hard total-wall-clock timeout enforced by a watchdog thread (so a wedged TLS read can't ride past the reqwest timeout in pathological cases) is a follow-up if Windows reqwest 120s turns out to leak in practice. Today's 120s reqwest timeout + visible progress is the smallest fix that turns "silent hang" into "observable progress or loud failure"; that's enough to close the operator-experience hole.
Stack
Stacks on top of #294 (Windows test/clippy hygiene). Sibling Windows-cluster PRs already open: #296 (#284.5), #297 (#247.4), #298 (#284.1), #300 (#284.2).
Test plan
🤖 Generated with Claude Code