From 6bb53eaadbae1afbf9a9945a8e939adb232935da Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 9 Nov 2025 08:23:04 +0000 Subject: [PATCH] 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). --- internal/updates/manager.go | 60 +++++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/internal/updates/manager.go b/internal/updates/manager.go index 4aaa5ca..2389079 100644 --- a/internal/updates/manager.go +++ b/internal/updates/manager.go @@ -234,14 +234,15 @@ func (m *Manager) CheckForUpdatesWithChannel(ctx context.Context, channel string return info, nil } // 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 } latestVer, err := ParseVersion(release.TagName) if err != nil { - m.updateStatus("error", 0, "Invalid latest version") - return nil, fmt.Errorf("failed to parse latest version: %w", err) + parseErr := 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 @@ -351,8 +352,9 @@ func (m *Manager) ApplyUpdate(ctx context.Context, downloadURL string) error { // Last resort: current directory tempDir, err = os.MkdirTemp(".", "pulse-update-*") if err != nil { - m.updateStatus("error", 10, "Failed to create temp directory") - return fmt.Errorf("failed to create temp directory in any location: %w", err) + tempErr := 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 tarballPath := filepath.Join(tempDir, "update.tar.gz") if err := m.downloadFile(ctx, downloadURL, tarballPath); err != nil { - m.updateStatus("error", 20, "Failed to download update") - return fmt.Errorf("failed to download update: %w", err) + downloadErr := fmt.Errorf("failed to download update: %w", err) + m.updateStatus("error", 20, "Failed to download update", downloadErr) + return downloadErr } // Verify checksum if available m.updateStatus("verifying", 30, "Verifying download...") if err := m.verifyChecksum(ctx, downloadURL, tarballPath); err != nil { - m.updateStatus("error", 30, "Failed to verify update checksum") - return fmt.Errorf("checksum verification failed: %w", err) + checksumErr := 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") @@ -378,8 +382,9 @@ func (m *Manager) ApplyUpdate(ctx context.Context, downloadURL string) error { // Extract tarball extractDir := filepath.Join(tempDir, "extracted") if err := m.extractTarball(tarballPath, extractDir); err != nil { - m.updateStatus("error", 40, "Failed to extract update") - return fmt.Errorf("failed to extract update: %w", err) + extractErr := 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...") @@ -387,8 +392,9 @@ func (m *Manager) ApplyUpdate(ctx context.Context, downloadURL string) error { // Create backup backupPath, err := m.createBackup() if err != nil { - m.updateStatus("error", 60, "Failed to create backup") - return fmt.Errorf("failed to create backup: %w", err) + backupErr := 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") @@ -411,12 +417,13 @@ func (m *Manager) ApplyUpdate(ctx context.Context, downloadURL string) error { log.Info().Msg("Applying update files") 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 if restoreErr := m.restoreBackup(backupPath); restoreErr != nil { 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...") @@ -1047,7 +1054,7 @@ func (m *Manager) applyUpdateFiles(extractDir string) error { } // 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.status = UpdateStatus{ Status: status, @@ -1055,6 +1062,10 @@ func (m *Manager) updateStatus(status string, progress int, message string) { Message: message, 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 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 func (m *Manager) cleanupOldTempDirs() { // Check multiple locations where temp dirs might exist