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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<void>) | null = null;
|
||||
close = vi.fn();
|
||||
private listeners: Record<string, ((e: MessageEvent) => 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<typeof vi.fn>;
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,6 +6,10 @@ let notifications = $state<NotificationItem[]>([]);
|
||||
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<void> {
|
||||
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;
|
||||
|
||||
Reference in New Issue
Block a user