fix: improve error handling for node connection tests (addresses #362, #363)

- Handle standalone nodes properly in diagnostics (fixes #362)
  - Try /nodes endpoint first which works for both clustered and standalone
  - Make cluster status optional, not required for connection success
  - Set cluster node count to 1 for standalone nodes

- Fix test connection UI showing success styling on errors (fixes #363)
  - Return proper HTTP error status when test-config endpoint fails
  - Clean up error messages to remove "API request failed: XXX" prefix
  - Add debug logging to trace test result states
  - Ensure consistent error status codes from all test endpoints

Both issues reported connection problems - standalone nodes failing to connect
and error messages appearing with success styling. These fixes ensure proper
handling of both scenarios.
This commit is contained in:
Pulse Monitor 2025-08-28 11:56:02 +00:00
parent 63f18afdef
commit 4f1f77e0ae
3 changed files with 50 additions and 15 deletions

View file

@ -205,9 +205,15 @@ export const NodeModal: Component<NodeModalProps> = (props) => {
message: result.message || 'Connection successful'
});
} catch (error) {
console.error('Test existing node error:', error);
let errorMessage = 'Connection failed';
if (error instanceof Error) {
// Remove "API request failed: XXX " prefix if present
errorMessage = error.message.replace(/^API request failed: \d{3}\s*/, '');
}
setTestResult({
status: 'error',
message: error instanceof Error ? error.message : 'Connection failed'
message: errorMessage
});
} finally {
setIsTesting(false);
@ -261,9 +267,15 @@ export const NodeModal: Component<NodeModalProps> = (props) => {
isCluster: result.isCluster
});
} catch (error) {
console.error('Test connection error:', error);
let errorMessage = 'Connection failed';
if (error instanceof Error) {
// Remove "API request failed: XXX " prefix if present
errorMessage = error.message.replace(/^API request failed: \d{3}\s*/, '');
}
setTestResult({
status: 'error',
message: error instanceof Error ? error.message : 'Connection failed'
message: errorMessage
});
} finally {
setIsTesting(false);
@ -1256,6 +1268,11 @@ export const NodeModal: Component<NodeModalProps> = (props) => {
{/* Test Result */}
<Show when={testResult()}>
{(() => {
const result = testResult();
console.log('Test result display:', { status: result?.status, message: result?.message });
return null;
})()}
<div class={`mx-6 p-3 rounded-lg text-sm ${
testResult()?.status === 'success'
? 'bg-green-50 dark:bg-green-900/20 border border-green-200 dark:border-green-800 text-green-800 dark:text-green-200'

View file

@ -1444,7 +1444,11 @@ func (h *ConfigHandlers) HandleTestNodeConfig(w http.ResponseWriter, r *http.Req
return
}
// Return appropriate HTTP status based on test result
w.Header().Set("Content-Type", "application/json")
if testResult["status"] == "error" {
w.WriteHeader(http.StatusBadRequest)
}
json.NewEncoder(w).Encode(testResult)
}
@ -1561,7 +1565,11 @@ func (h *ConfigHandlers) HandleTestNode(w http.ResponseWriter, r *http.Request)
}
}
// Return appropriate HTTP status based on test result
w.Header().Set("Content-Type", "application/json")
if testResult["status"] == "error" {
w.WriteHeader(http.StatusBadRequest)
}
json.NewEncoder(w).Encode(testResult)
}

View file

@ -156,32 +156,42 @@ func (r *Router) handleDiagnostics(w http.ResponseWriter, req *http.Request) {
nodeDiag.Connected = false
nodeDiag.Error = err.Error()
} else {
// Try to get cluster status
if clusterStatus, err := client.GetClusterStatus(ctx); err != nil {
// Try to get nodes first (this should work for both clustered and standalone)
nodes, err := client.GetNodes(ctx)
if err != nil {
nodeDiag.Connected = false
nodeDiag.Error = "Connection established but cluster status failed: " + err.Error()
nodeDiag.Error = "Failed to connect to Proxmox API: " + err.Error()
} else {
nodeDiag.Connected = true
nodeDiag.ClusterInfo = &ClusterInfo{
Nodes: len(clusterStatus),
}
// Get node details
if nodes, err := client.GetNodes(ctx); err == nil && len(nodes) > 0 {
// Set node details
if len(nodes) > 0 {
nodeDiag.Details = &NodeDetails{
NodeCount: len(nodes),
}
// Get version from first node
if len(nodes) > 0 {
if status, err := client.GetNodeStatus(ctx, nodes[0].Node); err == nil && status != nil {
if status.PVEVersion != "" {
nodeDiag.Details.Version = status.PVEVersion
}
if status, err := client.GetNodeStatus(ctx, nodes[0].Node); err == nil && status != nil {
if status.PVEVersion != "" {
nodeDiag.Details.Version = status.PVEVersion
}
}
}
// Try to get cluster status (this may fail for standalone nodes, which is OK)
if clusterStatus, err := client.GetClusterStatus(ctx); err == nil {
nodeDiag.ClusterInfo = &ClusterInfo{
Nodes: len(clusterStatus),
}
} else {
// Standalone node or cluster status not available
// This is not an error - standalone nodes don't have cluster status
log.Debug().Str("node", node.Name).Msg("Cluster status not available (likely standalone node)")
nodeDiag.ClusterInfo = &ClusterInfo{
Nodes: 1, // Standalone node
}
}
// Run VM disk monitoring check
nodeDiag.VMDiskCheck = r.checkVMDiskMonitoring(ctx, client, node.Name)
}