From 26133cf0cfdfa5f26208980156063a45cec94fed Mon Sep 17 00:00:00 2001 From: rcourtman Date: Tue, 16 Dec 2025 21:50:02 +0000 Subject: [PATCH] feat: Improve cluster endpoint error messages for users - Add sanitizeEndpointError() to transform raw Go errors into user-friendly messages - Transform 'context deadline exceeded' into helpful messages mentioning possible causes - Storage timeout errors now suggest checking PBS/NFS/Ceph backend connectivity - Connection refused, certificate errors, and auth errors get actionable hints - Apply sanitization everywhere cluster endpoint lastError is stored - Add comprehensive tests for all error transformations --- pkg/proxmox/cluster_client.go | 63 ++++++++++++++++++++++++++++-- pkg/proxmox/cluster_client_test.go | 40 +++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/pkg/proxmox/cluster_client.go b/pkg/proxmox/cluster_client.go index 48a4772..955c521 100644 --- a/pkg/proxmox/cluster_client.go +++ b/pkg/proxmox/cluster_client.go @@ -70,6 +70,61 @@ func isVMSpecificError(errStr string) bool { return false } +// sanitizeEndpointError transforms raw Go errors into user-friendly messages +// for display in the UI. The original error is preserved in logs. +func sanitizeEndpointError(errMsg string) string { + if errMsg == "" { + return errMsg + } + + lower := strings.ToLower(errMsg) + + // Context deadline exceeded - usually means slow API response + if strings.Contains(lower, "context deadline exceeded") { + // Check for specific causes + if strings.Contains(lower, "/storage") { + return "Request timed out - storage API slow (check for unreachable PBS/NFS/Ceph backends)" + } + if strings.Contains(lower, "pbs-") || strings.Contains(lower, ":8007") { + return "Request timed out - PBS storage backend unreachable" + } + return "Request timed out - Proxmox API may be slow or waiting on unreachable backend services" + } + + // Client timeout - similar to context deadline + if strings.Contains(lower, "client.timeout exceeded") { + return "Connection timed out - Proxmox API not responding in time" + } + + // Connection refused + if strings.Contains(lower, "connection refused") { + return "Connection refused - Proxmox API not running or firewall blocking" + } + + // No route to host + if strings.Contains(lower, "no route to host") { + return "Network unreachable - check network connectivity to Proxmox host" + } + + // TLS/certificate errors + if strings.Contains(lower, "certificate") || strings.Contains(lower, "x509") { + return "TLS certificate error - check SSL settings or add fingerprint" + } + + // Auth errors - keep these specific + if strings.Contains(lower, "authentication") || strings.Contains(lower, "401") || strings.Contains(lower, "403") { + return "Authentication failed - check API token or credentials" + } + + // PBS-specific errors + if strings.Contains(lower, "can't connect to") && strings.Contains(lower, ":8007") { + return "PBS storage unreachable - check Proxmox Backup Server connectivity" + } + + // Return original if no transformation applies + return errMsg +} + // NewClusterClient creates a new cluster-aware client func NewClusterClient(name string, config ClientConfig, endpoints []string) *ClusterClient { cc := &ClusterClient{ @@ -127,7 +182,7 @@ func (cc *ClusterClient) initialHealthCheck() { if err != nil { cc.mu.Lock() cc.nodeHealth[ep] = false - cc.lastError[ep] = err.Error() + cc.lastError[ep] = sanitizeEndpointError(err.Error()) cc.lastHealthCheck[ep] = time.Now() cc.mu.Unlock() log.Info(). @@ -155,7 +210,7 @@ func (cc *ClusterClient) initialHealthCheck() { fullClient, clientErr := NewClient(fullCfg) if clientErr != nil { cc.nodeHealth[ep] = false - cc.lastError[ep] = clientErr.Error() + cc.lastError[ep] = sanitizeEndpointError(clientErr.Error()) cc.lastHealthCheck[ep] = time.Now() log.Warn(). Str("cluster", cc.name). @@ -182,7 +237,7 @@ func (cc *ClusterClient) initialHealthCheck() { } else { // Real connectivity issue cc.nodeHealth[ep] = false - cc.lastError[ep] = err.Error() + cc.lastError[ep] = sanitizeEndpointError(err.Error()) cc.lastHealthCheck[ep] = time.Now() log.Info(). Str("cluster", cc.name). @@ -422,7 +477,7 @@ func (cc *ClusterClient) markUnhealthyWithError(endpoint string, errMsg string) cc.nodeHealth[endpoint] = false } if errMsg != "" { - cc.lastError[endpoint] = errMsg + cc.lastError[endpoint] = sanitizeEndpointError(errMsg) } cc.lastHealthCheck[endpoint] = time.Now() } diff --git a/pkg/proxmox/cluster_client_test.go b/pkg/proxmox/cluster_client_test.go index 1f55c39..04345f9 100644 --- a/pkg/proxmox/cluster_client_test.go +++ b/pkg/proxmox/cluster_client_test.go @@ -254,6 +254,46 @@ func TestIsVMSpecificError(t *testing.T) { } } +func TestSanitizeEndpointError(t *testing.T) { + tests := []struct { + name string + errMsg string + expected string + }{ + {"empty string", "", ""}, + {"generic error unchanged", "some random error", "some random error"}, + // Context deadline exceeded + {"context deadline basic", "context deadline exceeded", "Request timed out - Proxmox API may be slow or waiting on unreachable backend services"}, + {"context deadline storage", "Get /api2/json/nodes/delly/storage: context deadline exceeded", "Request timed out - storage API slow (check for unreachable PBS/NFS/Ceph backends)"}, + {"context deadline pbs", "pbs-backup context deadline exceeded", "Request timed out - PBS storage backend unreachable"}, + {"context deadline port 8007", "Can't connect to verdeclose:8007 context deadline exceeded", "Request timed out - PBS storage backend unreachable"}, + // Client timeout + {"client timeout", "Client.Timeout exceeded while awaiting headers", "Connection timed out - Proxmox API not responding in time"}, + // Connection refused + {"connection refused", "dial tcp 192.168.0.5:8006: connect: connection refused", "Connection refused - Proxmox API not running or firewall blocking"}, + // No route to host + {"no route to host", "dial tcp: no route to host", "Network unreachable - check network connectivity to Proxmox host"}, + // Certificate errors + {"certificate error", "x509: certificate signed by unknown authority", "TLS certificate error - check SSL settings or add fingerprint"}, + {"cert in message", "certificate has expired", "TLS certificate error - check SSL settings or add fingerprint"}, + // Auth errors + {"401 error", "api error 401: Unauthorized", "Authentication failed - check API token or credentials"}, + {"403 error", "status 403: Forbidden", "Authentication failed - check API token or credentials"}, + {"authentication keyword", "authentication failed: invalid token", "Authentication failed - check API token or credentials"}, + // PBS specific + {"pbs connect error", "Can't connect to verdeclose:8007 (Connection timed out)", "PBS storage unreachable - check Proxmox Backup Server connectivity"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := sanitizeEndpointError(tt.errMsg) + if result != tt.expected { + t.Errorf("sanitizeEndpointError(%q) = %q, want %q", tt.errMsg, result, tt.expected) + } + }) + } +} + func TestCalculateRateLimitBackoff(t *testing.T) { // Test that backoff increases with attempt number prev := time.Duration(0)