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.

Author: teeohhem

.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,15 +128,28 @@ 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 && {
         //   logger: console,
         // }),
       })(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,17 +139,41 @@ 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 {
       updateConnection.mutate(
         { 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();
+    });
+  });
+});