fix: show honest message when update check fails during release transition (#501)

Previously, user-initiated update checks during a GitHub release
transition sent 'not-available', falsely telling users they were on
the latest version. Now all benign check failures send 'idle', and the
toast controller converts user-initiated checking→idle into a neutral
"unable to check" message that covers both release transitions and
network blips honestly.
This commit is contained in:
Jinjing 2026-04-11 16:28:39 -07:00 committed by GitHub
parent 7cb57470f1
commit 40d431795f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 70 additions and 25 deletions

View File

@ -100,7 +100,7 @@ describe('updater check failure handling', () => {
vi.unstubAllGlobals()
})
it('treats GitHub release transition errors as not-available for user-initiated checks', async () => {
it('treats GitHub release transition errors as idle for user-initiated checks', async () => {
autoUpdaterMock.checkForUpdates.mockResolvedValueOnce(undefined).mockImplementationOnce(() => {
autoUpdaterMock.emit('checking-for-update')
queueMicrotask(() => {
@ -120,11 +120,18 @@ describe('updater check failure handling', () => {
const statuses = sendMock.mock.calls
.filter(([channel]) => channel === 'updater:status')
.map(([, status]) => status)
expect(statuses).toContainEqual({ state: 'not-available', userInitiated: true })
// Why: release transition failures should NOT pretend the user is up to
// date. Sending 'idle' lets the toast controller show an honest
// "currently rolling out" message instead of the misleading "you're on the
// latest version" that auto-dismisses.
expect(statuses).toContainEqual({ state: 'idle' })
expect(statuses).not.toContainEqual(
expect.objectContaining({ state: 'not-available', userInitiated: true })
)
})
})
it('treats missing latest-mac.yml during user-initiated checks as not-available', async () => {
it('treats missing latest-mac.yml during user-initiated checks as idle', async () => {
autoUpdaterMock.checkForUpdates.mockResolvedValueOnce(undefined).mockImplementationOnce(() => {
autoUpdaterMock.emit('checking-for-update')
queueMicrotask(() => {
@ -149,7 +156,10 @@ describe('updater check failure handling', () => {
const statuses = sendMock.mock.calls
.filter(([channel]) => channel === 'updater:status')
.map(([, status]) => status)
expect(statuses).toContainEqual({ state: 'not-available', userInitiated: true })
expect(statuses).toContainEqual({ state: 'idle' })
expect(statuses).not.toContainEqual(
expect.objectContaining({ state: 'not-available', userInitiated: true })
)
})
})
})

View File

@ -9,12 +9,7 @@ import {
markMacQuitAndInstallInFlight
} from './updater-mac-install'
import { registerAutoUpdaterHandlers } from './updater-events'
import {
compareVersions,
isBenignCheckFailure,
isGitHubReleaseTransitionFailure,
statusesEqual
} from './updater-fallback'
import { compareVersions, isBenignCheckFailure, statusesEqual } from './updater-fallback'
const AUTO_UPDATE_CHECK_INTERVAL_MS = 36 * 60 * 60 * 1000
const AUTO_UPDATE_RETRY_INTERVAL_MS = 60 * 60 * 1000
@ -112,22 +107,25 @@ async function sendCheckFailureStatus(message: string, userInitiated?: boolean):
const handleFailure = async (): Promise<void> => {
if (isBenignCheckFailure(message)) {
// Release transition failures (missing latest.yml during publishing) and
// network blips are transient. For user-initiated checks during a release
// transition, show "up to date" so the action doesn't appear to silently
// do nothing; for background checks, silently retry later.
if (userInitiated && isGitHubReleaseTransitionFailure(message.toLowerCase())) {
clearAvailableUpdateContext()
persistLastUpdateCheckAt?.(Date.now())
sendStatus({ state: 'not-available', userInitiated: true })
return
}
// Why: release transition failures (missing latest.yml while a new
// release is being published) and network blips are transient. The
// previous approach sent 'not-available' for user-initiated checks
// during a release transition, which falsely told the user "you're
// on the latest version" — the toast would flash and auto-dismiss,
// hiding the fact that a newer release is mid-publish. Now all
// benign failures go to 'idle' uniformly: the toast controller
// converts a user-initiated checking→idle transition into an honest
// "currently rolling out" message, and a background retry is
// always scheduled so the update notification arrives once the
// release finishes.
console.warn('[updater] benign check failure:', message)
clearAvailableUpdateContext()
if (!userInitiated) {
scheduleAutomaticUpdateCheck(AUTO_UPDATE_RETRY_INTERVAL_MS)
}
scheduleAutomaticUpdateCheck(AUTO_UPDATE_RETRY_INTERVAL_MS)
// Why: we intentionally do NOT call persistLastUpdateCheckAt here.
// The check didn't truly complete (the manifest was unreachable due
// to a release transition or network blip), so recording a timestamp
// would suppress the next startup check and delay discovery of the
// new version.
sendStatus({ state: 'idle' })
return
}

View File

@ -159,6 +159,34 @@ describe('createUpdateToastController', () => {
expect(toastApi.info).toHaveBeenCalledTimes(1)
})
it('replaces checking toast with an unable-to-check message when a user-initiated check goes idle', () => {
const toastApi = createToastApi()
toastApi.loading.mockReturnValue('checking-toast')
const updaterApi = createUpdaterApi()
const storeApi = createStoreApi()
const controller = createUpdateToastController({ toastApi, updaterApi, storeApi })
controller.handleStatus({ state: 'checking', userInitiated: true })
controller.handleStatus({ state: 'idle' })
expect(toastApi.info).toHaveBeenCalledWith(
"Unable to check for updates right now. We'll try again shortly.",
{ id: 'checking-toast' }
)
})
it('does not show a rolling-out message for background checks that go idle', () => {
const toastApi = createToastApi()
const updaterApi = createUpdaterApi()
const storeApi = createStoreApi()
const controller = createUpdateToastController({ toastApi, updaterApi, storeApi })
controller.handleStatus({ state: 'checking' })
controller.handleStatus({ state: 'idle' })
expect(toastApi.info).not.toHaveBeenCalled()
})
it('calls dismissUpdate when the user closes the available toast without updating', () => {
const toastApi = createToastApi()
toastApi.info.mockReturnValue('available-toast')

View File

@ -69,7 +69,16 @@ export function createUpdateToastController(deps?: {
checkingToastId = toastApi.loading('Checking for updates...')
} else if (status.state === 'idle') {
if (checkingToastId) {
toastApi.dismiss(checkingToastId)
// Why: a user-initiated check that resolves to idle (rather than
// 'not-available') means the check couldn't complete — this can
// happen because a new release is being published and the update
// manifest is temporarily unavailable, or because of a transient
// network failure. We use a neutral message that is honest for
// both scenarios instead of claiming a rollout is in progress
// (misleading when the real cause is a network blip).
toastApi.info('Unable to check for updates right now. We\'ll try again shortly.', {
id: checkingToastId
})
checkingToastId = undefined
}
} else if (status.state === 'not-available') {