Skip to content

fix(auto-updater): throw error on update check failure instead of returning result.#1213

Open
leozhang2018 wants to merge 4 commits into
getpaseo:mainfrom
leozhang2018:main
Open

fix(auto-updater): throw error on update check failure instead of returning result.#1213
leozhang2018 wants to merge 4 commits into
getpaseo:mainfrom
leozhang2018:main

Conversation

@leozhang2018
Copy link
Copy Markdown

Type of change

  • Bug fix
  • New feature (with prior issue + design alignment)
  • Refactor / code improvement
  • Docs

What does this PR do

Fixes desktop update checks reporting updater failures as “up to date.”

Previously, when electron-updater.checkForUpdates() failed, for example because the network was unavailable or the GitHub update manifest could not be fetched/parsed, the desktop main process caught the error and returned a normal “no update” result. That caused the renderer to show App is up to date. even though the check had failed.

This PR rethrows the updater check error after logging it, allowing the existing renderer error handling to show the update check as failed for manual checks. Silent background checks still only warn and preserve the current state.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 29, 2026

Greptile Summary

Fixes a silent-failure bug where a thrown error inside checkForAppUpdate was caught, logged, and converted into a fake "no update" result, causing the renderer to display "App is up to date" even when the check had failed. The fix is a five-line reduction: the catch block now rethrows so callers receive the real error.

  • auto-updater.ts: The catch block that previously returned buildCheckResult({ hasUpdate: false, readyToInstall: false }) now calls throw error, preserving the error for the caller to handle.
  • auto-updater.test.ts: Refactors module-level mocks to use vi.hoisted (required for Vitest to reference mock objects in vi.mock factory closures) and adds a new describe(\"checkForAppUpdate\") suite with a test that asserts the promise rejects when autoUpdater.checkForUpdates rejects.

Confidence Score: 5/5

Safe to merge — the change is a targeted five-line removal that eliminates a misleading swallow-and-return pattern in the updater catch block.

Both modified files are narrow in scope: the production change removes a single synthetic return in a catch block and lets the error propagate naturally, while the test change adds a new assertion that would catch any regression back to the old behavior. The call site in daemon-manager.ts propagates errors through the IPC layer to the renderer, which the PR description confirms already handles them correctly.

No files require special attention.

Important Files Changed

Filename Overview
packages/desktop/src/features/auto-updater.ts Catch block changed from returning synthetic "no update" result to rethrowing the error — a minimal, targeted fix for the silent-failure bug.
packages/desktop/src/features/auto-updater.test.ts Restructures mocks to use vi.hoisted so they can be referenced in vi.mock factories, and adds a new test asserting checkForAppUpdate rejects on updater failure.

Sequence Diagram

sequenceDiagram
    participant Renderer
    participant DaemonManager
    participant checkForAppUpdate
    participant electronUpdater as electron-updater

    Renderer->>DaemonManager: check_app_update (IPC)
    DaemonManager->>checkForAppUpdate: checkForAppUpdate(...)
    checkForAppUpdate->>electronUpdater: autoUpdater.checkForUpdates()

    alt updater succeeds
        electronUpdater-->>checkForAppUpdate: UpdateCheckResult
        checkForAppUpdate-->>DaemonManager: AppUpdateCheckResult
        DaemonManager-->>Renderer: "{ hasUpdate, ... }"
    else updater throws (network / manifest failure)
        electronUpdater--xcheckForAppUpdate: throws Error
        Note over checkForAppUpdate: console.error(...) then rethrow
        checkForAppUpdate--xDaemonManager: throws Error
        DaemonManager--xRenderer: IPC error
        Note over Renderer: Renderer error handler shows check failed
    end
Loading

Reviews (5): Last reviewed commit: "Merge branch 'main' into main" | Re-trigger Greptile

Comment thread packages/desktop/src/features/auto-updater.ts
…urning result.

Signed-off-by: leozhang2018 <leozhang2018@gmail.com>
Signed-off-by: leozhang2018 <leozhang2018@gmail.com>
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.

1 participant