fix: Ensure errors from proxy are shown to the user (#601)

1. Ensures errors from the ch proxy are properly returned 
2. Adds a connection check hook to periodically check connections and display a toast if connections are bad
3. Fixes the ConnectionForm to ensure that toast notifications are shown when test connections fail or when there's an error saving the form.

.changeset/nervous-timers-dream.md

@@ -0,0 +1,5 @@
+---
+"@hyperdx/api": patch
+---
+
+fix: Ensure errors from proxy are shown to the user

packages/api/src/routers/api/clickhouseProxy.ts

@@ -28,10 +28,33 @@ router.post(
           'X-ClickHouse-Key': password || '',
         },
         signal: AbortSignal.timeout(2000),
-      }).then(res => res.json());
-      return res.json({ success: result === 1 });
-    } catch (e) {
-      return res.json({ success: false });
+      });
+      // For status codes 204-399
+      if (!result.ok) {
+        const errorText = await result.text();
+        return res.status(result.status).json({
+          success: false,
+          error: errorText || 'Error connecting to ClickHouse server',
+        });
+      }
+      const data = await result.json();
+      return res.json({ success: data === 1 });
+    } catch (e: any) {
+      // fetch returns a 400+ error and throws
+      console.error(e);
+      const errorMessage =
+        e.cause?.code === 'ENOTFOUND'
+          ? `Unable to resolve host: ${e.cause.hostname}`
+          : e.cause?.message ||
+            e.message ||
+            'Error connecting to ClickHouse server';
+
+      return res.status(500).json({
+        success: false,
+        error:
+          errorMessage +
+          ', please check the host and credentials and try again.',
+      });
     }
   },
 );
@@ -105,7 +128,16 @@ router.get(
             }
           },
           error: (err, _req, _res) => {
-            console.error(err);
+            console.error('Proxy error:', err);
+            res.writeHead(500, {
+              'Content-Type': 'application/json',
+            });
+            res.end(
+              JSON.stringify({
+                success: false,
+                error: err.message || 'Failed to connect to ClickHouse server',
+              }),
+            );
           },
         },
         // ...(config.IS_DEV && {
@@ -113,7 +145,11 @@ router.get(
         // }),
       })(req, res, next);
     } catch (e) {
-      next(e);
+      console.error('Router error:', e);
+      res.status(500).json({
+        success: false,
+        error: e instanceof Error ? e.message : 'Internal server error',
+      });
     }
   },
 );

packages/app/src/ThemeWrapper.tsx

@@ -8,6 +8,8 @@ import {
 } from '@mantine/core';
 import { Notifications } from '@mantine/notifications';
 
+import { useConnectionHealth } from '@/hooks/useConnectionHealth';
+
 const makeTheme = ({
   fontFamily = '"IBM Plex Sans", monospace',
 }: {
@@ -149,6 +151,9 @@ export const ThemeWrapper = ({
 }) => {
   const theme = React.useMemo(() => makeTheme({ fontFamily }), [fontFamily]);
 
+  // Add connection health monitoring
+  useConnectionHealth();
+
   return (
     <MantineProvider forceColorScheme="dark" theme={theme}>
       <Notifications zIndex={999999} />

packages/app/src/api.ts

@@ -761,7 +761,7 @@ const api = {
             username,
             password,
           },
-        }).json() as Promise<{ success: boolean }>,
+        }).json() as Promise<{ success: boolean; error?: string }>,
     });
   },
 };

packages/app/src/components/ConnectionForm.tsx

@@ -1,7 +1,8 @@
-import { useCallback, useState } from 'react';
+import { useCallback, useEffect, useState } from 'react';
 import { useForm } from 'react-hook-form';
 import { testLocalConnection } from '@hyperdx/common-utils/dist/clickhouse';
 import { Box, Button, Flex, Group, Stack, Text } from '@mantine/core';
+import { notifications } from '@mantine/notifications';
 
 import api from '@/api';
 import { InputControlled } from '@/components/InputControlled';
