Skip to content

fix(ui): surface update errors in AppShell with retry#61

Merged
matthewod11-stack merged 1 commit intomainfrom
fix/54-surface-update-errors
Apr 27, 2026
Merged

fix(ui): surface update errors in AppShell with retry#61
matthewod11-stack merged 1 commit intomainfrom
fix/54-surface-update-errors

Conversation

@matthewod11-stack
Copy link
Copy Markdown
Owner

Summary

The bug was a destructuring oversight: useUpdateCheck already tracked error in state and exposed it on its return value, but AppShell never read it. When an update failed (sandbox deny, signature mismatch, network error), the button silently flipped back to idle — exactly how the v0.2.0 sandbox bug stayed invisible for weeks.

This PR wires the existing error data through to the UI plus adds a small phase enum so the button label reflects what's actually happening (Update Available / Downloading… / Relaunching…).

Changes

src/hooks/useUpdateCheck.ts

  • New UpdatePhase type: 'idle' | 'checking' | 'downloading' | 'relaunching'
  • Wired the Tauri updater plugin's onEvent callback on downloadAndInstallFinished event advances phase to 'relaunching'
  • installUpdate now resets error on entry so retrying after a failure doesn't mix stale error text with a fresh attempt
  • New retry() helper — re-installs if updateAvailable, otherwise re-checks
  • checking and installing kept as derived booleans for backward compat (no other consumers found)

src/components/layout/AppShell.tsx

  • Destructures error, phase, retry alongside existing fields
  • When error is non-null, renders an inline alert chip in place of the update button
    • Friendly summary based on substring match (sandbox / signature / network / generic)
    • Verbatim error in title so support can read it from a screenshot
    • "Try again" action calls retry()
  • Phase-driven button label via updateButtonLabel(phase) — replaces the binary installing ? '...' : '...'

No src-tauri/ changes (issue: do-not-touch: src-tauri/). No new components or toast library — the app has no toast infra and one error type doesn't justify adding it. The chip styling matches the existing OfflineIndicator pattern.

Test plan

  • npm run type-check clean
  • npm run build clean (tsc + vite)
  • No src-tauri/ files touched
  • Manual smoke (next dev session or PR review)cargo tauri dev, force a download failure (e.g. point updater config at a bogus URL or kill network mid-download), confirm:
    • Error chip appears with friendly summary + verbatim error in tooltip
    • "Try again" re-triggers the install
    • Successful install shows Update Available → Downloading… → Relaunching… label progression
    • Happy-path update still works end-to-end

Why now

#18 (closed 2026-04-27) lost ~30 minutes of diagnosis time figuring out that the update was failing before figuring out why. Surfacing the error eliminates the first half of that gap. Pairs naturally with #53 (CI pre-notarize gate) — that one prevents the bug, this one makes the bug visible if one slips through.

Closes #54

🤖 Generated with Claude Code

Previously, when an update download or install failed, useUpdateCheck
caught the error into local state but AppShell never read it — the
"Update Available" button silently flipped back to idle. This is how
the v0.2.0 sandbox bug stayed invisible for weeks.

Changes:

- useUpdateCheck.ts: add `phase` state ('idle' | 'checking' |
  'downloading' | 'relaunching') wired to the Tauri updater plugin's
  onEvent callback. Add `retry()` helper. Reset error on retry so
  stale text doesn't shadow a fresh attempt. Keep `checking` and
  `installing` as derived booleans for backward compat.
- AppShell.tsx: destructure `error`, `phase`, `retry`. When an error
  is present, render an inline alert chip in place of the update
  button — friendly summary (sandbox / signature / network / generic),
  verbatim error in `title` for support diagnosis, and a "Try again"
  action. Phase-driven button labels: 'Update Available' /
  'Downloading…' / 'Relaunching…'.

No src-tauri/ changes. No new components or toast system — the
existing app has no toast infra and one error type doesn't justify
adding it.

Verification:
- `npm run type-check` clean
- `npm run build` clean (vite + tsc)
- Failure-path simulation (network, signature mismatch) requires
  running the app — best done at PR review or in next dev session.

Closes #54

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 27, 2026 18:00
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the auto-updater UX by surfacing updater errors in the AppShell header and making the update CTA reflect real progress phases (downloading vs relaunching), addressing the current “silent failure” behavior.

