fix(desktop): distinguish refresh failures from native update errors

Author: waleedlatif1

apps/desktop/e2e/updater.spec.ts

@@ -116,7 +116,8 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret
 
     await check('replacement cannot restart into stale native update', async () => {
       offeredVersion = '2.1.0'
-      await expect.poll(async () => (await read()).nativeArchive, { timeout: 20_000 }).toBe('2.1.0')
+      await shell.evaluate(() => globalThis.desktopUpdaterFixture.check())
+      await expect.poll(async () => (await read()).nativeArchive).toBe('2.1.0')
       expect((await read()).state).toEqual({
         status: 'downloading',
         version: '2.1.0',

apps/desktop/src/main/updater.test.ts

@@ -88,10 +88,11 @@ describe('initUpdater state machine', () => {
   }
 
   /** Replays a native Squirrel.Mac event, e.g. `update-downloaded` once a bundle is staged. */
-  function emitSquirrel(event: string) {
+  function emitSquirrel(event: string, ...args: unknown[]) {
+    if (event === 'error') emit(event, ...args)
     for (const [name, listener] of vi.mocked(squirrelUpdater.on).mock.calls) {
       if (name === event) {
-        ;(listener as () => void)()
+        ;(listener as (...values: unknown[]) => void)(...args)
       }
     }
   }
@@ -274,7 +275,7 @@ describe('initUpdater state machine', () => {
     expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1)
   })
 
-  it('does not install when the updater fails during pre-install teardown', async () => {
+  it('does not install when native staging fails during teardown with a background check pending', async () => {
     let finishTeardown: (() => void) | undefined
     const setRelaunchPending = vi.fn()
     const { handle } = await createUpdater({
@@ -290,14 +291,15 @@ describe('initUpdater state machine', () => {
     emit('update-available', { version: '2.0.0' })
     emit('update-downloaded', { version: '2.0.0' })
     emitSquirrel('update-downloaded')
+    await vi.advanceTimersByTimeAsync(10_000)
     vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({
       response: 1,
       checkboxChecked: false,
     })
     handle.install()
     await vi.advanceTimersByTimeAsync(0)
 
-    emit('error', new Error('native installer failed'))
+    emitSquirrel('error', new Error('native installer failed'))
     finishTeardown?.()
     await vi.advanceTimersByTimeAsync(0)
 
@@ -306,6 +308,39 @@ describe('initUpdater state machine', () => {
     expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled()
   })
 
+  it('finishes a confirmed restart when an earlier background check fails during teardown', async () => {
+    let finishTeardown: (() => void) | undefined
+    let failRefresh: ((error: Error) => void) | undefined
+    const { handle } = await createUpdater({
+      beforeInstall: () =>
+        new Promise<void>((resolve) => {
+          finishTeardown = resolve
+        }),
+    })
+    await stageUpdate(handle, '2.0.0')
+    autoUpdaterMock.checkForUpdates.mockImplementationOnce(
+      () =>
+        new Promise((_, reject) => {
+          failRefresh = (error) => {
+            emit('error', error)
+            reject(error)
+          }
+        })
+    )
+    await vi.advanceTimersByTimeAsync(10_000)
+    vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ response: 1, checkboxChecked: false })
+    handle.install()
+    await vi.advanceTimersByTimeAsync(0)
+
+    failRefresh?.(new Error('Background feed unavailable'))
+    await vi.advanceTimersByTimeAsync(0)
+    expect(handle.getState()).toEqual({ status: 'ready', version: '2.0.0' })
+    expect(autoUpdaterMock.autoInstallOnAppQuit).toBe(true)
+    finishTeardown?.()
+    await vi.advanceTimersByTimeAsync(0)
+    expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1)
+  })
+
   it('bypasses renderer unload guards only after teardown succeeds', async () => {
     const setRelaunchPending = vi.fn()
     vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({
@@ -382,16 +417,17 @@ describe('initUpdater state machine', () => {
     emit('update-available', { version: '2.0.0' })
     await vi.advanceTimersByTimeAsync(30 * 60 * 1000 - 10_000)
     emit('update-not-available')
+    autoUpdaterMock.checkForUpdates.mockRejectedValueOnce(new Error('net::ERR_NETWORK_CHANGED'))
     await vi.advanceTimersByTimeAsync(30 * 60 * 1000)
-    emit('error', new Error('net::ERR_NETWORK_CHANGED'))
     await vi.advanceTimersByTimeAsync(30 * 60 * 1000)
+    autoUpdaterMock.downloadUpdate.mockRejectedValueOnce(new Error('download interrupted'))
     emit('update-available', { version: '2.1.0' })
-    emit('error', new Error('download interrupted'))
+    await vi.advanceTimersByTimeAsync(0)
     expect(handle.getState()).toEqual({ status: 'ready', version: '2.0.0' })
     await vi.advanceTimersByTimeAsync(30 * 60 * 1000)
     emit('update-available', { version: '2.2.0' })
     emit('update-downloaded', { version: '2.2.0' })
-    emit('error', new Error('Squirrel could not verify the replacement'))
+    emitSquirrel('error', new Error('Squirrel could not verify the replacement'))
 
     expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(6)
     expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(3)

apps/desktop/src/main/updater.ts

@@ -683,29 +683,13 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
       setState({ status: 'ready', version })
     })
 
-    autoUpdater.on('error', (error) => {
-      const checkId = activeUpdaterCheckId
-      if (checkId !== null) {
-        finishUpdaterCheck(checkId)
-        if (updaterRequestId === checkId) updaterRequestId = null
-      }
-      const message = getErrorMessage(error, 'unknown')
-      if (stagedVersion !== null && !installInFlight && !relaunchRequested) {
-        acceptedUpdateVersion = null
-        pendingStagingVersion = null
-        logger.warn('Update refresh failed; keeping the staged update', { message })
-        if (state.status !== 'ready') setState({ status: 'ready', version: stagedVersion })
-        return
-      }
-      if (state.status === 'available') {
-        logger.warn('Update re-check failed; keeping the offered update', { message })
-        return
-      }
+    /** Network failures belong to their request promises; native errors invalidate staging. */
+    squirrelUpdater.on('error', (error) => {
       if (
-        checkId === null &&
         state.status !== 'downloading' &&
         state.status !== 'ready' &&
-        !installInFlight
+        !installInFlight &&
+        !relaunchRequested
       ) {
         return
       }
@@ -716,6 +700,8 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
       pendingStagingVersion = null
       deps.setRelaunchPending?.(false)
       autoUpdater.autoInstallOnAppQuit = false
+      const message = getErrorMessage(error, 'unknown')
+      logger.warn('Native update failed', { message })
       deps.events.record('update_error', { message })
       setState({ status: 'error', version: state.version })
     })
@@ -802,7 +788,9 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
           if (updaterRequestId === checkId) updaterRequestId = null
           if (activeUpdaterCheckId !== checkId) return
           finishUpdaterCheck(checkId)
-          logger.warn('Update check failed', { message: getErrorMessage(error, 'unknown') })
+          const message = getErrorMessage(error, 'unknown')
+          logger.warn('Update check failed', { message })
+          deps.events.record('update_error', { message })
           if (state.status === 'checking') setState({ status: 'error' })
         })
     }
@@ -817,10 +805,8 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
           return
         }
         if (isRefreshingOffer()) {
-          if (interactive) return
           if (state.status === 'ready' && !canRefreshStagedUpdate()) return
-        }
-        if (interactive) {
+        } else if (interactive) {
           setState({ status: 'checking' })
         }
         if (originFeedConfigured) {