Conversation
Greptile SummaryThis PR upgrades the Web SDK to version 25.0.0, refactoring the Realtime subscription system from URL-parameter-based channel registration to a WebSocket message-based protocol (
Confidence Score: 3/5Not safe to merge — the Client.subscribe() unsubscribe path silently leaks server-side subscriptions under the new protocol. One confirmed P1 regression: the unsubscribe closure in Client.subscribe() does not send an unsubscribe message to the server when the socket is open, causing the server to continue routing events for removed subscriptions. This is a behavioral correctness issue on the primary realtime code path introduced by this PR's protocol change. src/client.ts — the unsubscribe closure returned by Client.subscribe() (lines 823-836) needs an explicit unsubscribe frame sent to the server. Important Files Changed
Reviews (5): Last reviewed commit: "chore: update Web SDK to 25.0.0" | Re-trigger Greptile |
Made-with: Cursor
Made-with: Cursor
| return () => { | ||
| this.realtime.subscriptions.delete(counter); | ||
| this.realtime.cleanUp(channelStrings, queryStrings); | ||
| this.realtime.subscriptions.delete(subscriptionId); | ||
| this.realtime.pendingSubscribes.delete(subscriptionId); | ||
| const stillUsed = new Set<string>(); | ||
| this.realtime.subscriptions.forEach(sub => { | ||
| sub.channels.forEach(channel => stillUsed.add(channel)); | ||
| }); | ||
| this.realtime.channels.forEach(channel => { | ||
| if (!stillUsed.has(channel)) { | ||
| this.realtime.channels.delete(channel); | ||
| } | ||
| }); | ||
| this.realtime.connect(); | ||
| } |
There was a problem hiding this comment.
Unsubscribe doesn't notify the server via the new protocol
The returned unsubscribe function deletes the subscription from local maps and calls connect(), but connect() only calls sendPendingSubscribes() when the socket is already OPEN — and pendingSubscribes was just cleared for this subscriptionId, so nothing is sent.
Under the old URL-parameter model, reconnecting with an updated channel list was enough to inform the server. Under the new message-based model the server must receive an explicit unsubscribe frame. Without it, the server continues routing events for the removed subscription for the lifetime of the connection; they are silently dropped client-side only after a reconnect removes the subscription from the re-subscription list.
The Realtime service class's unsubscribe() closure correctly calls sendUnsubscribeMessage([subscriptionId]) — the same call is missing here.
There was a problem hiding this comment.
its fine as the deprecated client.ts shouldn't do this
This PR contains updates to the SDK for version 25.0.0.
What's Changed