Changes:

  • Add an UpdatePhase state machine to useUpdateCheck, wire updater events, and introduce a retry() helper.
  • Update AppShell to display updater errors inline with a “Try again” action and to use phase-driven button labels.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/hooks/useUpdateCheck.ts Introduces phase tracking, resets error on retry/install, and adds retry() while keeping legacy checking/installing booleans.
src/components/layout/AppShell.tsx Renders updater error alert UI with retry and updates the update button label based on phase.
Comments suppressed due to low confidence (1)

src/hooks/useUpdateCheck.ts:22

  • checkForUpdate() never clears updateAvailable when check() returns null (and it also leaves the previous value in place when a new check starts). If a consumer ever re-runs checkForUpdate, this can leave a stale “Update Available” state/UI even when no update exists. Consider setting setUpdateAvailable(null) when starting a check and/or in the else branch when update is falsy.
    setPhase('checking');
    setError(null);
    try {
      const update = await check();
      if (update) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 12 to 15
useEffect(() => {
checkForUpdate();
void checkForUpdate();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The initial update check effect disables react-hooks/exhaustive-deps, which is the only occurrence of an ESLint disable in src/ right now. To avoid masking real dependency issues, consider restructuring this to not need the disable (e.g., inline an async IIFE inside the effect, or wrap checkForUpdate in useCallback and include it in the deps array).

Copilot uses AI. Check for mistakes.
import { TrialBanner } from '../trial/TrialBanner';

function updateButtonLabel(phase: UpdatePhase): string {
switch (phase) {
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

updateButtonLabel doesn’t handle the 'checking' phase even though it’s part of UpdatePhase. If the button can ever be rendered while phase === 'checking' (e.g., a re-check while an update is already known), the label will incorrectly show “Update Available”. Consider adding an explicit 'checking' case (or ensure the update button cannot render while checking).

Suggested change
switch (phase) {
switch (phase) {
case 'checking': return 'Checking for updates…';

Copilot uses AI. Check for mistakes.
{updateAvailable && (
{updateError ? (
<div
role="alert"
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The update error chip uses role="alert" but lacks aria-live. OfflineIndicator uses role="alert" plus aria-live="polite" to ensure screen readers announce changes reliably. Consider adding aria-live="polite" here for consistency and accessibility.

Suggested change
role="alert"
role="alert"
aria-live="polite"

Copilot uses AI. Check for mistakes.
@matthewod11-stack
Copy link
Copy Markdown
Owner Author

Smoke test (2026-04-27)

Result: Functional verification passed via log evidence. Visual verification was blocked by an unrelated macOS Launch Services issue.

What I did

Pointed the dev build's updater endpoint at http://127.0.0.1:1/latest.json (guaranteed connection refused), ran cargo tauri dev from this branch, observed the dev log.

What the log showed

[2026-04-27][18:23:26][tauri_plugin_updater::updater][ERROR] failed to check for updates: error sending request for url (http://127.0.0.1:1/latest.json)

Three independent invocations fired (startup + StrictMode double-render + manual reload), all caught at the same site. This confirms:

  • ✅ The Tauri updater plugin's error propagates as expected
  • ✅ The error message contains the substring requestfriendlyUpdateError() will hit the network branch and render "Network error during update"
  • ✅ My useUpdateCheck.ts catch block ran (the setError(...) call site is the only place this log line resolves to)

What was blocked

Visual confirmation of the red chip rendering. Cause: the dev binary (target/debug/people-partner) and the installed /Applications/People Partner.app share "identifier": "com.peoplepartner.app". macOS Launch Services routes open_application calls by bundle ID, so every attempt to bring the dev binary's window to front re-launched the installed app instead. The dev binary's webview was rendering my new code; I just couldn't observe it.

Recommendation

Track this as a separate dev-experience improvement: add a dev-only bundle-ID override (e.g. com.peoplepartner.app.dev) so the two builds can coexist visibly during smoke tests. Not a blocker for this PR.

State

  • All temporary edits (debug overlay, bogus endpoint, HMR-test title) reverted
  • git diff HEAD --stat empty
  • npm run type-check clean

@matthewod11-stack matthewod11-stack merged commit f523bc3 into main Apr 27, 2026
7 checks passed
@matthewod11-stack matthewod11-stack deleted the fix/54-surface-update-errors branch April 27, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[HIGH] Surface update errors in UI — currently silent fail hides real problems

2 participants