Skip to content

Commit fb1fead

Browse files
committed
fix(sse): bound workspace SSE connection lifetime
Teardown ran only from the request abort listener and the stream cancel callback, both of which fire only when the runtime reports a client disconnect. Nothing else bounded the connection, so a missed report left the pub/sub handler, the heartbeat timer, and the stream's undrained queue held for the life of the process. Add a jittered lifetime ceiling checked on the existing heartbeat tick, tighten reclaim for a vanished consumer via desiredSize, remove the abort listener on every teardown path, and run full teardown when a heartbeat enqueue fails. Log the close reason so opens minus closes is observable.
1 parent 7b761ba commit fb1fead

2 files changed

Lines changed: 180 additions & 25 deletions

File tree

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
5+
import { authMockFns, permissionsMock, permissionsMockFns } from '@sim/testing'
6+
import { NextRequest } from 'next/server'
7+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
8+
import {
9+
createWorkspaceSSE,
10+
HEARTBEAT_INTERVAL_MS,
11+
MAX_CONNECTION_JITTER_MS,
12+
MAX_CONNECTION_MS,
13+
MAX_UNDRAINED_CHUNKS,
14+
} from '@/lib/events/sse-endpoint'
15+
16+
vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
17+
18+
const PAST_CEILING_MS = MAX_CONNECTION_MS + MAX_CONNECTION_JITTER_MS + HEARTBEAT_INTERVAL_MS
19+
20+
/** Enough undrained heartbeats to trip the unread check, and no more. */
21+
const PAST_UNREAD_MS = (MAX_UNDRAINED_CHUNKS + 2) * HEARTBEAT_INTERVAL_MS
22+
23+
async function openConnection(signal: AbortSignal = new AbortController().signal) {
24+
const unsubscribe = vi.fn()
25+
const handler = createWorkspaceSSE({
26+
label: 'test',
27+
subscriptions: [{ subscribe: () => unsubscribe }],
28+
})
29+
const request = new NextRequest(new URL('https://sim.test/api/test/events?workspaceId=ws-1'), {
30+
signal,
31+
})
32+
const response = await handler(request)
33+
34+
return { body: response.body as ReadableStream<Uint8Array>, unsubscribe }
35+
}
36+
37+
/** Resolves once the stream closes. */
38+
async function drain(body: ReadableStream<Uint8Array>): Promise<void> {
39+
const reader = body.getReader()
40+
while (true) {
41+
const { done } = await reader.read()
42+
if (done) return
43+
}
44+
}
45+
46+
describe('createWorkspaceSSE', () => {
47+
beforeEach(() => {
48+
vi.clearAllMocks()
49+
vi.useFakeTimers()
50+
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-1' } })
51+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('admin')
52+
})
53+
54+
afterEach(() => {
55+
vi.useRealTimers()
56+
})
57+
58+
it('releases subscriptions and closes the stream when the connection reaches its ceiling', async () => {
59+
const { body, unsubscribe } = await openConnection()
60+
const drained = drain(body)
61+
62+
await vi.advanceTimersByTimeAsync(PAST_CEILING_MS)
63+
64+
await drained
65+
expect(unsubscribe).toHaveBeenCalledTimes(1)
66+
})
67+
68+
it('releases subscriptions when the consumer stops draining the stream', async () => {
69+
const { unsubscribe } = await openConnection()
70+
71+
await vi.advanceTimersByTimeAsync(PAST_UNREAD_MS)
72+
73+
expect(unsubscribe).toHaveBeenCalledTimes(1)
74+
})
75+
76+
it('keeps a drained connection alive past the unread threshold', async () => {
77+
const { body, unsubscribe } = await openConnection()
78+
void drain(body)
79+
80+
await vi.advanceTimersByTimeAsync(PAST_UNREAD_MS)
81+
82+
expect(unsubscribe).not.toHaveBeenCalled()
83+
})
84+
85+
it('releases subscriptions when the request aborts', async () => {
86+
const controller = new AbortController()
87+
const { unsubscribe } = await openConnection(controller.signal)
88+
89+
controller.abort()
90+
91+
expect(unsubscribe).toHaveBeenCalledTimes(1)
92+
})
93+
94+
it('releases subscriptions when the consumer cancels the stream', async () => {
95+
const { body, unsubscribe } = await openConnection()
96+
97+
await body.cancel()
98+
99+
expect(unsubscribe).toHaveBeenCalledTimes(1)
100+
})
101+
102+
it('releases subscriptions once when abort and the ceiling both elapse', async () => {
103+
const controller = new AbortController()
104+
const { unsubscribe } = await openConnection(controller.signal)
105+
106+
controller.abort()
107+
await vi.advanceTimersByTimeAsync(PAST_CEILING_MS)
108+
109+
expect(unsubscribe).toHaveBeenCalledTimes(1)
110+
})
111+
})

apps/sim/lib/events/sse-endpoint.ts

Lines changed: 69 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
import { createLogger } from '@sim/logger'
9+
import { randomFloat } from '@sim/utils/random'
910
import type { NextRequest } from 'next/server'
1011
import { getSession } from '@/lib/auth'
1112
import { SSE_HEADERS } from '@/lib/core/utils/sse'
@@ -23,7 +24,34 @@ interface WorkspaceSSEConfig {
2324
subscriptions: SSESubscription[]
2425
}
2526

