Surface update errors to UI for better user feedback (related to #671)

User ZaDarkSide reported that when updates fail, the UI shows a loading
spinner indefinitely with no feedback about what went wrong. Users had to
check backend logs to understand failures like "checksum verification failed".

The infrastructure was already in place:
- UpdateStatus struct had an Error field
- Frontend already renders error details when present
- But updateStatus() never populated the Error field

Changes:
- Modified updateStatus() to accept optional error parameter
- Added sanitizeError() to cap error message length (500 chars max)
- Updated all error cases in ApplyUpdate() to pass error details:
  - Temp directory creation failures
  - Download failures
  - Checksum verification failures (most common user complaint)
  - Extraction failures
  - Backup creation failures
  - Apply update failures
- Also updated CheckForUpdates() error cases

Now when updates fail, users immediately see the error message in the UI's
red error panel instead of being stuck on a loading spinner.

Security: Errors are only shown to authenticated admin users with update
permissions. Error messages are capped at 500 chars to prevent extremely
long output. Current error messages don't contain sensitive data (mainly
HTTP status codes, file paths, checksum mismatches).
This commit is contained in:
rcourtman 2025-11-09 08:23:04 +00:00
parent cb682ed369
commit 6bb53eaadb

View file

@ -234,14 +234,15 @@ func (m *Manager) CheckForUpdatesWithChannel(ctx context.Context, channel string
return info, nil return info, nil
} }
// For other errors, return the error // For other errors, return the error
m.updateStatus("error", 0, "Failed to check for updates") m.updateStatus("error", 0, "Failed to check for updates", err)
return nil, err return nil, err
} }
latestVer, err := ParseVersion(release.TagName) latestVer, err := ParseVersion(release.TagName)
if err != nil { if err != nil {
m.updateStatus("error", 0, "Invalid latest version") parseErr := fmt.Errorf("failed to parse latest version: %w", err)
return nil, fmt.Errorf("failed to parse latest version: %w", err) m.updateStatus("error", 0, "Invalid latest version", parseErr)
return nil, parseErr
} }
// Find download URL for current architecture // Find download URL for current architecture
@ -351,8 +352,9 @@ func (m *Manager) ApplyUpdate(ctx context.Context, downloadURL string) error {
// Last resort: current directory // Last resort: current directory
tempDir, err = os.MkdirTemp(".", "pulse-update-*") tempDir, err = os.MkdirTemp(".", "pulse-update-*")
if err != nil { if err != nil {
m.updateStatus("error", 10, "Failed to create temp directory") tempErr := fmt.Errorf("failed to create temp directory in any location: %w", err)
return fmt.Errorf("failed to create temp directory in any location: %w", err) m.updateStatus("error", 10, "Failed to create temp directory", tempErr)
return tempErr
} }
} }
} }
@ -361,15 +363,17 @@ func (m *Manager) ApplyUpdate(ctx context.Context, downloadURL string) error {
// Download update // Download update
tarballPath := filepath.Join(tempDir, "update.tar.gz") tarballPath := filepath.Join(tempDir, "update.tar.gz")
if err := m.downloadFile(ctx, downloadURL, tarballPath); err != nil { if err := m.downloadFile(ctx, downloadURL, tarballPath); err != nil {
m.updateStatus("error", 20, "Failed to download update") downloadErr := fmt.Errorf("failed to download update: %w", err)
return fmt.Errorf("failed to download update: %w", err) m.updateStatus("error", 20, "Failed to download update", downloadErr)
return downloadErr
} }
// Verify checksum if available // Verify checksum if available
m.updateStatus("verifying", 30, "Verifying download...") m.updateStatus("verifying", 30, "Verifying download...")
if err := m.verifyChecksum(ctx, downloadURL, tarballPath); err != nil { if err := m.verifyChecksum(ctx, downloadURL, tarballPath); err != nil {
m.updateStatus("error", 30, "Failed to verify update checksum") checksumErr := fmt.Errorf("checksum verification failed: %w", err)
return fmt.Errorf("checksum verification failed: %w", err) m.updateStatus("error", 30, "Failed to verify update checksum", checksumErr)
return checksumErr
} }
log.Info().Msg("Checksum verification passed") log.Info().Msg("Checksum verification passed")
@ -378,8 +382,9 @@ func (m *Manager) ApplyUpdate(ctx context.Context, downloadURL string) error {
// Extract tarball // Extract tarball
extractDir := filepath.Join(tempDir, "extracted") extractDir := filepath.Join(tempDir, "extracted")
if err := m.extractTarball(tarballPath, extractDir); err != nil { if err := m.extractTarball(tarballPath, extractDir); err != nil {
m.updateStatus("error", 40, "Failed to extract update") extractErr := fmt.Errorf("failed to extract update: %w", err)
return fmt.Errorf("failed to extract update: %w", err) m.updateStatus("error", 40, "Failed to extract update", extractErr)
return extractErr
} }
m.updateStatus("backing-up", 60, "Creating backup...") m.updateStatus("backing-up", 60, "Creating backup...")
@ -387,8 +392,9 @@ func (m *Manager) ApplyUpdate(ctx context.Context, downloadURL string) error {
// Create backup // Create backup
backupPath, err := m.createBackup() backupPath, err := m.createBackup()
if err != nil { if err != nil {
m.updateStatus("error", 60, "Failed to create backup") backupErr := fmt.Errorf("failed to create backup: %w", err)
return fmt.Errorf("failed to create backup: %w", err) m.updateStatus("error", 60, "Failed to create backup", backupErr)
return backupErr
} }
log.Info().Str("backup", backupPath).Msg("Created backup") log.Info().Str("backup", backupPath).Msg("Created backup")
@ -411,12 +417,13 @@ func (m *Manager) ApplyUpdate(ctx context.Context, downloadURL string) error {
log.Info().Msg("Applying update files") log.Info().Msg("Applying update files")
if err := m.applyUpdateFiles(extractDir); err != nil { if err := m.applyUpdateFiles(extractDir); err != nil {
m.updateStatus("error", 80, "Failed to apply update") applyErr := fmt.Errorf("failed to apply update: %w", err)
m.updateStatus("error", 80, "Failed to apply update", applyErr)
// Attempt to restore backup // Attempt to restore backup
if restoreErr := m.restoreBackup(backupPath); restoreErr != nil { if restoreErr := m.restoreBackup(backupPath); restoreErr != nil {
log.Error().Err(restoreErr).Msg("Failed to restore backup") log.Error().Err(restoreErr).Msg("Failed to restore backup")
} }
return fmt.Errorf("failed to apply update: %w", err) return applyErr
} }
m.updateStatus("restarting", 95, "Restarting service...") m.updateStatus("restarting", 95, "Restarting service...")
@ -1047,7 +1054,7 @@ func (m *Manager) applyUpdateFiles(extractDir string) error {
} }
// updateStatus updates the current status // updateStatus updates the current status
func (m *Manager) updateStatus(status string, progress int, message string) { func (m *Manager) updateStatus(status string, progress int, message string, err ...error) {
m.statusMu.Lock() m.statusMu.Lock()
m.status = UpdateStatus{ m.status = UpdateStatus{
Status: status, Status: status,
@ -1055,6 +1062,10 @@ func (m *Manager) updateStatus(status string, progress int, message string) {
Message: message, Message: message,
UpdatedAt: time.Now().Format(time.RFC3339), UpdatedAt: time.Now().Format(time.RFC3339),
} }
// If error provided, sanitize and add to status
if len(err) > 0 && err[0] != nil {
m.status.Error = sanitizeError(err[0])
}
statusCopy := m.status statusCopy := m.status
m.statusMu.Unlock() m.statusMu.Unlock()
@ -1065,6 +1076,23 @@ func (m *Manager) updateStatus(status string, progress int, message string) {
} }
} }
// sanitizeError removes potentially sensitive information from error messages
func sanitizeError(err error) string {
if err == nil {
return ""
}
errMsg := err.Error()
// Cap length to prevent extremely long error messages
maxLen := 500
if len(errMsg) > maxLen {
errMsg = errMsg[:maxLen] + "..."
}
return errMsg
}
// cleanupOldTempDirs removes old pulse-update-* temp directories from previous runs // cleanupOldTempDirs removes old pulse-update-* temp directories from previous runs
func (m *Manager) cleanupOldTempDirs() { func (m *Manager) cleanupOldTempDirs() {
// Check multiple locations where temp dirs might exist // Check multiple locations where temp dirs might exist