Skip to content

fix(updater): suppress automatic check failure banners - #690

Open
Salah-Eddine Lachkar (artpumpkin) wants to merge 1 commit into
darkmatter:mainfrom
artpumpkin:fix/quiet-automatic-update-check
Open

Salah-Eddine Lachkar (artpumpkin) wants to merge 1 commit into
darkmatter:mainfrom
artpumpkin:fix/quiet-automatic-update-check

Conversation

@artpumpkin

Copy link
Copy Markdown

Summary

Fixes #657. An automatic update-check failure now logs a local warning and emits update_check_failed with the fixed reason code check_failed, without showing a red banner. Raw error strings and manifest URLs are not sent in this telemetry event. Update installation failures retain their existing banner and telemetry.

Removes the dead check-error banner branch. Manual checking/settings changes remain outside this issue.

Test Plan

  • Four hook tests pass with import.meta.env.DEV stubbed false: network, HTTP and malformed-manifest check rejections stay out of banner state; an install rejection retains its error. Tests also check warning output and telemetry URL exclusion.
  • Three check-failure tests failed on the original code before the fix.
  • TypeScript checking passes; targeted lint reports zero warnings/errors; git diff --check passes.
  • Native release-build verification: launch offline and confirm no global error banner; verify a failed installation still surfaces its error. Native updater calls are mocked in the hook tests, and no real update was installed during validation.

Docs

  • No docs update needed.

Implementation and local validation were AI-assisted.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-13T14:16:33.729076Z 66e13fc PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@prelint

prelint Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Ship with changes Silent automatic update-check failures

Product decisions in this change

Agree 1. When the launch-time update check fails, the user sees no error banner. The app writes a local warning only.

A failed best-effort check at launch gives the user nothing to act on. A red global banner on every offline launch is noise and makes the app look broken. Most desktop apps with auto-update, such as Slack, VS Code and Sparkle-based apps, fail silently on background checks and show errors only for user-initiated checks. The install failure keeps its banner, because the user chose to install and expects an outcome.

The approach is correct as long as no user-initiated check shares this path. The hook still exposes a check function. If a future settings button calls it, a failed manual check would also be silent, and that would confuse the user. The PR says manual checking is out of scope, and I found no current UI caller of the check function. The risk is low today, but the follow-up must split the two paths.

Agree with concerns 2. The failure event carries one fixed reason code (`check_failed`) and never the raw error or manifest URL.

Keeping manifest URLs out of telemetry is the right privacy call. The cost is that every failure looks the same. The team cannot tell a brief network drop from a persistent HTTP error or a broken manifest. A cohort whose network blocks the manifest URL would look the same as users who launch offline.

Option What it gives users What it costs Effort to change later
Single code (current) Strong privacy, simple No way to diagnose or segment failures Low. Add codes later, but old data stays undifferentiated
Coarse categories (network, http, parse) Same privacy, actionable signal A small mapping step Low now, same later
Raw error text Full detail Leaks private URLs Hard to undo once collected

Coarse categories keep the privacy guarantee and cost little. I recommend them now or a firm commitment to a follow-up. This concern from the earlier review is still open.

Agree with concerns 3. A user whose update check fails every time has no visible signal that updates are broken. Neither the user nor the team gets a clear signal.

With no banner and a single-value event, a cohort that is permanently blocked (for example by an organisation firewall) will silently stay on old versions. That cohort will also miss security fixes. The PR does not say how the team will find this group.

A low-cost answer is to count consecutive failures and show a quiet notice in settings, or to include a failure streak in telemetry. The team could also watch the rate of update_check_failed against successful checks. This is not a blocker, but the team needs to decide who watches the signal.

Agree 4. Install failures keep the banner and now always read "Update install failed". The check-error wording is removed.

The user started the install, so a visible failure is expected. Removing the dead check-error wording simplifies the banner and avoids a state that can no longer occur.

Since the last review

  • Still open: Telemetry uses a single fixed reason code, so failure types cannot be told apart. (The diff still emits only { reason: "check_failed" } and the type allows no other value.)
  • Addressed by explanation: Whether a manual check from settings would go through the same silent path. (The PR description says manual checking is outside this issue, and no settings UI calls the check function today.)
  • Still open: No in-product way to discover persistent check failures for a cohort. (The diff adds no notice, streak count or categorized signal. Only a local console warning remains.)
  • Still open: Unclear whether the single reason code is a placeholder or a permanent design. (The PR description does not say whether categorization will follow.)

Open questions

  • Is check_failed a placeholder for categorized reason codes in a follow-up, or a permanent choice?

  • When a manual update check is added, will it use a separate path that shows failures to the user?

  • Who will watch the rate of update_check_failed events, and what rate would trigger action?

Recommendation

Ship with changes
Hiding the launch-time banner is the right user experience. The single-code telemetry and missing discovery path for persistent failures leave the team blind to a permanently blocked cohort. Adding coarse categories, or committing to them, would close the gap.

@prelint prelint Bot added the ship it label Sep 13, 2026
@sonarqubecloud

Copy link
Copy Markdown

@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; no actionable issues were found.

Summary

Automatic update-check failures now log a local warning without showing a red banner.

  • Automatic update-check failures stay out of the banner.

Reviews (2) · Last reviewed commit: "fix(updater): keep automatic check failu..." · Reviewed by Greptile

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@arximboldi

Copy link
Copy Markdown
Contributor

Hi Salah-Eddine Lachkar (@artpumpkin)! Thank you for your contribution. However the "hard part" of this task is the manual verification in the step four of the test plan: "Native release-build verification: launch offline and confirm no global error banner; verify a failed installation still surfaces its error. Native updater calls are mocked in the hook tests, and no real update was installed during validation."

Can you please run this verification, providing some explanation on how this was run and some video as proof?

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update-check failure (offline / unreachable manifest) shows as a red error banner on every step

2 participants