Fix #288 Vite hot module reloading creating multiple SSE connections #290
Conversation
- move SSE (EventSource) connection to module level - manage EventSource as a singleton, closing open connection before reopening a new one
WalkthroughConsolidated EventSource management from component-scoped (useRef) to a module-level variable. Updated enable/disable and connect flows to operate on the shared EventSource, binding handlers directly and maintaining retry/backoff on errors. Connection state, logs, metrics, and models are reset on open. Public API signatures remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as UI
participant Provider as APIProvider (module)
participant SSE as EventSource (/api/events)
UI->>Provider: enableAPIEvents(true)
activate Provider
Provider->>Provider: connect()
alt existing apiEventSource
Provider->>SSE: close()
note right of Provider: Reset global apiEventSource to null
end
Provider->>SSE: new EventSource()
Provider->>Provider: set handlers (onopen, onmessage, onerror)
SSE-->>Provider: onopen
Provider->>Provider: reset logs/metrics/models<br/>set connected=true
SSE-->>Provider: onmessage
Provider->>Provider: process event
SSE-->>Provider: onerror
Provider->>SSE: close()
Provider->>Provider: schedule reconnect with backoff (setTimeout connect)
UI->>Provider: enableAPIEvents(false)
Provider->>SSE: close()
Provider->>Provider: apiEventSource = null
deactivate Provider
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
ui/src/contexts/APIProvider.tsx (4)
72-79: Fully disable the stream: clear pending retry, mark disabled, and reflect status.Currently disables SSE but can leave a pending reconnect and doesn’t update connectionStatus.
const enableAPIEvents = useCallback((enabled: boolean) => { if (!enabled) { - apiEventSource?.close(); - apiEventSource = null; + eventsEnabled = false; + apiEventSource?.close(); + apiEventSource = null; + if (retryTimer) { + clearTimeout(retryTimer); + retryTimer = null; + } + setConnectionState("disconnected"); setMetrics([]); return; }
80-83: Mark enabled before connecting.Ensures guards inside connect/onopen/onerror behave correctly.
- let retryCount = 0; + eventsEnabled = true; + let retryCount = 0;
142-148: Do not reconnect when disabled; de-dupe retries.Add guard and manage a single retry timer.
- apiEventSource.onerror = () => { - apiEventSource?.close(); - retryCount++; - const delay = Math.min(initialDelay * Math.pow(2, retryCount - 1), 5000); - setConnectionState("disconnected"); - setTimeout(connect, delay); - }; + apiEventSource.onerror = () => { + apiEventSource?.close(); + if (!eventsEnabled) return; + retryCount++; + const delay = Math.min(initialDelay * Math.pow(2, retryCount - 1), 5000); + setConnectionState("disconnected"); + if (retryTimer) clearTimeout(retryTimer); + retryTimer = setTimeout(connect, delay); + };
206-219: Fix stale connectionStatus in context value (missing dep).Consumers won’t re-render on status changes.
- [models, listModels, unloadAllModels, loadModel, enableAPIEvents, proxyLogs, upstreamLogs, metrics] + [models, listModels, unloadAllModels, loadModel, enableAPIEvents, proxyLogs, upstreamLogs, metrics, connectionStatus]
🧹 Nitpick comments (4)
ui/src/contexts/APIProvider.tsx (4)
61-61: Remove commented-out ref.Dead code adds noise.
- //const apiEventSource = useRef<EventSource | null>(null);
99-140: Minor: avoid shadowing ‘models’; tighten sort comparator.Renaming clarifies intent; comparator is cleaner and stable.
- case "modelStatus": - { - const models = JSON.parse(message.data) as Model[]; - - // sort models by name and id - models.sort((a, b) => { - return (a.name + a.id).localeCompare(b.name + b.id); - }); - - setModels(models); - } + case "modelStatus": { + const incomingModels = JSON.parse(message.data) as Model[]; + // sort by name, then id + incomingModels.sort((a, b) => a.name.localeCompare(b.name) || a.id.localeCompare(b.id)); + setModels(incomingModels); + } break;
131-134: Optional: cap metrics to prevent unbounded growth.Helps memory in long-running sessions.
- setMetrics((prevMetrics) => { - return [...newMetrics, ...prevMetrics]; - }); + setMetrics((prevMetrics) => { + const merged = [...newMetrics, ...prevMetrics]; + const MAX = 5000; + return merged.length > MAX ? merged.slice(0, MAX) : merged; + });
54-149: Optional: rely on native EventSource auto-retry instead of manual loop.If the server sends proper retry: directives, you can drop the manual close/reconnect to simplify. Not required if you prefer explicit control.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui/src/contexts/APIProvider.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ui/src/contexts/APIProvider.tsx (3)
ui/src/lib/types.ts (1)
ConnectionState(1-1)ui/src/pages/LogViewer.tsx (1)
useAPI(15-37)ui/src/components/ConnectionStatus.tsx (1)
useAPI(4-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-tests
🔇 Additional comments (2)
ui/src/contexts/APIProvider.tsx (2)
1-1: LGTM: removed unnecessary useRef import.
54-149: Single APIProvider usage confirmed; no conflicting EventSource instances found.
Ripgrep shows APIProvider is only mounted once inui/src/main.tsxand the onlynew EventSourcecall resides withinAPIProvideritself.
| let apiEventSource: EventSource | null = null; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent orphan SSE on HMR and reconnection after disable.
Add a retry timer + enabled flag, and close SSE on Vite HMR dispose. Without these, HMR can leave a dangling EventSource and scheduled retries can reopen after unmount/disable.
Apply:
let apiEventSource: EventSource | null = null;
+
+let retryTimer: ReturnType<typeof setTimeout> | null = null;
+let eventsEnabled = false;
+
+if (import.meta.hot) {
+ import.meta.hot.dispose(() => {
+ apiEventSource?.close();
+ apiEventSource = null;
+ if (retryTimer) {
+ clearTimeout(retryTimer);
+ retryTimer = null;
+ }
+ });
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let apiEventSource: EventSource | null = null; | |
| // ui/src/contexts/APIProvider.tsx | |
| let apiEventSource: EventSource | null = null; | |
| let retryTimer: ReturnType<typeof setTimeout> | null = null; | |
| let eventsEnabled = false; | |
| if (import.meta.hot) { | |
| import.meta.hot.dispose(() => { | |
| apiEventSource?.close(); | |
| apiEventSource = null; | |
| if (retryTimer) { | |
| clearTimeout(retryTimer); | |
| retryTimer = null; | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In ui/src/contexts/APIProvider.tsx around lines 54-55, the EventSource is
created as a single module-scoped variable which can be left dangling across HMR
and can be reopened by scheduled retries after the provider is disabled; add an
"enabled" flag and a retry timer handle (module-scoped), ensure any scheduled
retry is cleared when the provider is disabled or unmounted, set enabled=false
before closing the EventSource and null it out, and on Vite HMR disposal (if
import.meta.hot) close the EventSource, clear the retry timer, and set
enabled=false to prevent reconnection after HMR or disable.
| apiEventSource = new EventSource("/api/events"); | ||
|
|
||
| setConnectionState("connecting"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard connect and clear any previous retry timer.
Prevents reconnects when disabled and avoids timer accumulation.
- const connect = () => {
- apiEventSource?.close();
- apiEventSource = new EventSource("/api/events");
+ const connect = () => {
+ if (!eventsEnabled) return;
+ if (retryTimer) {
+ clearTimeout(retryTimer);
+ retryTimer = null;
+ }
+ apiEventSource?.close();
+ apiEventSource = new EventSource("/api/events");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| apiEventSource?.close(); | |
| apiEventSource = new EventSource("/api/events"); | |
| const connect = () => { | |
| if (!eventsEnabled) return; | |
| if (retryTimer) { | |
| clearTimeout(retryTimer); | |
| retryTimer = null; | |
| } | |
| apiEventSource?.close(); | |
| apiEventSource = new EventSource("/api/events"); | |
| }; |
🤖 Prompt for AI Agents
In ui/src/contexts/APIProvider.tsx around lines 84-86, the code unconditionally
closes any existing EventSource and immediately creates a new one which can
cause unwanted reconnects when the API is disabled and can accumulate retry
timers; guard the creation with the provider's "enabled" (or similar) flag so
you only instantiate EventSource when enabled, and before creating a new
EventSource clear any existing reconnect/retry timer
(clearTimeout/clearInterval) and nullify that timer variable so timers don't
stack; also ensure you remove existing event listeners and set apiEventSource =
null after closing if not reconnecting.
| // clear everything out on connect to keep things in sync | ||
| setProxyLogs(""); | ||
| setUpstreamLogs(""); | ||
| setMetrics([]); // clear metrics on reconnect | ||
| setModels([]); // clear models on reconnect | ||
| apiEventSource.current = eventSource; | ||
| retryCount = 0; | ||
| setConnectionState("connected"); | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ignore late onopen after disable.
Avoids updating state and keeping an open SSE when already disabled.
- apiEventSource.onopen = () => {
+ apiEventSource.onopen = () => {
+ if (!eventsEnabled) {
+ apiEventSource?.close();
+ return;
+ }
// clear everything out on connect to keep things in sync
setProxyLogs("");
setUpstreamLogs("");
setMetrics([]); // clear metrics on reconnect
setModels([]); // clear models on reconnect
retryCount = 0;
setConnectionState("connected");
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| apiEventSource.onopen = () => { | |
| // clear everything out on connect to keep things in sync | |
| setProxyLogs(""); | |
| setUpstreamLogs(""); | |
| setMetrics([]); // clear metrics on reconnect | |
| setModels([]); // clear models on reconnect | |
| apiEventSource.current = eventSource; | |
| retryCount = 0; | |
| setConnectionState("connected"); | |
| }; | |
| apiEventSource.onopen = () => { | |
| if (!eventsEnabled) { | |
| apiEventSource?.close(); | |
| return; | |
| } | |
| // clear everything out on connect to keep things in sync | |
| setProxyLogs(""); | |
| setUpstreamLogs(""); | |
| setMetrics([]); // clear metrics on reconnect | |
| setModels([]); // clear models on reconnect | |
| retryCount = 0; | |
| setConnectionState("connected"); | |
| }; |
🤖 Prompt for AI Agents
In ui/src/contexts/APIProvider.tsx around lines 89-97, the apiEventSource.onopen
handler can run after the connection has been disabled; update the handler to
first check whether the connection is currently disabled (e.g., connectionState
=== "disabled" or a local `isDisabled` flag) or the active event source doesn't
match the current ref, and if so call apiEventSource.close() and return early;
otherwise proceed to clear logs, reset retryCount, and
setConnectionState("connected") as before. Ensure the check prevents any state
updates when disabled and closes the stale SSE to avoid keeping it open.
…nnections (mostlygeek#290) - move SSE (EventSource) connection to module level - manage EventSource as a singleton, closing open connection before reopening a new one
* Add a config editor page * Support llama.cpp's cache_n in timings info (mostlygeek#287) Capture prompt cache metrics and surface them on Activities page in UI * Fix mostlygeek#288 Vite hot module reloading creating multiple SSE connections (mostlygeek#290) - move SSE (EventSource) connection to module level - manage EventSource as a singleton, closing open connection before reopening a new one * Add model name copy button to Models UI --------- Co-authored-by: Benson Wong <mostlygeek@gmail.com>
Summary by CodeRabbit
Bug Fixes
Performance
Refactor