From 16cf4c503e4e2dc8f476f971b424761e07e874f6 Mon Sep 17 00:00:00 2001 From: Abhigyan Patwari <126312502+abhigyanpatwari@users.noreply.github.com> Date: Sat, 4 Apr 2026 16:26:35 +0530 Subject: [PATCH] fix(web): replace aggressive heartbeat disconnect with graceful reconnection (#643) --- gitnexus-web/e2e/heartbeat-reconnect.spec.ts | 109 ++++++++++++++ gitnexus-web/src/App.tsx | 23 +-- gitnexus-web/src/services/backend-client.ts | 35 +++-- gitnexus-web/test/unit/heartbeat.test.ts | 147 +++++++++++++++++++ 4 files changed, 293 insertions(+), 21 deletions(-) create mode 100644 gitnexus-web/e2e/heartbeat-reconnect.spec.ts create mode 100644 gitnexus-web/test/unit/heartbeat.test.ts diff --git a/gitnexus-web/e2e/heartbeat-reconnect.spec.ts b/gitnexus-web/e2e/heartbeat-reconnect.spec.ts new file mode 100644 index 000000000..4fff83b58 --- /dev/null +++ b/gitnexus-web/e2e/heartbeat-reconnect.spec.ts @@ -0,0 +1,109 @@ +import { test, expect } from '@playwright/test'; + +/** + * E2E tests for heartbeat disconnect/reconnect behavior. + * + * Verifies the key regression: when the heartbeat fails, the UI shows a + * "reconnecting" banner instead of resetting to the onboarding screen. + * + * Strategy: block /api/heartbeat via route interception BEFORE loading the + * graph. The heartbeat EventSource can never connect, so onReconnecting + * fires on the first retry attempt. This reliably tests the banner behavior + * without depending on setOffline timing (which varies across CI environments). + */ + +const BACKEND_URL = process.env.BACKEND_URL ?? 'http://localhost:4747'; +const FRONTEND_URL = process.env.FRONTEND_URL ?? 'http://localhost:5173'; + +test.beforeAll(async () => { + if (process.env.E2E) return; + try { + const [backendRes, frontendRes] = await Promise.allSettled([ + fetch(`${BACKEND_URL}/api/repos`), + fetch(FRONTEND_URL), + ]); + if ( + backendRes.status === 'rejected' || + (backendRes.status === 'fulfilled' && !backendRes.value.ok) + ) { + test.skip(true, 'gitnexus serve not available'); + return; + } + if ( + frontendRes.status === 'rejected' || + (frontendRes.status === 'fulfilled' && !frontendRes.value.ok) + ) { + test.skip(true, 'Vite dev server not available'); + return; + } + if (backendRes.status === 'fulfilled') { + const repos = await backendRes.value.json(); + if (!repos.length) { + test.skip(true, 'No indexed repos'); + return; + } + } + } catch { + test.skip(true, 'servers not available'); + } +}); + +test.describe('Heartbeat Reconnect', () => { + test('shows reconnecting banner instead of onboarding reset when heartbeat is unavailable', async ({ + page, + }) => { + // Block the heartbeat BEFORE navigating — the EventSource will fail + // immediately on every connection attempt, triggering onReconnecting. + await page.route('**/api/heartbeat', (route) => route.abort('connectionrefused')); + + // Load the app and connect to a repo (all other endpoints work normally) + await page.goto('/'); + + const landingCard = page.locator('[data-testid="landing-repo-card"]').first(); + try { + await landingCard.waitFor({ state: 'visible', timeout: 15_000 }); + await landingCard.click(); + } catch { + // auto-connect may skip the landing screen + } + + // Wait for graph to load (heartbeat is blocked, but graph loads fine) + await expect(page.locator('[data-testid="status-ready"]')).toBeVisible({ timeout: 30_000 }); + + // The reconnecting banner should appear (heartbeat is failing) + const banner = page.getByText('Server connection lost'); + await expect(banner).toBeVisible({ timeout: 15_000 }); + + // The graph canvas should STILL be visible — NOT reset to onboarding + await expect(page.locator('canvas').first()).toBeVisible(); + }); + + test('banner clears when heartbeat becomes available', async ({ page }) => { + // Start with heartbeat blocked + await page.route('**/api/heartbeat', (route) => route.abort('connectionrefused')); + + await page.goto('/'); + const landingCard = page.locator('[data-testid="landing-repo-card"]').first(); + try { + await landingCard.waitFor({ state: 'visible', timeout: 15_000 }); + await landingCard.click(); + } catch { + // auto-connect may skip the landing screen + } + + await expect(page.locator('[data-testid="status-ready"]')).toBeVisible({ timeout: 30_000 }); + + // Verify banner appears + const banner = page.getByText('Server connection lost'); + await expect(banner).toBeVisible({ timeout: 15_000 }); + + // Unblock heartbeat — the real server is running, so reconnect will succeed + await page.unroute('**/api/heartbeat'); + + // Banner should disappear as heartbeat reconnects + await expect(banner).not.toBeVisible({ timeout: 30_000 }); + + // Graph should still be there + await expect(page.locator('[data-testid="status-ready"]')).toBeVisible(); + }); +}); diff --git a/gitnexus-web/src/App.tsx b/gitnexus-web/src/App.tsx index 7ce4c97df..97af8d14c 100644 --- a/gitnexus-web/src/App.tsx +++ b/gitnexus-web/src/App.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { AppStateProvider, useAppState } from './hooks/useAppState'; import { DropZone } from './components/DropZone'; import { LoadingOverlay } from './components/LoadingOverlay'; @@ -36,7 +36,6 @@ const AppContent = () => { refreshLLMSettings, initializeAgent, startEmbeddingsWithFallback, - embeddingStatus, codeReferences, selectedNode, isCodePanelOpen, @@ -49,6 +48,7 @@ const AppContent = () => { } = useAppState(); const graphCanvasRef = useRef(null); + const [serverDisconnected, setServerDisconnected] = useState(false); const handleServerConnect = useCallback( async (result: ConnectResult): Promise => { @@ -192,21 +192,18 @@ const AppContent = () => { // ── Server heartbeat: detect when server goes down while exploring ──────── // Uses SSE (EventSource) for instant detection — no polling delay. + // On disconnect: show a reconnecting banner instead of resetting to onboarding. + // The heartbeat retries indefinitely with capped backoff and recovers automatically. useEffect(() => { if (viewMode !== 'exploring') return; const cleanup = connectHeartbeat( - () => {}, // onConnect — already connected, no action needed - () => { - // Server went down — return to onboarding - setViewMode('onboarding'); - setGraph(null); - setProgress(null); - }, + () => setServerDisconnected(false), + () => setServerDisconnected(true), ); return cleanup; - }, [viewMode, setViewMode, setGraph, setProgress]); + }, [viewMode]); // Render based on view mode if (viewMode === 'onboarding') { @@ -296,6 +293,12 @@ const AppContent = () => { + {serverDisconnected && ( +
+ Server connection lost — reconnecting… +
+ )} + {/* Settings Panel (modal) */} => { }; /** - * Connect an SSE heartbeat to the backend. Fires `onDisconnect` when the - * server goes down (after one retry to avoid false positives from transient - * network hiccups). Returns a cleanup function. + * Connect an SSE heartbeat to the backend. Retries indefinitely with capped + * exponential backoff so transient hiccups don't reset the UI. + * + * - `onConnect` fires on every successful (re)connection. + * - `onReconnecting` fires on the first retry after a drop — use it to show + * a "reconnecting" banner while keeping the current view intact. + * + * Returns a cleanup function that tears down the EventSource and timers. */ -export const connectHeartbeat = (onConnect: () => void, onDisconnect: () => void): (() => void) => { +export const connectHeartbeat = ( + onConnect: () => void, + onReconnecting: () => void, +): (() => void) => { let closed = false; let retryTimer: ReturnType | null = null; let es: EventSource | null = null; let attempt = 0; - const MAX_RETRIES = 3; + /** Whether we've already fired onReconnecting for the current drop. */ + let notifiedReconnecting = false; + const MAX_BACKOFF_MS = 15_000; const connect = () => { if (closed) return; @@ -319,6 +329,7 @@ export const connectHeartbeat = (onConnect: () => void, onDisconnect: () => void es.onopen = () => { if (!closed) { attempt = 0; + notifiedReconnecting = false; onConnect(); } }; @@ -326,13 +337,15 @@ export const connectHeartbeat = (onConnect: () => void, onDisconnect: () => void es?.close(); es = null; if (closed) return; - if (attempt < MAX_RETRIES) { - const delay = 1_000 * Math.pow(2, attempt); - attempt++; - retryTimer = setTimeout(connect, delay); - } else { - onDisconnect(); + + if (!notifiedReconnecting) { + notifiedReconnecting = true; + onReconnecting(); } + + const delay = Math.min(1_000 * Math.pow(2, attempt), MAX_BACKOFF_MS); + attempt++; + retryTimer = setTimeout(connect, delay); }; }; diff --git a/gitnexus-web/test/unit/heartbeat.test.ts b/gitnexus-web/test/unit/heartbeat.test.ts new file mode 100644 index 000000000..b090f31f1 --- /dev/null +++ b/gitnexus-web/test/unit/heartbeat.test.ts @@ -0,0 +1,147 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { connectHeartbeat } from '../../src/services/backend-client'; + +// Mock EventSource to simulate SSE behavior +class MockEventSource { + onopen: (() => void) | null = null; + onerror: (() => void) | null = null; + closed = false; + + close() { + this.closed = true; + } +} + +let lastEventSource: MockEventSource | null = null; + +beforeEach(() => { + lastEventSource = null; + vi.stubGlobal( + 'EventSource', + vi.fn().mockImplementation(() => { + lastEventSource = new MockEventSource(); + return lastEventSource; + }), + ); + vi.useFakeTimers(); +}); + +afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); +}); + +describe('connectHeartbeat', () => { + it('calls onConnect when EventSource opens', () => { + const onConnect = vi.fn(); + const onReconnecting = vi.fn(); + connectHeartbeat(onConnect, onReconnecting); + + lastEventSource!.onopen!(); + expect(onConnect).toHaveBeenCalledOnce(); + expect(onReconnecting).not.toHaveBeenCalled(); + }); + + it('calls onReconnecting on first error, then retries', () => { + const onConnect = vi.fn(); + const onReconnecting = vi.fn(); + connectHeartbeat(onConnect, onReconnecting); + + // Simulate connection drop + lastEventSource!.onerror!(); + + expect(onReconnecting).toHaveBeenCalledOnce(); + expect(lastEventSource!.closed).toBe(true); + + // Advance past first retry delay (1s) + vi.advanceTimersByTime(1_000); + + // A new EventSource should have been created + expect(EventSource).toHaveBeenCalledTimes(2); + }); + + it('fires onReconnecting only once per disconnect', () => { + const onConnect = vi.fn(); + const onReconnecting = vi.fn(); + connectHeartbeat(onConnect, onReconnecting); + + // First error + lastEventSource!.onerror!(); + expect(onReconnecting).toHaveBeenCalledOnce(); + + // Second retry fires error again + vi.advanceTimersByTime(1_000); + lastEventSource!.onerror!(); + expect(onReconnecting).toHaveBeenCalledOnce(); // still 1 + + // Third retry fires error + vi.advanceTimersByTime(2_000); + lastEventSource!.onerror!(); + expect(onReconnecting).toHaveBeenCalledOnce(); // still 1 + }); + + it('retries indefinitely instead of giving up after 3 attempts', () => { + const onConnect = vi.fn(); + const onReconnecting = vi.fn(); + connectHeartbeat(onConnect, onReconnecting); + + // Simulate 10 consecutive failures — should never stop retrying + for (let i = 0; i < 10; i++) { + lastEventSource!.onerror!(); + // Advance past the max backoff (15s) to ensure the next retry fires + vi.advanceTimersByTime(16_000); + } + + // Should have created 11 EventSources (1 initial + 10 retries) + expect(EventSource).toHaveBeenCalledTimes(11); + }); + + it('resets reconnecting state when connection recovers', () => { + const onConnect = vi.fn(); + const onReconnecting = vi.fn(); + connectHeartbeat(onConnect, onReconnecting); + + // Drop + lastEventSource!.onerror!(); + expect(onReconnecting).toHaveBeenCalledOnce(); + + // Retry succeeds + vi.advanceTimersByTime(1_000); + lastEventSource!.onopen!(); + expect(onConnect).toHaveBeenCalledOnce(); + + // Drop again — should fire onReconnecting again (reset after recovery) + lastEventSource!.onerror!(); + expect(onReconnecting).toHaveBeenCalledTimes(2); + }); + + it('caps backoff at 15 seconds', () => { + const onConnect = vi.fn(); + const onReconnecting = vi.fn(); + connectHeartbeat(onConnect, onReconnecting); + + // Fail many times to push backoff past the cap + for (let i = 0; i < 6; i++) { + lastEventSource!.onerror!(); + // The delay for attempt i is min(1000 * 2^i, 15000) + // i=0: 1s, i=1: 2s, i=2: 4s, i=3: 8s, i=4: 15s (capped), i=5: 15s (capped) + vi.advanceTimersByTime(16_000); + } + + // All retries should have fired — 7 EventSources total + expect(EventSource).toHaveBeenCalledTimes(7); + }); + + it('stops retrying when cleanup is called', () => { + const onConnect = vi.fn(); + const onReconnecting = vi.fn(); + const cleanup = connectHeartbeat(onConnect, onReconnecting); + + lastEventSource!.onerror!(); + cleanup(); + + // Advance time — no new EventSource should be created + vi.advanceTimersByTime(30_000); + expect(EventSource).toHaveBeenCalledTimes(1); + }); +});