fix(updater): surface user-initiated check failures when GitHub is down (#1181)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson
GitHub
parent
7f6d42134d
commit
3edb2f3ea2
|
|
@ -107,7 +107,7 @@ describe('updater check failure handling', () => {
|
|||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
it('treats GitHub release transition errors as idle for user-initiated checks', async () => {
|
||||
it('surfaces GitHub release transition errors to user-initiated checks', async () => {
|
||||
autoUpdaterMock.checkForUpdates.mockResolvedValueOnce(undefined).mockImplementationOnce(() => {
|
||||
autoUpdaterMock.emit('checking-for-update')
|
||||
queueMicrotask(() => {
|
||||
|
|
@ -127,18 +127,22 @@ describe('updater check failure handling', () => {
|
|||
const statuses = sendMock.mock.calls
|
||||
.filter(([channel]) => channel === 'updater:status')
|
||||
.map(([, status]) => status)
|
||||
// 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' })
|
||||
// Why: a user-initiated benign failure must show a visible error. Silently
|
||||
// sending 'idle' (or 'not-available') makes the button look broken.
|
||||
expect(statuses).toContainEqual(
|
||||
expect.objectContaining({
|
||||
state: 'error',
|
||||
userInitiated: true,
|
||||
message: expect.stringContaining('GitHub may be temporarily unavailable')
|
||||
})
|
||||
)
|
||||
expect(statuses).not.toContainEqual(
|
||||
expect.objectContaining({ state: 'not-available', userInitiated: true })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
it('treats missing latest-mac.yml during user-initiated checks as idle', async () => {
|
||||
it('surfaces missing latest-mac.yml to user-initiated checks', async () => {
|
||||
autoUpdaterMock.checkForUpdates.mockResolvedValueOnce(undefined).mockImplementationOnce(() => {
|
||||
autoUpdaterMock.emit('checking-for-update')
|
||||
queueMicrotask(() => {
|
||||
|
|
@ -163,10 +167,43 @@ describe('updater check failure handling', () => {
|
|||
const statuses = sendMock.mock.calls
|
||||
.filter(([channel]) => channel === 'updater:status')
|
||||
.map(([, status]) => status)
|
||||
expect(statuses).toContainEqual({ state: 'idle' })
|
||||
expect(statuses).toContainEqual(
|
||||
expect.objectContaining({
|
||||
state: 'error',
|
||||
userInitiated: true,
|
||||
message: expect.stringContaining('GitHub may be temporarily unavailable')
|
||||
})
|
||||
)
|
||||
expect(statuses).not.toContainEqual(
|
||||
expect.objectContaining({ state: 'not-available', userInitiated: true })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
it('silently drops background benign failures to idle', async () => {
|
||||
// Why: background checks must stay quiet; only user-initiated clicks get
|
||||
// an error card. This prevents noisy nag during a release transition.
|
||||
autoUpdaterMock.checkForUpdates.mockImplementationOnce(() => {
|
||||
autoUpdaterMock.emit('checking-for-update')
|
||||
queueMicrotask(() => {
|
||||
autoUpdaterMock.emit('error', new Error('Unable to find latest version on GitHub'))
|
||||
})
|
||||
return Promise.reject(new Error('Unable to find latest version on GitHub'))
|
||||
})
|
||||
|
||||
const sendMock = vi.fn()
|
||||
const mainWindow = { webContents: { send: sendMock } }
|
||||
|
||||
const { setupAutoUpdater, checkForUpdates } = await import('./updater')
|
||||
|
||||
setupAutoUpdater(mainWindow as never)
|
||||
checkForUpdates()
|
||||
await vi.waitFor(() => {
|
||||
const statuses = sendMock.mock.calls
|
||||
.filter(([channel]) => channel === 'updater:status')
|
||||
.map(([, status]) => status)
|
||||
expect(statuses).toContainEqual({ state: 'idle' })
|
||||
expect(statuses).not.toContainEqual(expect.objectContaining({ state: 'error' }))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -182,7 +182,7 @@ describe('updater', () => {
|
|||
expect(errorStatuses).toEqual([{ state: 'error', message: 'boom', userInitiated: true }])
|
||||
})
|
||||
|
||||
it('treats net::ERR_FAILED during checks as a benign idle transition', async () => {
|
||||
it('surfaces net::ERR_FAILED to user-initiated checks with a friendly message', async () => {
|
||||
autoUpdaterMock.checkForUpdates.mockResolvedValueOnce(undefined).mockImplementationOnce(() => {
|
||||
autoUpdaterMock.emit('checking-for-update')
|
||||
queueMicrotask(() => {
|
||||
|
|
@ -202,7 +202,13 @@ describe('updater', () => {
|
|||
const statuses = sendMock.mock.calls
|
||||
.filter(([channel]) => channel === 'updater:status')
|
||||
.map(([, status]) => status)
|
||||
expect(statuses).toContainEqual({ state: 'idle' })
|
||||
expect(statuses).toContainEqual(
|
||||
expect.objectContaining({
|
||||
state: 'error',
|
||||
userInitiated: true,
|
||||
message: expect.stringContaining('GitHub may be temporarily unavailable')
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
const statuses = sendMock.mock.calls
|
||||
|
|
@ -210,7 +216,8 @@ describe('updater', () => {
|
|||
.map(([, status]) => status)
|
||||
|
||||
expect(statuses).toContainEqual({ state: 'checking', userInitiated: true })
|
||||
expect(statuses).toContainEqual({ state: 'idle' })
|
||||
// Why: the raw electron-updater message is replaced with a user-friendly
|
||||
// one so we never surface "net::ERR_FAILED" directly to the UI.
|
||||
expect(statuses).not.toContainEqual(
|
||||
expect.objectContaining({ state: 'error', message: 'net::ERR_FAILED' })
|
||||
)
|
||||
|
|
|
|||
|
|
@ -202,25 +202,24 @@ async function sendCheckFailureStatus(message: string, userInitiated?: boolean):
|
|||
const handleFailure = async (): Promise<void> => {
|
||||
if (isBenignCheckFailure(message)) {
|
||||
// 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.
|
||||
// release is being published) and network blips are transient. Schedule
|
||||
// a background retry so the notification arrives once the release
|
||||
// finishes, and intentionally skip persistLastUpdateCheckAt — the check
|
||||
// didn't truly complete, and recording a timestamp would suppress the
|
||||
// next startup check.
|
||||
console.warn('[updater] benign check failure:', message)
|
||||
clearAvailableUpdateContext()
|
||||
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' })
|
||||
if (userInitiated) {
|
||||
// Why: a user-initiated click expects visible feedback — silently
|
||||
// dropping to 'idle' makes the button look broken. The card already
|
||||
// prefixes "Could not check for updates." and Settings prefixes
|
||||
// "Update check failed.", so the message here only carries the
|
||||
// actionable cause.
|
||||
sendErrorStatus('GitHub may be temporarily unavailable. Try again in a minute.', true)
|
||||
} else {
|
||||
sendStatus({ state: 'idle' })
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -12,9 +12,13 @@ import type { ChangelogData } from '../../../shared/types'
|
|||
// ── Helpers ──────────────────────────────────────────────────────────
|
||||
|
||||
function releaseUrlForVersion(version: string | null): string {
|
||||
// Why: when no version is cached (typically a failed check), point at the
|
||||
// plain releases listing rather than /releases/latest — /latest also breaks
|
||||
// when GitHub's release API is degraded, and the listing is the most
|
||||
// reliable manual fallback.
|
||||
return version
|
||||
? `https://github.com/stablyai/orca/releases/tag/v${version}`
|
||||
: 'https://github.com/stablyai/orca/releases/latest'
|
||||
: 'https://github.com/stablyai/orca/releases'
|
||||
}
|
||||
|
||||
function isAnimatedGif(url: string | undefined): boolean {
|
||||
|
|
@ -312,7 +316,9 @@ export function UpdateCard() {
|
|||
const errorCard: ErrorCardModel | null =
|
||||
status.state === 'error'
|
||||
? {
|
||||
title: 'Update Error',
|
||||
// Why: title is scoped to the operation that failed so check-time
|
||||
// failures (commonly GitHub-side) don't read as a bug in Orca.
|
||||
title: cachedVersion ? 'Update Error' : 'Update Check Failed',
|
||||
summary: cachedVersion
|
||||
? 'Could not complete the update.'
|
||||
: 'Could not check for updates.',
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
/* eslint-disable max-lines -- Why: GeneralPane is the single owner of all general settings UI;
|
||||
splitting individual settings into separate files would scatter related controls without a
|
||||
meaningful abstraction boundary. */
|
||||
import { useEffect, useState } from 'react'
|
||||
import { useEffect, useRef, useState } from 'react'
|
||||
import type {
|
||||
ClaudeRateLimitAccountsState,
|
||||
CodexRateLimitAccountsState,
|
||||
|
|
@ -121,6 +121,30 @@ export function GeneralPane({ settings, updateSettings }: GeneralPaneProps): Rea
|
|||
const searchQuery = useAppStore((s) => s.settingsSearchQuery)
|
||||
const updateStatus = useAppStore((s) => s.updateStatus)
|
||||
const fetchSettings = useAppStore((s) => s.fetchSettings)
|
||||
// Why: the 'error' variant of UpdateStatus does not carry a `version` field.
|
||||
// The main process emits `{ state: 'error' }` for both check failures (no
|
||||
// version known yet) and download/install failures (version was known from
|
||||
// the preceding 'available'/'downloading'/'downloaded' state). Cache the
|
||||
// last-known version so the error copy below can distinguish the two cases
|
||||
// without adding IPC. Mirrors `versionRef` in UpdateCard.tsx.
|
||||
const updateVersionRef = useRef<string | null>(null)
|
||||
if (
|
||||
(updateStatus.state === 'available' ||
|
||||
updateStatus.state === 'downloading' ||
|
||||
updateStatus.state === 'downloaded') &&
|
||||
updateStatus.version
|
||||
) {
|
||||
updateVersionRef.current = updateStatus.version
|
||||
} else if (
|
||||
updateStatus.state === 'checking' ||
|
||||
updateStatus.state === 'idle' ||
|
||||
updateStatus.state === 'not-available'
|
||||
) {
|
||||
// Why: a new check cycle has started or completed cleanly. Clear the
|
||||
// cached version so a subsequent check failure cannot be mis-classified
|
||||
// as a download failure based on a stale version from a prior cycle.
|
||||
updateVersionRef.current = null
|
||||
}
|
||||
const [appVersion, setAppVersion] = useState<string | null>(null)
|
||||
const [autoSaveDelayDraft, setAutoSaveDelayDraft] = useState(
|
||||
String(settings.editorAutoSaveDelayMs)
|
||||
|
|
@ -1089,7 +1113,15 @@ export function GeneralPane({ settings, updateSettings }: GeneralPaneProps): Rea
|
|||
</a>
|
||||
</>
|
||||
)}
|
||||
{updateStatus.state === 'error' && `Update error: ${updateStatus.message}`}
|
||||
{updateStatus.state === 'error' &&
|
||||
// Why: `{ state: 'error' }` is emitted for both check-time
|
||||
// failures (no version cached) and download/install failures
|
||||
// (version cached from a prior 'available'/'downloading'/
|
||||
// 'downloaded' state). Label accordingly so a download failure
|
||||
// isn't mislabeled as a "check" failure. Mirrors UpdateCard.tsx.
|
||||
(updateVersionRef.current
|
||||
? `Update error. ${updateStatus.message}`
|
||||
: `Update check failed. ${updateStatus.message}`)}
|
||||
</p>
|
||||
</SearchableSetting>
|
||||
</section>
|
||||
|
|
|
|||
Loading…
Reference in New Issue