26-
const HEARTBEAT_INTERVAL_MS = 30_000
27+
const encoder = new TextEncoder()
28+
29+
export const HEARTBEAT_INTERVAL_MS = 30_000
30+
31+
/**
32+
* Hard ceiling on one connection's lifetime.
33+
*
34+
* `request.signal` abort and stream `cancel()` are the primary teardown paths,
35+
* but both fire only when the runtime reports the client disconnect. This
36+
* ceiling releases the connection without depending on that report, so a
37+
* missed disconnect costs one connection rather than accumulating for the life
38+
* of the process. `EventSource` reconnects on its own, so delivery continues
39+
* across the boundary.
40+
*/
41+
export const MAX_CONNECTION_MS = 15 * 60 * 1000
42+
43+
/** Spreads reconnects so connections opened together do not expire together. */
44+
export const MAX_CONNECTION_JITTER_MS = 60_000
45+
46+
/**
47+
* Undrained chunk count that marks a connection unread, which shortens the
48+
* reclaim window for a vanished consumer that the ceiling above would
49+
* otherwise hold for its full duration. The default queuing strategy reports
50+
* `desiredSize` as `1 - queued`, so this trips only once the consumer has
51+
* pulled nothing for several minutes — well beyond the transient backpressure
52+
* of a slow but live client.
53+
*/
54+
export const MAX_UNDRAINED_CHUNKS = 16
2755

2856
export function createWorkspaceSSE(config: WorkspaceSSEConfig) {
2957
const logger = createLogger(`${config.label}-SSE`)
@@ -45,21 +73,30 @@ export function createWorkspaceSSE(config: WorkspaceSSEConfig) {
4573
return new Response('Access denied to workspace', { status: 403 })
4674
}
4775

48-
const encoder = new TextEncoder()
49-
const unsubscribers: Array<() => void> = []
76+
const teardowns: Array<() => void> = []
5077
let cleaned = false
5178

52-
const cleanup = () => {
79+
const cleanup = (reason: string) => {
5380
if (cleaned) return
5481
cleaned = true
55-
for (const unsub of unsubscribers) {
56-
unsub()
82+
for (const teardown of teardowns) {
83+
teardown()
5784
}
58-
logger.info(`SSE connection closed for workspace ${workspaceId}`)
85+
teardowns.length = 0
86+
logger.info(`SSE connection closed for workspace ${workspaceId}`, { reason })
5987
}
6088

6189
const stream = new ReadableStream({
6290
start(controller) {
91+
const close = (reason: string) => {
92+
cleanup(reason)
93+
try {
94+
controller.close()
95+
} catch {
96+
// Already closed
97+
}
98+
}
99+
63100
const send = (eventName: string, data: Record<string, unknown>) => {
64101
if (cleaned) return
65102
try {
@@ -72,40 +109,47 @@ export function createWorkspaceSSE(config: WorkspaceSSEConfig) {
72109
}
73110

74111
for (const subscription of config.subscriptions) {
75-
const unsub = subscription.subscribe(workspaceId, send)
76-
unsubscribers.push(unsub)
112+
teardowns.push(subscription.subscribe(workspaceId, send))
77113
}
78114

115+
const deadline = Date.now() + MAX_CONNECTION_MS + randomFloat() * MAX_CONNECTION_JITTER_MS
116+
79117
const heartbeat = setInterval(() => {
80118
if (cleaned) {
81119
clearInterval(heartbeat)
82120
return
83121
}
122+
if (Date.now() >= deadline) {
123+
close('expired')
124+
return
125+
}
126+
const desiredSize = controller.desiredSize
127+
if (desiredSize !== null && desiredSize <= -MAX_UNDRAINED_CHUNKS) {
128+
close('unread')
129+
return
130+
}
84131
try {
85132
controller.enqueue(encoder.encode(': heartbeat\n\n'))
86133
} catch {
87-
clearInterval(heartbeat)
134+
close('errored')
88135
}
89136
}, HEARTBEAT_INTERVAL_MS)
90-
unsubscribers.push(() => clearInterval(heartbeat))
91-
92-
request.signal.addEventListener(
93-
'abort',
94-
() => {
95-
cleanup()
96-
try {
97-
controller.close()
98-
} catch {
99-
// Already closed
100-
}
101-
},
102-
{ once: true }
103-
)
137+
teardowns.push(() => clearInterval(heartbeat))
138+
139+
// `once` only self-removes if abort fires; the expiry and unread paths
140+
// close the connection while the signal is still live, so the listener
141+
// needs its own removal or it retains this whole scope.
142+
const listenerScope = new AbortController()
143+
request.signal.addEventListener('abort', () => close('aborted'), {
144+
once: true,
145+
signal: listenerScope.signal,
146+
})
147+
teardowns.push(() => listenerScope.abort())
104148

105149
logger.info(`SSE connection opened for workspace ${workspaceId}`)
106150
},
107151
cancel() {
108-
cleanup()
152+
cleanup('cancelled')
109153
},
110154
})
111155

0 commit comments

Comments
 (0)