@@ -15,15 +16,20 @@ import {
 
 import ConfirmDeleteMenu from './ConfirmDeleteMenu';
 
+enum TestConnectionState {
+  Loading = 'loading',
+  Valid = 'valid',
+  Invalid = 'invalid',
+}
+
 function useTestConnection({
   getValues,
 }: {
   getValues: (name: string) => string;
 }) {
   const testConnection = api.useTestConnection();
-  const [testConnectionState, setTestConnectionState] = useState<
-    null | 'loading' | 'valid' | 'invalid'
-  >(null);
+  const [testConnectionState, setTestConnectionState] =
+    useState<TestConnectionState | null>(null);
 
   const handleTestConnection = useCallback(async () => {
     const host = getValues('host');
@@ -34,15 +40,29 @@ function useTestConnection({
       return;
     }
 
-    setTestConnectionState('loading');
+    setTestConnectionState(TestConnectionState.Loading);
 
     if (IS_LOCAL_MODE) {
       try {
         const result = await testLocalConnection({ host, username, password });
-        setTestConnectionState(result ? 'valid' : 'invalid');
+        if (result) {
+          setTestConnectionState(TestConnectionState.Valid);
+        } else {
+          setTestConnectionState(TestConnectionState.Invalid);
+          notifications.show({
+            color: 'red',
+            message: 'Connection test failed',
+            autoClose: 5000,
+          });
+        }
       } catch (e) {
         console.error(e);
-        setTestConnectionState('invalid');
+        setTestConnectionState(TestConnectionState.Invalid);
+        notifications.show({
+          color: 'red',
+          message: e.message,
+          autoClose: 5000,
+        });
       }
     } else {
       try {
@@ -51,10 +71,24 @@ function useTestConnection({
           username,
           password,
         });
-        setTestConnectionState(result.success ? 'valid' : 'invalid');
-      } catch (e) {
-        console.error(e);
-        setTestConnectionState('invalid');
+        if (result.success) {
+          setTestConnectionState(TestConnectionState.Valid);
+        } else {
+          setTestConnectionState(TestConnectionState.Invalid);
+          notifications.show({
+            color: 'red',
+            message: result.error || 'Connection test failed',
+            autoClose: 5000,
+          });
+        }
+      } catch (error: any) {
+        const body = await error.response?.json();
+        setTestConnectionState(TestConnectionState.Invalid);
+        notifications.show({
+          color: 'red',
+          message: body?.error ?? 'Failed to test connection',
+          autoClose: 5000,
+        });
       }
     }
 
@@ -105,8 +139,20 @@ export function ConnectionForm({
         { connection: data },
         {
           onSuccess: () => {
+            notifications.show({
+              color: 'green',
+              message: 'Connection created successfully',
+            });
             onSave?.();
           },
+          onError: () => {
+            notifications.show({
+              color: 'red',
+              message:
+                'Error creating connection, please check the host and credentials and try again.',
+              autoClose: 5000,
+            });
+          },
         },
       );
     } else {
@@ -114,8 +160,20 @@ export function ConnectionForm({
         { connection: data, id: connection.id },
         {
           onSuccess: () => {
+            notifications.show({
+              color: 'green',
+              message: 'Connection updated successfully',
+            });
             onSave?.();
           },
+          onError: () => {
+            notifications.show({
+              color: 'red',
+              message:
+                'Error updating connection, please check the host and credentials and try again.',
+              autoClose: 5000,
+            });
+          },
         },
       );
     }
@@ -207,12 +265,6 @@ export function ConnectionForm({
             </Flex>
           )}
         </Box>
-        {createConnection.isError && (
-          <Text c="red.7" size="sm">
-            Error creating connection, please check the host and credentials and
-            try again.
-          </Text>
-        )}
         <Group justify="space-between">
           <Group gap="xs" justify="flex-start">
             <Button
@@ -229,12 +281,16 @@ export function ConnectionForm({
               variant="subtle"
               type="button"
               onClick={handleTestConnection}
-              loading={testConnectionState === 'loading'}
-              color={testConnectionState === 'invalid' ? 'yellow' : 'teal'}
+              loading={testConnectionState === TestConnectionState.Loading}
+              color={
+                testConnectionState === TestConnectionState.Invalid
+                  ? 'yellow'
+                  : 'teal'
+              }
             >
-              {testConnectionState === 'valid' ? (
+              {testConnectionState === TestConnectionState.Valid ? (
                 <>Connection successful</>
-              ) : testConnectionState === 'invalid' ? (
+              ) : testConnectionState === TestConnectionState.Invalid ? (
                 <>Unable to connect</>
               ) : (
                 'Test Connection'

packages/app/src/hooks/__tests__/useConnectionHealth.test.tsx

@@ -0,0 +1,119 @@
+import { notifications } from '@mantine/notifications';
+import { act, renderHook } from '@testing-library/react';
+
+import api from '@/api';
+import { useConnections } from '@/connection';
+
+import { useConnectionHealth } from '../useConnectionHealth';
+
+// Mock dependencies
+jest.mock('@mantine/notifications');
+jest.mock('@/api');
+jest.mock('@/connection');
+
+describe('useConnectionHealth', () => {
+  const mockConnections = [
+    {
+      id: '1',
+      name: 'Test Connection 1',
+      host: 'localhost',
+      username: 'user1',
+      password: 'pass1',
+    },
+  ];
+
+  beforeEach(() => {
+    jest.useFakeTimers();
+    jest.clearAllMocks();
+
+    (useConnections as jest.Mock).mockReturnValue({
+      data: mockConnections,
+    });
+  });
+
+  afterEach(() => {
+    jest.useRealTimers();
+  });
+
+  describe('connection monitoring', () => {
+    it('should start monitoring after initial delay', async () => {
+      const testConnectionMock = jest.fn().mockResolvedValue({ success: true });
+      (api.useTestConnection as jest.Mock).mockReturnValue({
+        mutateAsync: testConnectionMock,
+      });
+
+      renderHook(() => useConnectionHealth());
+
+      await act(async () => {
+        jest.advanceTimersByTime(5000);
+      });
+
+      expect(testConnectionMock).toHaveBeenCalledWith({
+        host: mockConnections[0].host,
+        username: mockConnections[0].username,
+        password: mockConnections[0].password,
+      });
+    });
+
+    it('should show notification when connection fails', async () => {
+      const testConnectionMock = jest.fn().mockResolvedValue({
+        success: false,
+        error: 'Connection failed',
+      });
+      (api.useTestConnection as jest.Mock).mockReturnValue({
+        mutateAsync: testConnectionMock,
+      });
+
+      renderHook(() => useConnectionHealth());
+
+      await act(async () => {
+        jest.advanceTimersByTime(5000);
+      });
+
+      expect(notifications.show).toHaveBeenCalledWith({
+        id: 'connection-error-1',
+        color: 'red',
+        message:
+          'Connection "Test Connection 1" is not responding: Connection failed',
+        autoClose: false,
+      });
+    });
+
+    it('should respect retry delay for failed connections', async () => {
+      const testConnectionMock = jest.fn().mockResolvedValue({
+        success: false,
+      });
+      (api.useTestConnection as jest.Mock).mockReturnValue({
+        mutateAsync: testConnectionMock,
+      });
+
+      renderHook(() => useConnectionHealth());
+
+      await act(async () => {
+        jest.advanceTimersByTime(5000);
+      });
+
+      await act(async () => {
+        jest.advanceTimersByTime(10000);
+      });
+
+      expect(testConnectionMock).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('cleanup', () => {
+    it('should clean up intervals on unmount', () => {
+      const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout');
+      const clearIntervalSpy = jest.spyOn(global, 'clearInterval');
+
+      const { unmount } = renderHook(() => useConnectionHealth());
+
+      act(() => {
+        unmount();
+      });
+
+      expect(clearTimeoutSpy).toHaveBeenCalled();
+      expect(clearIntervalSpy).toHaveBeenCalled();
+    });
+  });
+});

packages/app/src/hooks/useConnectionHealth.ts

@@ -0,0 +1,131 @@
+import { useCallback, useEffect, useState } from 'react';
+import { notifications } from '@mantine/notifications';
+
+import api from '@/api';
+import { Connection, useConnections } from '@/connection';
+
+const INITIAL_DELAY = 5000;
+const CHECK_INTERVAL = 5 * 60 * 1000;
+const RETRY_DELAY = 15 * 1000;
+
+export function useConnectionHealth() {
+  const { data: connections } = useConnections();
+  const testConnection = api.useTestConnection();
+  const [failedConnections, setFailedConnections] = useState<
+    Map<string, number>
+  >(new Map());
+  const [isChecking, setIsChecking] = useState(false);
+
+  /* Simple function for showing notifications */
+  const showNotification = useCallback(
+    (
+      connectionId: string,
+      connectionName: string,
+      isError: boolean,
+      message?: string,
+    ) => {
+      notifications.show({
+        id: `connection-${isError ? 'error' : 'restored'}-${connectionId}`,
+        color: isError ? 'red' : 'green',
+        message: isError
+          ? `Connection "${connectionName}" is not responding: ${message}`
+          : `Connection "${connectionName}" has been restored`,
+        autoClose: isError ? false : 5000,
+      });
+    },
+    [],
+  );
+
+  /* Maintains a map of connection IDs to their last failed check timestamp */
+  const updateFailedConnections = useCallback(
+    (connectionId: string, shouldAdd: boolean, timestamp?: number) => {
+      setFailedConnections(prev => {
+        const next = new Map(prev);
+        if (shouldAdd) {
+          next.set(connectionId, timestamp || Date.now());
+        } else {
+          next.delete(connectionId);
+        }
+        return next;
+      });
+    },
+    [],
+  );
+
+  /* Checks if a single connection is failing and shows a notification if it is */
+  const checkConnection = useCallback(
+    async (connection: Connection) => {
+      const now = Date.now();
+      const lastCheckTime = failedConnections.get(connection.id) || 0;
+
+      if (now - lastCheckTime < RETRY_DELAY) {
+        return;
+      }
+
+      try {
+        const result = await testConnection.mutateAsync({
+          host: connection.host,
+          username: connection.username,
+          password: connection.password,
+        });
+
+        const wasFailedPreviously = failedConnections.has(connection.id);
+
+        if (!result.success) {
+          if (!wasFailedPreviously) {
+            updateFailedConnections(connection.id, true, now);
+            showNotification(
+              connection.id,
+              connection.name,
+              true,
+              result.error,
+            );
+          }
+        } else if (wasFailedPreviously) {
+          updateFailedConnections(connection.id, false);
+          showNotification(connection.id, connection.name, false);
+        }
+      } catch (error: any) {
+        if (!failedConnections.has(connection.id)) {
+          const body = await error.response?.json();
+          const errorMessage = body?.error ?? error.message;
+          updateFailedConnections(connection.id, true, now);
+          showNotification(connection.id, connection.name, true, errorMessage);
+        }
+      }
+    },
+    [
+      testConnection,
+      failedConnections,
+      updateFailedConnections,
+      showNotification,
+    ],
+  );
+
+  /* Checks all connections and shows notifications if they are failing */
+  const checkConnections = useCallback(async () => {
+    if (!connections?.length || isChecking) return;
+    setIsChecking(true);
+
+    try {
+      for (const connection of connections) {
+        await checkConnection(connection);
+        // Add small delay between checks to prevent overwhelming the server
+        await new Promise(resolve => setTimeout(resolve, 1000));
+      }
+    } finally {
+      setIsChecking(false);
+    }
+  }, [connections, isChecking, checkConnection]);
+
+  /* Sets up the initial check and the interval check */
+  useEffect(() => {
+    const initialCheckTimeout = setTimeout(checkConnections, INITIAL_DELAY);
+    const interval = setInterval(checkConnections, CHECK_INTERVAL);
+
+    return () => {
+      clearTimeout(initialCheckTimeout);
+      clearInterval(interval);
+    };
+  }, [checkConnections]);
+}