From ac2118db14d0905d6b782984565da911b4298966 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 22 Apr 2026 15:41:39 +0200 Subject: [PATCH] fix(notifications): replace aggressive EventSource close with threshold-based 401-aware retry On CLOSED readyState, probes session and redirects to /login only on 401. On CONNECTING, counts consecutive errors and closes + probes only after 3 failures, preventing infinite retries without killing transient reconnects. Closes #203 Co-Authored-By: Claude Sonnet 4.6 --- .../lib/stores/notifications.svelte.spec.ts | 96 ++++++++++++++++++- .../src/lib/stores/notifications.svelte.ts | 26 ++++- 2 files changed, 118 insertions(+), 4 deletions(-) diff --git a/frontend/src/lib/stores/notifications.svelte.spec.ts b/frontend/src/lib/stores/notifications.svelte.spec.ts index 01acbd21..bb2a0ca6 100644 --- a/frontend/src/lib/stores/notifications.svelte.spec.ts +++ b/frontend/src/lib/stores/notifications.svelte.spec.ts @@ -5,8 +5,13 @@ let lastEventSource: MockEventSource | null = null; let eventSourceCount = 0; class MockEventSource { + static CONNECTING = 0; + static OPEN = 1; + static CLOSED = 2; + + readyState = MockEventSource.CONNECTING; onopen: (() => void) | null = null; - onerror: (() => void) | null = null; + onerror: (() => void | Promise) | null = null; close = vi.fn(); private listeners: Record void)[]> = {}; @@ -34,13 +39,18 @@ vi.stubGlobal('EventSource', MockEventSource); const mockFetch = vi.fn(); vi.stubGlobal('fetch', mockFetch); -const { notificationStore, __resetForTest } = await import('./notifications.svelte'); +const { notificationStore, __resetForTest, __setNavigateForTest } = + await import('./notifications.svelte'); + +let navigateSpy: ReturnType; beforeEach(() => { mockFetch.mockReset(); mockFetch.mockResolvedValue(new Response(JSON.stringify({ count: 0 }), { status: 200 })); lastEventSource = null; eventSourceCount = 0; + navigateSpy = vi.fn(); + __setNavigateForTest(navigateSpy); __resetForTest(); }); @@ -106,3 +116,85 @@ describe('notificationStore (singleton)', () => { expect(notificationStore.unreadCount).toBe(0); }); }); + +describe('notificationStore onerror handler', () => { + it('redirects to /login when readyState is CLOSED and server returns 401', async () => { + mockFetch.mockResolvedValue(new Response(null, { status: 401 })); + notificationStore.init(); + const es = lastEventSource!; + es.readyState = MockEventSource.CLOSED; + + await es.onerror?.(); + + expect(navigateSpy).toHaveBeenCalledWith('/login'); + }); + + it('does not redirect when readyState is CLOSED and session is still valid', async () => { + notificationStore.init(); + const es = lastEventSource!; + es.readyState = MockEventSource.CLOSED; + + await es.onerror?.(); + + expect(navigateSpy).not.toHaveBeenCalled(); + }); + + it('does not close or redirect before the error threshold when readyState is CONNECTING', async () => { + notificationStore.init(); + const es = lastEventSource!; + es.readyState = MockEventSource.CONNECTING; + + await es.onerror?.(); + await es.onerror?.(); + + expect(es.close).not.toHaveBeenCalled(); + expect(navigateSpy).not.toHaveBeenCalled(); + }); + + it('closes and redirects after 3 consecutive CONNECTING errors when session returns 401', async () => { + mockFetch.mockResolvedValue(new Response(null, { status: 401 })); + notificationStore.init(); + const es = lastEventSource!; + es.readyState = MockEventSource.CONNECTING; + + await es.onerror?.(); + await es.onerror?.(); + await es.onerror?.(); + + expect(es.close).toHaveBeenCalledTimes(1); + expect(navigateSpy).toHaveBeenCalledWith('/login'); + }); + + it('closes but does not redirect after threshold when session is still valid', async () => { + notificationStore.init(); + const es = lastEventSource!; + es.readyState = MockEventSource.CONNECTING; + + await es.onerror?.(); + await es.onerror?.(); + await es.onerror?.(); + + expect(es.close).toHaveBeenCalledTimes(1); + expect(navigateSpy).not.toHaveBeenCalled(); + }); + + it('resets error count after a successful reconnect (onopen)', async () => { + notificationStore.init(); + const es = lastEventSource!; + es.readyState = MockEventSource.CONNECTING; + + // Two errors — not yet at threshold + await es.onerror?.(); + await es.onerror?.(); + + // Successful reconnect resets counter + es.onopen?.(); + + // Two more errors — should still be below threshold + await es.onerror?.(); + await es.onerror?.(); + + expect(es.close).not.toHaveBeenCalled(); + expect(navigateSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/lib/stores/notifications.svelte.ts b/frontend/src/lib/stores/notifications.svelte.ts index 28ac9eb9..b2aa2b28 100644 --- a/frontend/src/lib/stores/notifications.svelte.ts +++ b/frontend/src/lib/stores/notifications.svelte.ts @@ -6,6 +6,10 @@ let notifications = $state([]); let unreadCount = $state(0); let eventSource: EventSource | null = null; let refCount = 0; +let errorCount = 0; +let navigate: (url: string) => void = (url) => { + window.location.href = url; +}; async function fetchNotifications(): Promise { try { @@ -69,9 +73,22 @@ function init(): void { }); eventSource.onopen = () => { fetchUnreadCount(); + errorCount = 0; }; - eventSource.onerror = () => { - eventSource?.close(); + eventSource.onerror = async () => { + if (eventSource?.readyState === EventSource.CLOSED) { + const res = await fetch('/api/notifications/unread-count'); + if (res.status === 401) navigate('/login'); + return; + } + errorCount += 1; + if (errorCount >= 3) { + eventSource?.close(); + eventSource = null; + errorCount = 0; + const res = await fetch('/api/notifications/unread-count'); + if (res.status === 401) navigate('/login'); + } }; } @@ -88,10 +105,15 @@ export function __resetForTest(): void { eventSource?.close(); eventSource = null; refCount = 0; + errorCount = 0; notifications = []; unreadCount = 0; } +export function __setNavigateForTest(fn: (url: string) => void): void { + navigate = fn; +} + export const notificationStore = { get notifications() { return notifications;