fix(updater): handle benign network errors and deduplicate error statuses (#146)
* fix(updater): handle benign network errors and deduplicate error statuses Treats net::ERR_FAILED during update checks as benign (transitions to idle instead of showing error), deduplicates identical error statuses, and adds a manual download link to user-facing error toasts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use T[] array syntax instead of Array<T> to pass oxlint Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Jinjing
GitHub
parent
a1e2ba2003
commit
bbc4a98f63
|
|
@ -0,0 +1,155 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const {
|
||||
appMock,
|
||||
browserWindowMock,
|
||||
nativeUpdaterMock,
|
||||
autoUpdaterMock,
|
||||
isMock,
|
||||
killAllPtyMock
|
||||
} = vi.hoisted(() => {
|
||||
const eventHandlers = new Map<string, ((...args: unknown[]) => void)[]>()
|
||||
|
||||
const on = vi.fn((event: string, handler: (...args: unknown[]) => void) => {
|
||||
const handlers = eventHandlers.get(event) ?? []
|
||||
handlers.push(handler)
|
||||
eventHandlers.set(event, handlers)
|
||||
return autoUpdaterMock
|
||||
})
|
||||
|
||||
const emit = (event: string, ...args: unknown[]) => {
|
||||
for (const handler of eventHandlers.get(event) ?? []) {
|
||||
handler(...args)
|
||||
}
|
||||
}
|
||||
|
||||
const reset = () => {
|
||||
eventHandlers.clear()
|
||||
on.mockClear()
|
||||
autoUpdaterMock.checkForUpdates.mockReset()
|
||||
autoUpdaterMock.downloadUpdate.mockReset()
|
||||
autoUpdaterMock.quitAndInstall.mockReset()
|
||||
}
|
||||
|
||||
const autoUpdaterMock = {
|
||||
autoDownload: false,
|
||||
autoInstallOnAppQuit: false,
|
||||
allowPrerelease: false,
|
||||
on,
|
||||
checkForUpdates: vi.fn(),
|
||||
downloadUpdate: vi.fn(),
|
||||
quitAndInstall: vi.fn(),
|
||||
emit,
|
||||
reset
|
||||
}
|
||||
|
||||
return {
|
||||
appMock: {
|
||||
isPackaged: true,
|
||||
getVersion: vi.fn(() => '1.0.51')
|
||||
},
|
||||
browserWindowMock: {
|
||||
getAllWindows: vi.fn(() => [])
|
||||
},
|
||||
nativeUpdaterMock: {
|
||||
on: vi.fn()
|
||||
},
|
||||
autoUpdaterMock,
|
||||
isMock: { dev: false },
|
||||
killAllPtyMock: vi.fn()
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
app: appMock,
|
||||
BrowserWindow: browserWindowMock,
|
||||
autoUpdater: nativeUpdaterMock
|
||||
}))
|
||||
|
||||
vi.mock('electron-updater', () => ({
|
||||
autoUpdater: autoUpdaterMock
|
||||
}))
|
||||
|
||||
vi.mock('@electron-toolkit/utils', () => ({
|
||||
is: isMock
|
||||
}))
|
||||
|
||||
vi.mock('./ipc/pty', () => ({
|
||||
killAllPty: killAllPtyMock
|
||||
}))
|
||||
|
||||
describe('updater', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules()
|
||||
autoUpdaterMock.reset()
|
||||
nativeUpdaterMock.on.mockReset()
|
||||
browserWindowMock.getAllWindows.mockReset()
|
||||
browserWindowMock.getAllWindows.mockReturnValue([])
|
||||
appMock.getVersion.mockReset()
|
||||
appMock.getVersion.mockReturnValue('1.0.51')
|
||||
appMock.isPackaged = true
|
||||
isMock.dev = false
|
||||
killAllPtyMock.mockReset()
|
||||
})
|
||||
|
||||
it('deduplicates identical check errors from the event and rejected promise', async () => {
|
||||
autoUpdaterMock.checkForUpdates
|
||||
.mockResolvedValueOnce(undefined)
|
||||
.mockImplementationOnce(() => {
|
||||
autoUpdaterMock.emit('checking-for-update')
|
||||
queueMicrotask(() => {
|
||||
autoUpdaterMock.emit('error', new Error('boom'))
|
||||
})
|
||||
return Promise.reject(new Error('boom'))
|
||||
})
|
||||
|
||||
const sendMock = vi.fn()
|
||||
const mainWindow = { webContents: { send: sendMock } }
|
||||
|
||||
const { setupAutoUpdater, checkForUpdatesFromMenu } = await import('./updater')
|
||||
|
||||
setupAutoUpdater(mainWindow as never)
|
||||
checkForUpdatesFromMenu()
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
|
||||
const errorStatuses = sendMock.mock.calls
|
||||
.filter(([channel]) => channel === 'updater:status')
|
||||
.map(([, status]) => status)
|
||||
.filter((status) => typeof status === 'object' && status !== null && status.state === 'error')
|
||||
|
||||
expect(errorStatuses).toEqual([{ state: 'error', message: 'boom', userInitiated: true }])
|
||||
})
|
||||
|
||||
it('treats net::ERR_FAILED during checks as a benign idle transition', async () => {
|
||||
autoUpdaterMock.checkForUpdates
|
||||
.mockResolvedValueOnce(undefined)
|
||||
.mockImplementationOnce(() => {
|
||||
autoUpdaterMock.emit('checking-for-update')
|
||||
queueMicrotask(() => {
|
||||
autoUpdaterMock.emit('error', new Error('net::ERR_FAILED'))
|
||||
})
|
||||
return Promise.reject(new Error('net::ERR_FAILED'))
|
||||
})
|
||||
|
||||
const sendMock = vi.fn()
|
||||
const mainWindow = { webContents: { send: sendMock } }
|
||||
|
||||
const { setupAutoUpdater, checkForUpdatesFromMenu } = await import('./updater')
|
||||
|
||||
setupAutoUpdater(mainWindow as never)
|
||||
checkForUpdatesFromMenu()
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
|
||||
const statuses = sendMock.mock.calls
|
||||
.filter(([channel]) => channel === 'updater:status')
|
||||
.map(([, status]) => status)
|
||||
|
||||
expect(statuses).toContainEqual({ state: 'checking', userInitiated: true })
|
||||
expect(statuses).toContainEqual({ state: 'idle' })
|
||||
expect(statuses).not.toContainEqual(
|
||||
expect.objectContaining({ state: 'error', message: 'net::ERR_FAILED' })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
|
@ -18,6 +18,30 @@ function sendStatus(status: UpdateStatus): void {
|
|||
mainWindowRef?.webContents.send('updater:status', status)
|
||||
}
|
||||
|
||||
function sendErrorStatus(message: string, userInitiated?: boolean): void {
|
||||
if (
|
||||
currentStatus.state === 'error' &&
|
||||
currentStatus.message === message &&
|
||||
currentStatus.userInitiated === userInitiated
|
||||
) {
|
||||
return
|
||||
}
|
||||
sendStatus({ state: 'error', message, userInitiated })
|
||||
}
|
||||
|
||||
function isBenignCheckFailure(message: string): boolean {
|
||||
return message.includes('net::ERR_FAILED')
|
||||
}
|
||||
|
||||
function sendCheckFailureStatus(message: string, userInitiated?: boolean): void {
|
||||
if (isBenignCheckFailure(message)) {
|
||||
console.warn('[updater] benign check failure:', message)
|
||||
sendStatus({ state: 'idle' })
|
||||
return
|
||||
}
|
||||
sendErrorStatus(message, userInitiated)
|
||||
}
|
||||
|
||||
export function getUpdateStatus(): UpdateStatus {
|
||||
return currentStatus
|
||||
}
|
||||
|
|
@ -30,7 +54,7 @@ export function checkForUpdates(): void {
|
|||
// Don't send 'checking' here — the 'checking-for-update' event handler does it,
|
||||
// and sending it from both places causes duplicate notifications (issue #35).
|
||||
autoUpdater.checkForUpdates().catch((err) => {
|
||||
sendStatus({ state: 'error', message: String(err?.message ?? err) })
|
||||
sendCheckFailureStatus(String(err?.message ?? err))
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -47,7 +71,7 @@ export function checkForUpdatesFromMenu(): void {
|
|||
|
||||
autoUpdater.checkForUpdates().catch((err) => {
|
||||
userInitiatedCheck = false
|
||||
sendStatus({ state: 'error', message: String(err?.message ?? err), userInitiated: true })
|
||||
sendCheckFailureStatus(String(err?.message ?? err), true)
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -162,11 +186,12 @@ export function setupAutoUpdater(
|
|||
autoUpdater.on('error', (err) => {
|
||||
const wasUserInitiated = userInitiatedCheck
|
||||
userInitiatedCheck = false
|
||||
sendStatus({
|
||||
state: 'error',
|
||||
message: err?.message ?? 'Unknown error',
|
||||
userInitiated: wasUserInitiated || undefined
|
||||
})
|
||||
const message = err?.message ?? 'Unknown error'
|
||||
if (currentStatus.state === 'checking') {
|
||||
sendCheckFailureStatus(message, wasUserInitiated || undefined)
|
||||
return
|
||||
}
|
||||
sendErrorStatus(message, wasUserInitiated || undefined)
|
||||
})
|
||||
|
||||
autoUpdater.checkForUpdates().catch((err) => {
|
||||
|
|
@ -181,6 +206,6 @@ export function downloadUpdate(): void {
|
|||
}
|
||||
squirrelReady = false
|
||||
autoUpdater.downloadUpdate().catch((err) => {
|
||||
sendStatus({ state: 'error', message: String(err?.message ?? err) })
|
||||
sendErrorStatus(String(err?.message ?? err))
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { useEffect } from 'react'
|
||||
import { useEffect, createElement } from 'react'
|
||||
import { toast } from 'sonner'
|
||||
import { useAppStore } from '../store'
|
||||
import { applyUIZoom } from '@/lib/ui-zoom'
|
||||
|
|
@ -44,6 +44,11 @@ export function useIpcEvents(): void {
|
|||
// Show toasts for user-initiated checks
|
||||
if (status.state === 'checking' && 'userInitiated' in status && status.userInitiated) {
|
||||
checkingToastId = toast.loading('Checking for updates...')
|
||||
} else if (status.state === 'idle') {
|
||||
if (checkingToastId) {
|
||||
toast.dismiss(checkingToastId)
|
||||
checkingToastId = undefined
|
||||
}
|
||||
} else if (status.state === 'not-available') {
|
||||
if ('userInitiated' in status && status.userInitiated) {
|
||||
toast.success('You\u2019re on the latest version.', { id: checkingToastId })
|
||||
|
|
@ -87,7 +92,23 @@ export function useIpcEvents(): void {
|
|||
toast.dismiss(downloadToastId)
|
||||
if ('userInitiated' in status && status.userInitiated) {
|
||||
toast.error('Could not check for updates.', {
|
||||
description: status.message,
|
||||
description: createElement(
|
||||
'span',
|
||||
null,
|
||||
status.message,
|
||||
' You can download the latest version manually from ',
|
||||
createElement(
|
||||
'a',
|
||||
{
|
||||
href: 'https://github.com/stablyai/orca/releases/latest',
|
||||
target: '_blank',
|
||||
rel: 'noopener noreferrer',
|
||||
style: { textDecoration: 'underline' }
|
||||
},
|
||||
'our GitHub releases page'
|
||||
),
|
||||
'.'
|
||||
),
|
||||
id: checkingToastId
|
||||
})
|
||||
checkingToastId = undefined
|
||||
|
|
|
|||
Loading…
Reference in New Issue