fix: chat-feature#422
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds resilient SSE reconnection logic to chat streams in ChatConversation.vue and ChatList.vue: introduces connectionAttempts, maxAttempts, reconnectDelay, and reconnectTimer; resets state on open; on error closes the EventSource and retries with capped exponential backoff (1.5x, max 10s) up to maxAttempts; surfaces an error and stops loading when attempts are exhausted; cleans up timers on unmount. Existing message handling is preserved. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as ChatConversation / ChatList
participant S as Chat SSE Server
U->>C: Open chat / view list
C->>S: Subscribe (EventSource)
S-->>C: onopen
Note right of C #D6F5D6: connectionAttempts = 0\nreconnectDelay = 2000ms
loop Stream messages
S-->>C: message
C-->>C: Handle message (unchanged)
end
alt Error during stream
S--xC: onerror
C-->>C: Close EventSource\nclear timers
alt connectionAttempts < maxAttempts
C-->>C: connectionAttempts += 1\ndelay = min(reconnectDelay,10000)\nreconnectDelay = min(delay*1.5,10000)
Note right of C #FFF3D6: Schedule reconnect after delay
C->>S: Re-subscribe after delay
else Reached maxAttempts
C-->>U: "Connection to chat server failed."
C-->>C: set isLoading = false (if applicable)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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: 1
🧹 Nitpick comments (2)
components/chat/ChatConversation.vue (2)
22-24: Backoff controls look good; add jitter to avoid sync reconnects.To reduce thundering‑herd reconnects across clients, add small random jitter to the delay calculation (see diff in the onerror handler comment).
128-131: Clear any pending reconnect timer on successful connect.Prevents a late timer from tearing down a healthy connection and resubscribing.
Apply this diff:
eventSource.onopen = () => { console.log('SSE Connected') + if (reconnectTimer) { + clearTimeout(reconnectTimer) + reconnectTimer = null + } connectionAttempts.value = 0 reconnectDelay.value = 2000 error.value = null }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/chat/ChatConversation.vue(2 hunks)
⏰ 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: test
| eventSource?.close() | ||
| if (connectionAttempts.value < maxAttempts) { | ||
| connectionAttempts.value++ | ||
| const delay = Math.min(reconnectDelay.value, 10000) | ||
| reconnectDelay.value = Math.min(delay * 1.5, 10000) | ||
|
|
||
| console.log(`Reconnecting in ${delay} ms`) | ||
| setTimeout(() => { | ||
| subscribeToConversation(props.conversationId) | ||
| }, delay) | ||
| } else { | ||
| error.value = new Error('Connection to chat server failed.') | ||
| isLoading.value = false | ||
| } |
There was a problem hiding this comment.
Guard against leaks/duplicates: store and clear reconnect timer; add jitter; null out closed source.
Without tracking the timeout, a pending reconnect can fire after unmount or after a manual resubscribe, creating a stray EventSource. Also add jitter and null the closed instance.
Apply this diff:
console.error('SSE CONNECTION ERROR:', err)
- eventSource?.close()
+ eventSource?.close()
+ eventSource = null
if (connectionAttempts.value < maxAttempts) {
connectionAttempts.value++
- const delay = Math.min(reconnectDelay.value, 10000)
- reconnectDelay.value = Math.min(delay * 1.5, 10000)
-
- console.log(`Reconnecting in ${delay} ms`)
- setTimeout(() => {
- subscribeToConversation(props.conversationId)
- }, delay)
+ const base = Math.min(reconnectDelay.value, 10000)
+ const delay = Math.round(base * (0.85 + Math.random() * 0.30)) // jitter
+ reconnectDelay.value = Math.min(Math.round(base * 1.5), 10000)
+ console.log(`Reconnecting in ${delay} ms (attempt ${connectionAttempts.value}/${maxAttempts})`)
+ reconnectTimer = setTimeout(() => {
+ subscribeToConversation(props.conversationId)
+ }, delay)
} else {
error.value = new Error('Connection to chat server failed.')
isLoading.value = false
}Add these supporting changes outside the ranges above:
// near other timers
let reconnectTimer: ReturnType<typeof setTimeout> | null = null
// in onUnmounted()
if (reconnectTimer) {
clearTimeout(reconnectTimer)
reconnectTimer = null
}
// in the watcher before subscribeToConversation(newId)
if (reconnectTimer) {
clearTimeout(reconnectTimer)
reconnectTimer = null
}
connectionAttempts.value = 0
reconnectDelay.value = 2000🤖 Prompt for AI Agents
In components/chat/ChatConversation.vue around lines 136 to 149, the reconnect
logic creates timeouts and closes the EventSource but doesn't store/clear the
reconnect timer or null out the closed EventSource, which can lead to duplicate
connections or leaks; add a module-scoped variable reconnectTimer:
ReturnType<typeof setTimeout> | null = null, replace setTimeout calls with
reconnectTimer = setTimeout(...), and when closing the EventSource set
eventSource = null; clear and null reconnectTimer before creating a new
subscribe (in the watcher) and in onUnmounted() to prevent stray timers;
additionally add small random jitter to the computed delay (e.g., multiply delay
by 1 + random*0.1) and reset connectionAttempts.value = 0 and
reconnectDelay.value = 2000 when manually resubscribing.
refactor:Remove log statements
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
components/chat/ChatConversation.vue (1)
150-166: Reconnect timer not stored; can leak/duplicate after navigation. Also null the closed ES and add jitter.Track the timeout in reconnectTimer, set eventSource = null after close, and jitter the delay.
Apply this diff:
- if (reconnectTimer) { - clearTimeout(reconnectTimer) - reconnectTimer = null - } - eventSource?.close() - if (connectionAttempts.value < maxAttempts) { - connectionAttempts.value++ - const delay = Math.min(reconnectDelay.value, 10000) - reconnectDelay.value = Math.min(delay * 1.5, 10000) - setTimeout(() => { - subscribeToConversation(props.conversationId) - }, delay) - } else { + if (reconnectTimer) { + clearTimeout(reconnectTimer) + reconnectTimer = null + } + eventSource?.close() + eventSource = null + if (connectionAttempts.value < maxAttempts) { + connectionAttempts.value++ + const base = Math.min(reconnectDelay.value, 10000) + const delay = Math.round(base * (0.85 + Math.random() * 0.30)) // jitter + reconnectDelay.value = Math.min(Math.round(base * 1.5), 10000) + reconnectTimer = setTimeout(() => { + subscribeToConversation(props.conversationId) + }, delay) + } else { error.value = new Error('Connection to chat server failed.') isLoading.value = false }
🧹 Nitpick comments (3)
components/chat/ChatList.vue (1)
28-39: Also clear any pending reconnect timer and reset backoff when the user id changes.Prevents a stray timer from closing a fresh connection after an account switch.
Outside this range, add before closing the old ES:
if (reconnectTimer) { clearTimeout(reconnectTimer) reconnectTimer = null } connectionAttempts.value = 0 reconnectDelay.value = 2000components/chat/ChatConversation.vue (2)
139-144: Use EventSource.OPEN and null the instance before resubscribing.Improves clarity and avoids using magic numbers.
- reconnectTimer = setTimeout(() => { - if (eventSource && eventSource.readyState === 1) { - eventSource.close() - subscribeToConversation(props.conversationId) - } - }, 8000) + reconnectTimer = setTimeout(() => { + if (eventSource && eventSource.readyState === EventSource.OPEN) { + eventSource.close() + eventSource = null + subscribeToConversation(props.conversationId) + } + }, Math.round(55_000 * (0.85 + Math.random() * 0.30)))
69-80: Clear pending reconnect timer and reset backoff on conversation switch.Prevents a late timer from closing the new connection.
Outside this range, add at the top of the watcher:
if (reconnectTimer) { clearTimeout(reconnectTimer) reconnectTimer = null } connectionAttempts.value = 0 reconnectDelay.value = 2000
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/chat/ChatConversation.vue(3 hunks)components/chat/ChatList.vue(3 hunks)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: ci
🔇 Additional comments (4)
components/chat/ChatList.vue (2)
7-11: LGTM on reconnection state plumbing.The reactive counters and timer handle look good.
90-93: LGTM on timer cleanup.Clearing and nulling the timer on unmount prevents leaks.
components/chat/ChatConversation.vue (2)
22-25: LGTM on reconnection state additions.These provide the right primitives for backoff and attempt caps.
Also applies to: 28-28
53-67: LGTM on cleanup.Closing and nulling ES and timers on unmount is correct.
| eventSource.onopen = () => { | ||
| connectionAttempts.value = 0 | ||
| reconnectDelay.value = 2000 | ||
| error.value = null | ||
|
|
||
| if (reconnectTimer) clearTimeout(reconnectTimer) | ||
|
|
||
| reconnectTimer = setTimeout(() => { | ||
| if (eventSource && eventSource.readyState === 1) { | ||
| eventSource.close() | ||
| subscribeToConversation(props.conversationId) | ||
| } | ||
| }, 8000) | ||
| } |
There was a problem hiding this comment.
Forced reconnect every 8s is too frequent; prefer ~55s + jitter or heartbeat server-side.
Same rationale as ChatList; keep connections stable and avoid load spikes.
🤖 Prompt for AI Agents
In components/chat/ChatConversation.vue around lines 132 to 145, the code forces
a reconnect after 8000ms which is too frequent and can create load spikes;
change the reconnect behavior to wait about 55 seconds plus some jitter instead
(e.g., 55000ms + random 0–5000ms) or switch to using a heartbeat/ping from the
server to decide when to reconnect; implement this by replacing the hardcoded
8000 timeout with a computed reconnect interval (base 55000 + jitter) and use
that for setTimeout, keeping the existing readyState check and subscribe logic
unchanged.
| inboxEs.onopen = () => { | ||
| connectionAttempts.value = 0 | ||
| reconnectDelay.value = 2000 | ||
|
|
||
| if (reconnectTimer) clearTimeout(reconnectTimer) | ||
| reconnectTimer = setTimeout(() => { | ||
| if (inboxEs && inboxEs.readyState === 1) { | ||
| inboxEs.close() | ||
| refresh() | ||
| inboxEs = new EventSource('/api/chat/stream') | ||
| } | ||
| }, 8000) | ||
| } |
There was a problem hiding this comment.
8s forced reconnect is too aggressive; raise to ~55s with jitter (or rely on server heartbeats).
8s will churn connections and load; target <60s to play nicely with proxies and Vercel timeouts, and add jitter to avoid sync storms. The diff above already applies this. If your server can emit heartbeats every ~20–25s, you can remove the forced reconnect entirely.
🤖 Prompt for AI Agents
In components/chat/ChatList.vue around lines 50–62, the forced 8s reconnect is
too aggressive; change the reconnect timer to target ~55s with jitter (e.g.
55_000 ms ± random jitter of a few seconds) instead of 8_000 ms to avoid
connection churn and proxy timeouts, implement jitter via Math.random() to
add/subtract a small random offset, and keep the existing readyState
check/close/refresh/recreate logic; alternatively, if the server emits
heartbeats every ~20–25s, remove the forced reconnect entirely and rely on
heartbeat-based reconnection.
| reconnectTimer = setTimeout(() => { | ||
| if (inboxEs && inboxEs.readyState === 1) { | ||
| inboxEs.close() | ||
| refresh() | ||
| inboxEs = new EventSource('/api/chat/stream') | ||
| } | ||
| }, 8000) |
There was a problem hiding this comment.
Blocker: Recreated EventSource loses all handlers; also add jitter, null the closed instance, and use EventSource.OPEN.
Reopening the SSE with new EventSource drops onmessage/onopen/onerror, so the list will stop updating after the first forced reconnect or error. Also, prefer jittered backoff, set inboxEs = null after close, and use the OPEN constant.
Apply this diff:
- reconnectTimer = setTimeout(() => {
- if (inboxEs && inboxEs.readyState === 1) {
- inboxEs.close()
- refresh()
- inboxEs = new EventSource('/api/chat/stream')
- }
- }, 8000)
+ reconnectTimer = setTimeout(() => {
+ if (inboxEs && inboxEs.readyState === EventSource.OPEN) {
+ inboxEs?.close()
+ inboxEs = createInboxEventSource()
+ }
+ }, Math.round(55_000 * (0.85 + Math.random() * 0.30)))- if (reconnectTimer) {
- clearTimeout(reconnectTimer)
- reconnectTimer = null
- }
- inboxEs?.close()
- if (connectionAttempts.value < maxAttempts) {
- connectionAttempts.value++
- const delay = Math.min(reconnectDelay.value, 10000)
- reconnectDelay.value = Math.min(delay * 1.5, 10000)
- setTimeout(() => {
- refresh()
- inboxEs = new EventSource('/api/chat/stream')
- }, delay)
- }
+ if (reconnectTimer) {
+ clearTimeout(reconnectTimer)
+ reconnectTimer = null
+ }
+ inboxEs?.close()
+ inboxEs = null
+ if (connectionAttempts.value < maxAttempts) {
+ connectionAttempts.value++
+ const base = Math.min(reconnectDelay.value, 10000)
+ const delay = Math.round(base * (0.85 + Math.random() * 0.30)) // jitter
+ reconnectDelay.value = Math.min(Math.round(base * 1.5), 10000)
+ reconnectTimer = setTimeout(() => {
+ refresh()
+ inboxEs = createInboxEventSource()
+ }, delay)
+ }Add this helper outside the ranges above to ensure every new connection has handlers:
function createInboxEventSource(): EventSource {
const es = new EventSource('/api/chat/stream')
es.onmessage = (event) => {
try {
const data = JSON.parse(event.data)
if (data?.type === 'conversation.updated') {
refresh()
}
} catch (error) {
console.warn('Invalid SSE Payload', error)
}
}
es.onopen = () => {
connectionAttempts.value = 0
reconnectDelay.value = 2000
if (reconnectTimer) clearTimeout(reconnectTimer)
reconnectTimer = setTimeout(() => {
if (inboxEs && inboxEs.readyState === EventSource.OPEN) {
inboxEs?.close()
inboxEs = createInboxEventSource()
}
}, Math.round(55_000 * (0.85 + Math.random() * 0.30)))
}
es.onerror = (error) => {
console.error('Inbox SSE connection error:', error)
if (reconnectTimer) {
clearTimeout(reconnectTimer)
reconnectTimer = null
}
es.close()
inboxEs = null
if (connectionAttempts.value < maxAttempts) {
connectionAttempts.value++
const base = Math.min(reconnectDelay.value, 10000)
const delay = Math.round(base * (0.85 + Math.random() * 0.30))
reconnectDelay.value = Math.min(Math.round(base * 1.5), 10000)
reconnectTimer = setTimeout(() => {
refresh()
inboxEs = createInboxEventSource()
}, delay)
}
}
return es
}Also applies to: 66-79
🤖 Prompt for AI Agents
In components/chat/ChatList.vue around lines 55-61 (and also apply similar
changes for 66-79), the code recreates an EventSource directly which loses its
onmessage/onopen/onerror handlers; create a helper function
createInboxEventSource() (outside these ranges) that constructs a new
EventSource('/api/chat/stream'), assigns onmessage to parse event.data and call
refresh on conversation.updated, onopen to reset connectionAttempts and
reconnectDelay, clear any reconnectTimer and set a jittered reconnectTimer using
EventSource.OPEN to close & recreate when still open, and onerror to log the
error, clear and null reconnectTimer, close the source and set inboxEs = null,
then implement jittered backoff logic (increment connectionAttempts, compute
base/delay with 0.85–1.15 random factor, cap delays as in the diff) and schedule
reconnects that call refresh and set inboxEs = createInboxEventSource(); replace
direct new EventSource usages with inboxEs = createInboxEventSource(), ensure
inboxEs is set to null after close, and use EventSource.OPEN constant wherever
checking readyState.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
components/chat/ChatList.vue (2)
49-56: Force-reconnect: 8s is too aggressive; use ~55s with jitter and EventSource.OPEN.
Prevents churn/proxy issues and avoids magic number 1.Apply this diff:
- if (reconnectTimer) clearTimeout(reconnectTimer) - reconnectTimer = setTimeout(() => { - if (inboxEs && inboxEs.readyState === 1) { - inboxEs.close() - refresh() - setupInboxConnection() - } - }, 8000) + if (reconnectTimer) clearTimeout(reconnectTimer) + reconnectTimer = setTimeout(() => { + if (inboxEs && inboxEs.readyState === EventSource.OPEN) { + inboxEs.close() + inboxEs = null + refresh() + setupInboxConnection() + } + }, Math.round(55_000 * (0.85 + Math.random() * 0.30)))
60-75: Backoff timer isn’t tracked (leak/race); also null closed ES and add jitter.
Untracked setTimeout can fire after a successful reconnect and spawn a second connection.Apply this diff:
if (reconnectTimer) { clearTimeout(reconnectTimer) reconnectTimer = null } - inboxEs?.close() + inboxEs?.close() + inboxEs = null if (connectionAttempts.value < maxAttempts) { connectionAttempts.value++ - const delay = Math.min(reconnectDelay.value, 10000) - reconnectDelay.value = Math.min(delay * 1.5, 10000) - setTimeout(() => { - refresh() - setupInboxConnection() - }, delay) + const base = Math.min(reconnectDelay.value, 10_000) + const delay = Math.round(base * (0.85 + Math.random() * 0.30)) // jitter + reconnectDelay.value = Math.min(Math.round(base * 1.5), 10_000) + reconnectTimer = setTimeout(() => { + // another reconnect may have succeeded meanwhile + if (inboxEs) return + refresh() + setupInboxConnection() + }, delay) }
🧹 Nitpick comments (1)
components/chat/ChatList.vue (1)
28-32: Optional: client-guard inside setupInboxConnection.
Defensive no-op if ever called from SSR context.Apply this diff:
function setupInboxConnection() { - if (!currentUser.value?.id) return + if (!import.meta.client || !currentUser.value?.id) return
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/chat/ChatList.vue(3 hunks)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: ci
🔇 Additional comments (5)
components/chat/ChatList.vue (5)
7-11: LGTM: reconnection state scaffolding is clear and contained.
No issues; sane defaults.
33-42: Message handler looks solid.
Parsing guarded; refresh scoped to relevant event.
44-47: Good: reset attempts/backoff on open.
This pairs well with the backoff logic.
88-88: LGTM: centralized reconnection via setupInboxConnection().
Avoids handler loss on recreate.
99-102: Cleanup note: will only clear the tracked timer.
After adopting the backoff change above (assigning the timeout to reconnectTimer), this cleanup will also cancel pending retries.
2 participants
This PR tries to resolve SSE Connection errors caused on Vercel deployment environment for the chat feature
Summary by CodeRabbit