Map update-check failures to Error, not Idle (SBS-931) - #327
Conversation
A GitHub outage, rate-limit, or unreadable release payload was treated as "no newer release", so About said the user was current. Idle is only a successful current check now.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
ceiling | 74421e3 | Commit Preview URL Branch Preview URL |
Aug 18 2026, 02:23 AM |
|
Warning Review limit reached
Next review available in: 21 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Automated reviewNew in this pass: 3 issues.
Resolved since the previous pass: 6. For coding agents: fix BLOCK and FIX IF QUICK findings now; everything else is tracked or informational; never exceed one CodeRev fix round per PR. Advisory. Findings generated by |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Four things the review found, all in how a failed update is reported. An empty beta listing was an error. A repository with nothing published yet, or only drafts, is a successful "no release to offer", and calling it a failure put "Could not check for updates" on About for a channel that was simply empty. It now reads as Idle. About prefixed every error with the check-failure sentence. Download and apply set their own errors, so a hash mismatch or a rejected signature read as a failed check, and a real check failure read the sentence twice. The backend message now stands alone, with the prefix kept only as the fallback for an error with no message of its own. That message was English glued onto a localized prefix. The three check-failure sentences are now locale keys in both languages, so the whole line is translated rather than half of it. A failed check no longer wipes a downloaded update off the screen. The catch reset the surface to idle-plus-error, which took Install and Restart away from an installer the backend still held on disk.
The three update-error keys were added to locale.rs and both .ftl files but not to the frontend key list, which the build cross-checks. That is what failed the Frontend job. The PTY test asked for a bare "cmd". That resolves through the enriched PATH, which puts the npm directory ahead of System32, so a machine carrying an extensionless npm shim named `cmd` launched a shell script through CreateProcessW and failed with "not a valid Win32 application". It now names the shell by COMSPEC, which is what the test meant. This is the failure that has been red on this branch and intermittently elsewhere; the shared suite is green with it.
Three from the latest review. The timeout was the one check failure still hardcoded in English while its neighbours went through the locale. It has a key now, in both languages. The PTY test needed an absolute shell on Windows to dodge an npm shim, but making the Unix branch absolute too traded one machine-specific assumption for another: sh is not always at /bin/sh. Unix goes back to the PATH lookup. UpdateCheckError::Empty had no constructor left once an empty beta listing stopped being an error, so it is gone rather than sitting there implying a state that cannot happen.
…eck-idle # Conflicts: # CHANGELOG.md
1 participant
Summary
check_for_updates_with_channelnow returnsResult<Option<UpdateInfo>, UpdateCheckError>.Ok(None)is only a successful latest-is-not-newer. GitHub HTTP errors, transport failures, client-build failures, and unreadable JSON areErr.Err(and the existing 15s timeout) toUpdateState::Error. Idle is no longer the collapse for we-could-not-tell.AboutUpdateCheckFailed(Could not check for updates.) on Error, not You're up to date. A swallowed invoke failure incheckNowis also Error, so idle+hasChecked cannot claim current.A user who opens Settings, About, clicks Check for updates, and hits a GitHub 403 / outage / unreadable payload now sees Could not check for updates, not You're up to date.
Fixes SBS-931. Not GH 269 (update a11y).
Test plan
Required CI gate (from .github/workflows/ci.yml)
Rust / shared (windows-latest): cargo fmt --all --check, cargo test --manifest-path rust/Cargo.toml, cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings
Ran here on Linux:
New SBS-931 tests passed: github_http_error_is_not_a_current_release, unreadable_release_json_is_not_a_current_release, successful_same_version_is_current_not_an_error, successful_newer_release_is_available
The 6 failures are pre-existing Windows-path / project-name assertions (CI job is windows-latest; this box is Linux).
Rust / desktop (windows-latest): cargo test + cargo clippy on apps/desktop-tauri/src-tauri/Cargo.toml
Could not compile here: glib-2.0 is not installed (Tauri GTK). Desktop mapping tests (failed_github_check_is_error_not_idle, successful_current_release_is_idle, ready_check_is_skipped_so_install_affordance_stays) will run on the Windows CI job.
Frontend: package-manager binary was Auto-review blocked here. AboutTab + useUpdateState tests are in the diff.
Fail-without-fix
Reverted only interpret_release_http to the old fail-open (HTTP/parse to dummy current release / Idle). The new tests failed:
Restored the production change. Those tests pass with the fix.
Pattern sweep
rg UpdateState::Idle, status === idle, Ok(None), hasChecked
What this change makes more likely
Error will fire on every GitHub blip, including the 2s startup check. The update banner can now appear at launch when GitHub is down (it previously stayed hidden on Idle). That is the intended direction: unknown is not current. Ready is no longer re-checked, so a user who already has an installer cannot discover a newer newer release until they dismiss. That is the tradeoff that keeps Install and Restart.
Gaps
Not merged.
Note
Map update-check failures to
Errorstate instead ofIdlein the updaterIdle, incorrectly reporting the app as up to date.check_for_updates_with_channelin updater.rs now returnsResult<Option<UpdateInfo>, UpdateCheckError>, propagating failures as typed errors with localized user messages.state_from_check_resulthelper in updater.rs maps the result toUpdateState::Error(with message) on failure,Idleonly on a confirmed "no newer release", andAvailableon a newer release.AboutTabnow displays the specific backend error message directly, falling back to a generic "check failed" string when the message is empty.Macroscope summarized 74421e3.