From 449d77504f70ce99a23a441b069319c8ceb88ed2 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 5 Nov 2025 18:40:39 +0000 Subject: [PATCH] Improve PMG connection testing to validate metrics endpoints Related to #551 Enhanced the PMG connection test to actually validate the metrics endpoints that Pulse uses for monitoring, rather than only checking the version endpoint. This provides users with immediate feedback if their PMG credentials lack the necessary permissions to collect metrics. Backend changes: - Test mail statistics, cluster status, and quarantine endpoints during connection test (internal/api/config_handlers.go:1695-1714) - Return warnings array in test response when endpoints are unavailable - Increased timeout from 10s to 15s to accommodate multiple endpoint checks - Added warning logs for failed endpoint checks Frontend changes: - Added showWarning() toast function for warning messages - Enhanced NodeModal to display warning status with amber styling - Added warnings list display in test results UI - Updated Settings.tsx to show warnings from connection tests This change helps users identify permission issues immediately rather than discovering later that metrics aren't being collected despite a "successful" connection. --- .../src/components/Settings/NodeModal.tsx | 33 +++++++++++++++++-- .../src/components/Settings/Settings.tsx | 10 ++++-- frontend-modern/src/utils/toast.ts | 2 ++ internal/api/config_handlers.go | 30 ++++++++++++++++- 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/frontend-modern/src/components/Settings/NodeModal.tsx b/frontend-modern/src/components/Settings/NodeModal.tsx index 434dd7f44..725ea39e3 100644 --- a/frontend-modern/src/components/Settings/NodeModal.tsx +++ b/frontend-modern/src/components/Settings/NodeModal.tsx @@ -57,6 +57,7 @@ export const NodeModal: Component = (props) => { status: string; message: string; isCluster?: boolean; + warnings?: string[]; } | null>(null); const [isTesting, setIsTesting] = createSignal(false); @@ -362,9 +363,10 @@ export const NodeModal: Component = (props) => { try { const result = await NodesAPI.testConnection(testData as NodeConfig); setTestResult({ - status: 'success', + status: result.warnings && result.warnings.length > 0 ? 'warning' : 'success', message: result.message || 'Connection successful', isCluster: result.isCluster, + warnings: result.warnings, }); } catch (error) { logger.error('Test connection error:', error); @@ -1863,7 +1865,9 @@ export const NodeModal: Component = (props) => { 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' - : 'bg-red-50 dark:bg-red-900/20 border border-red-200 dark:border-red-800 text-red-800 dark:text-red-200' + : testResult()?.status === 'warning' + ? 'bg-amber-50 dark:bg-amber-900/20 border border-amber-200 dark:border-amber-800 text-amber-800 dark:text-amber-200' + : 'bg-red-50 dark:bg-red-900/20 border border-red-200 dark:border-red-800 text-red-800 dark:text-red-200' }`} >
@@ -1881,6 +1885,19 @@ export const NodeModal: Component = (props) => { + + + + + = (props) => { -
+

{testResult()?.message}

✨ Cluster detected! All cluster nodes will be automatically added.

+ 0}> +
+

Warnings:

+
    + + {(warning) =>
  • • {warning}
    • } +     +
+
+
diff --git a/frontend-modern/src/components/Settings/Settings.tsx b/frontend-modern/src/components/Settings/Settings.tsx index c248b219e..2c9bb4a51 100644 --- a/frontend-modern/src/components/Settings/Settings.tsx +++ b/frontend-modern/src/components/Settings/Settings.tsx @@ -12,7 +12,7 @@ import { import type { JSX } from 'solid-js'; import { useNavigate, useLocation } from '@solidjs/router'; import { useWebSocket } from '@/App'; -import { showSuccess, showError } from '@/utils/toast'; +import { showSuccess, showError, showWarning } from '@/utils/toast'; import { copyToClipboard } from '@/utils/clipboard'; import { getPulsePort, getPulseWebSocketUrl } from '@/utils/url'; import { logger } from '@/utils/logger'; @@ -1843,7 +1843,13 @@ const Settings: Component = (props) => { // Use the existing node test endpoint which uses stored credentials const result = await NodesAPI.testExistingNode(nodeId); if (result.status === 'success') { - showSuccess(result.message || 'Connection successful'); + // Check for warnings in the response + if (result.warnings && Array.isArray(result.warnings) && result.warnings.length > 0) { + const warningMessage = result.message + '\n\nWarnings:\n' + result.warnings.map((w: string) => '• ' + w).join('\n'); + showWarning(warningMessage); + } else { + showSuccess(result.message || 'Connection successful'); + } } else { throw new Error(result.message || 'Connection failed'); } diff --git a/frontend-modern/src/utils/toast.ts b/frontend-modern/src/utils/toast.ts index b8aadff04..12aa16859 100644 --- a/frontend-modern/src/utils/toast.ts +++ b/frontend-modern/src/utils/toast.ts @@ -23,3 +23,5 @@ export const showSuccess = (title: string, message?: string, duration?: number) showToast('success', title, message, duration); export const showError = (title: string, message?: string, duration?: number) => showToast('error', title, message, duration); +export const showWarning = (title: string, message?: string, duration?: number) => + showToast('warning', title, message, duration); diff --git a/internal/api/config_handlers.go b/internal/api/config_handlers.go index d4d9f2ae1..a7a6e6a62 100644 --- a/internal/api/config_handlers.go +++ b/internal/api/config_handlers.go @@ -1675,7 +1675,7 @@ func (h *ConfigHandlers) HandleTestConnection(w http.ResponseWriter, r *http.Req return } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() version, err := tempClient.GetVersion(ctx) @@ -1692,10 +1692,34 @@ func (h *ConfigHandlers) HandleTestConnection(w http.ResponseWriter, r *http.Req } } + // Test actual metrics endpoints to ensure monitoring will work + warnings := []string{} + + // Test mail statistics endpoint (core PMG functionality) + if _, err := tempClient.GetMailStatistics(ctx, "day"); err != nil { + warnings = append(warnings, "Mail statistics endpoint unavailable - check user permissions") + log.Warn().Err(err).Msg("PMG connection test: mail statistics check failed") + } + + // Test cluster status endpoint + if _, err := tempClient.GetClusterStatus(ctx, true); err != nil { + warnings = append(warnings, "Cluster status endpoint unavailable") + log.Warn().Err(err).Msg("PMG connection test: cluster status check failed") + } + + // Test quarantine endpoint + if _, err := tempClient.GetQuarantineStatus(ctx, "spam"); err != nil { + warnings = append(warnings, "Quarantine endpoint unavailable") + log.Warn().Err(err).Msg("PMG connection test: quarantine check failed") + } + message := "Connected to PMG instance" if versionLabel != "" { message = fmt.Sprintf("Connected to PMG instance (version %s)", versionLabel) } + if len(warnings) > 0 { + message += " (some metrics may be unavailable - check logs for details)" + } response := map[string]interface{}{ "status": "success", @@ -1711,6 +1735,10 @@ func (h *ConfigHandlers) HandleTestConnection(w http.ResponseWriter, r *http.Req } } + if len(warnings) > 0 { + response["warnings"] = warnings + } + w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(response) }