-
Notifications
You must be signed in to change notification settings - Fork 0
fix(ui): surface update errors in AppShell with retry #61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,12 +1,28 @@ | ||||||||
| import { type ReactNode } from 'react'; | ||||||||
| import { useLayout } from '../../contexts/LayoutContext'; | ||||||||
| import { useNetwork } from '../../hooks'; | ||||||||
| import { useUpdateCheck } from '../../hooks/useUpdateCheck'; | ||||||||
| import { useUpdateCheck, type UpdatePhase } from '../../hooks/useUpdateCheck'; | ||||||||
| import { OfflineIndicator } from '../shared'; | ||||||||
| import { TabSwitcher, ConversationSidebar } from '../conversations'; | ||||||||
| import { EmployeePanel } from '../employees'; | ||||||||
| import { TrialBanner } from '../trial/TrialBanner'; | ||||||||
|
|
||||||||
| function updateButtonLabel(phase: UpdatePhase): string { | ||||||||
| switch (phase) { | ||||||||
| case 'downloading': return 'Downloading…'; | ||||||||
| case 'relaunching': return 'Relaunching…'; | ||||||||
| default: return 'Update Available'; | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| function friendlyUpdateError(raw: string): string { | ||||||||
| const lower = raw.toLowerCase(); | ||||||||
| if (lower.includes('sandbox')) return 'Update blocked by sandbox'; | ||||||||
| if (lower.includes('signature') || lower.includes('sig ')) return 'Update signature invalid'; | ||||||||
| if (lower.includes('network') || lower.includes('fetch') || lower.includes('request')) return 'Network error during update'; | ||||||||
| return 'Update failed'; | ||||||||
| } | ||||||||
|
|
||||||||
| interface AppShellProps { | ||||||||
| children: ReactNode; | ||||||||
| contextPanel?: ReactNode; | ||||||||
|
|
@@ -79,7 +95,7 @@ function IconButton({ | |||||||
| export function AppShell({ children, contextPanel, onSettingsClick }: AppShellProps) { | ||||||||
| const { sidebarOpen, contextPanelOpen, sidebarTab, toggleSidebar, toggleContextPanel, setSidebarTab } = useLayout(); | ||||||||
| const { isOnline, errorMessage, checkNow, isChecking } = useNetwork(); | ||||||||
| const { updateAvailable, installing, installUpdate } = useUpdateCheck(); | ||||||||
| const { updateAvailable, phase, installing, error: updateError, installUpdate, retry: retryUpdate } = useUpdateCheck(); | ||||||||
|
|
||||||||
| return ( | ||||||||
| <div className="h-screen flex flex-col bg-stone-50 overflow-hidden"> | ||||||||
|
|
@@ -115,7 +131,45 @@ export function AppShell({ children, contextPanel, onSettingsClick }: AppShellPr | |||||||
| /> | ||||||||
|
|
||||||||
| <div className="flex items-center gap-1"> | ||||||||
| {updateAvailable && ( | ||||||||
| {updateError ? ( | ||||||||
| <div | ||||||||
| role="alert" | ||||||||
|
||||||||
| role="alert" | |
| role="alert" | |
| aria-live="polite" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,18 +2,20 @@ import { useEffect, useState } from 'react'; | |
| import { check, Update } from '@tauri-apps/plugin-updater'; | ||
| import { relaunch } from '@tauri-apps/plugin-process'; | ||
|
|
||
| export type UpdatePhase = 'idle' | 'checking' | 'downloading' | 'relaunching'; | ||
|
|
||
| export function useUpdateCheck() { | ||
| const [updateAvailable, setUpdateAvailable] = useState<Update | null>(null); | ||
| const [checking, setChecking] = useState(false); | ||
| const [installing, setInstalling] = useState(false); | ||
| const [phase, setPhase] = useState<UpdatePhase>('idle'); | ||
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| checkForUpdate(); | ||
| void checkForUpdate(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, []); | ||
|
Comment on lines
12
to
15
|
||
|
|
||
| async function checkForUpdate() { | ||
| setChecking(true); | ||
| setPhase('checking'); | ||
| setError(null); | ||
| try { | ||
| const update = await check(); | ||
|
|
@@ -23,21 +25,47 @@ export function useUpdateCheck() { | |
| } catch (e) { | ||
| setError(e instanceof Error ? e.message : 'Failed to check for updates'); | ||
| } finally { | ||
| setChecking(false); | ||
| setPhase('idle'); | ||
| } | ||
| } | ||
|
|
||
| async function installUpdate() { | ||
| if (!updateAvailable) return; | ||
| setInstalling(true); | ||
| setError(null); | ||
| setPhase('downloading'); | ||
| try { | ||
| await updateAvailable.downloadAndInstall(); | ||
| await updateAvailable.downloadAndInstall((progress) => { | ||
| if (progress.event === 'Finished') { | ||
| setPhase('relaunching'); | ||
| } | ||
| }); | ||
| await relaunch(); | ||
| } catch (e) { | ||
| setError(e instanceof Error ? e.message : 'Failed to install update'); | ||
| setInstalling(false); | ||
| setPhase('idle'); | ||
| } | ||
| } | ||
|
|
||
| return { updateAvailable, checking, installing, error, checkForUpdate, installUpdate }; | ||
| async function retry() { | ||
| setError(null); | ||
| if (updateAvailable) { | ||
| await installUpdate(); | ||
| } else { | ||
| await checkForUpdate(); | ||
| } | ||
| } | ||
|
|
||
| const checking = phase === 'checking'; | ||
| const installing = phase === 'downloading' || phase === 'relaunching'; | ||
|
|
||
| return { | ||
| updateAvailable, | ||
| phase, | ||
| checking, | ||
| installing, | ||
| error, | ||
| checkForUpdate, | ||
| installUpdate, | ||
| retry, | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateButtonLabeldoesn’t handle the'checking'phase even though it’s part ofUpdatePhase. If the button can ever be rendered whilephase === 'checking'(e.g., a re-check while an update is already known), the label will incorrectly show “Update Available”. Consider adding an explicit'checking'case (or ensure the update button cannot render while checking).