fix(client): ignore NaN in setDownloadProgress instead of storing it#5070
Open
jeffrey701 wants to merge 1 commit into
Open
fix(client): ignore NaN in setDownloadProgress instead of storing it#5070jeffrey701 wants to merge 1 commit into
jeffrey701 wants to merge 1 commit into
Conversation
setDownloadProgress documents a 0–100 clamp, but NaN slips through:
Math.max/Math.min propagate NaN, and the `downloadProgress === clamped` change
guard never fires (NaN !== NaN), so NaN is stored and pushed to listeners. The
BuildBadge UI then renders "NaN%" and style={{ width: "NaN%" }} (invalid CSS),
and every subsequent NaN re-notifies. NaN is exactly the receivedBytes /
totalBytes result when a download reports no content-length (totalBytes === 0),
which is why the Tauri updater caller already had to add a totalBytes > 0
workaround — the utility should defend its own contract.
Add a Number.isNaN early return so the last valid progress is kept. ±Infinity
still clamp correctly to 100 / 0. Adds the module's first test, covering the
clamp, ±Infinity, and the NaN case (which fails without the guard).
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
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.
Problem
setDownloadProgress(client/src/pwa/updateStatus.ts) documents a 0–100 clamp, butNaNescapes it:Math.max/Math.minpropagateNaN, and thedownloadProgress === clampedchange guard never trips (NaN !== NaN), soNaNis stored and pushed to listeners.BuildBadgethen renders"NaN%"andstyle={{ width: "NaN%" }}(invalid CSS), and each subsequentNaNre-notifies every listener.NaNis exactly thereceivedBytes / totalBytes * 100result when a download reports no content-length —tauriUpdater.tsreadsevent.data.contentLength ?? 0, sototalBytescan be 0. That caller already had to add atotalBytes > 0workaround; the utility should defend its own documented contract rather than rely on every caller to guard.Fix
A
Number.isNaN(value)early return that keeps the last valid progress. Every finite value and±Infinitystill clamp correctly (Infinity→100,-Infinity→0) — onlyNaNis rejected.Tests
Adds the module's first test: the clamp/rounding,
±Infinity, and theNaNcase (which fails without the guard — discriminating, seeded with a known value so it is immune to singleton state).tsc -b --noEmitandeslintclean.Self-contained: only
updateStatus.ts+ a new test; no engine/Rust/protocol changes.Model: claude-opus-4-8