fix: bug bashing ChannelList + Channel (#2474, #2441, #2393)#3227
Open
oliverlaz wants to merge 8 commits into
Open
fix: bug bashing ChannelList + Channel (#2474, #2441, #2393)#3227oliverlaz wants to merge 8 commits into
oliverlaz wants to merge 8 commits into
Conversation
useIsChannelMuted called channel.muteStatus() during render, which throws "hasn't been initialized yet" when the channel has not been watched. The ChannelList can briefly hold such a channel (e.g. a message.new for a channel that has not been watched yet), and the throw crashed the whole app. Guard the call behind channel.initialized and fall back to a not-muted status. Closes #2474
…ssage_new handleNotificationMessageNew called getChannel() (channel.watch()) before checking allowNewMessagesFromUnfilteredChannels, so a queryChannel was issued for every notification.message_new even when the result was discarded. When many such events arrive at once (e.g. a bulk campaign) this exhausted the rate limit. Move the guard ahead of the query so the flag actually suppresses the watch(). Closes #2441
Removing a channel from the list did not stop watching it, so the client kept receiving its events and a later message.new / notification.message_new re-added it to the list until a hard refresh. notification.removed_from_channel now calls channel.stopWatching(), and a new member.removed handler removes the channel and stops watching it when the current user is the removed member. Closes #2599
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a ChannelList early return for unfiltered new-message events, guards mute-status reads before channel initialization, and prevents Channel pagination and state reads after disconnect. Adds tests for the new ChannelList and Channel behaviors. ChangesChannelList message-new guard
Channel mute and disconnect guards
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Size Change: +598 B (+0.08%) Total Size: 798 kB 📦 View Changed
ℹ️ View Unchanged
|
oliverlaz
commented
Jun 29, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3227 +/- ##
==========================================
- Coverage 84.20% 84.20% -0.01%
==========================================
Files 485 485
Lines 14867 14872 +5
Branches 4704 4707 +3
==========================================
+ Hits 12519 12523 +4
- Misses 2348 2349 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/ChannelListItem/hooks/useIsChannelMuted.ts`:
- Around line 21-24: The local muted state in useIsChannelMuted is only
initialized once and then updated on notification.channel_mutes_updated, so it
can stay stale if the channel mounts before watch()/query() finishes. Add a
resync path in useIsChannelMuted that re-reads getMuteStatus(channel) when the
channel initializes or becomes available, and keep the existing event-driven
update intact. Use the useEffect and getMuteStatus symbols in this hook to
locate the fix, and add a regression test for the mount-before-watch
initialization case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38a4028a-0f91-4f43-98cc-e2cc77805bbe
📒 Files selected for processing (4)
src/components/ChannelList/__tests__/ChannelList.test.tsxsrc/components/ChannelList/hooks/useChannelListShape.tssrc/components/ChannelListItem/hooks/__tests__/useIsChannelMuted.test.tsxsrc/components/ChannelListItem/hooks/useIsChannelMuted.ts
channel.lastRead() runs during render (useCreateChannelStateContext) and throws "You can't use a channel after client.disconnect() was called" once the shared client is disconnected - crashing the render, with no error boundary to catch it. Guard the call with channel.disconnected. Also skip loadMore's pagination query while the client is disconnected, mirroring the existing markRead guard. Closes #2393
oliverlaz
changed the title
oliverlaz
changed the title
Browser verification showed the stopWatching-based fix does not make removal durable: channel.stopWatching() does not evict the channel from client.activeChannels, so a reconnect (recoverState re-watches every active channel) or a queued/in-flight message.new re-adds the removed channel to the list - the exact reported symptom. The correct fix belongs in stream-chat core: evict activeChannels[cid] on notification.removed_from_channel / member.removed, mirroring the existing channel.deleted handling. Dropping #2599 from this PR until that core change lands. #2474, #2441 and #2393 are unaffected.
oliverlaz
changed the title
oliverlaz
changed the title
arnautov-anton
approved these changes
Jun 30, 2026
MartinCupela
approved these changes
Jun 30, 2026
The notification.channel_mutes_updated effect depended on [muted], so it re-subscribed on its own output and could capture a stale channel when the channel prop changed without the muted value changing. Depend on [channel, client] instead and clean up via the unsubscribe handle returned by client.on(), dropping the exhaustive-deps override.
An initialized channel whose client has disconnected still routes channel.muteStatus() through channel.getClient(), which throws "You can't use a channel after client.disconnect() was called". Guard channel.disconnected alongside channel.initialized in getMuteStatus so rendering a ChannelListItem with a disconnected channel no longer crashes the app (same failure class as the #2393 Channel-state guard).
…nnels (#2599) Regression test for #2599: removing the current user from a channel must evict it from client.activeChannels, otherwise recoverState() re-watches it on the next reconnect (recoverStateOnReconnect defaults to true) and the list resurrects it. The eviction itself lives in stream-chat core (GetStream/stream-chat-js#1788). This test is red until that fix ships in a stream-chat release and the dependency here is bumped to it; the bump + green run will accompany closing #2599.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Goal
Fixes three Tier‑1 issues from the bug‑bash triage. Each is an independent fix kept as its own self‑contained commit (with tests):
message.newfor a not‑yet‑watched channel inserts an uninitialized channel into the list; rendering it calledchannel.muteStatus(), which throws"hasn't been initialized yet"and crashed the whole app.ChannelListissued aqueryChannel(channel.watch()) for everynotification.message_new, even when the result was discarded, exhausting the rate limit during bulk channel activity.channel.lastRead()(called during render inuseCreateChannelStateContext) throws"You can't use a channel after client.disconnect() was called", crashing the render with no error boundary to catch it.🛠 Implementation details
#2474 —
fix(ChannelListItem): guard muteStatus against an uninitialized/disconnected channeluseIsChannelMutedreads the mute status through a helper that callschannel.muteStatus()only when the channel is initialized and not disconnected, falling back to a not‑muted status otherwise.muteStatus()throws on an uninitialized channel (#2474) and also after the client disconnects — viachannel.getClient()— even though the channel stays initialized (the bug: ChannelInner can call methods on a disconnected channel, causing uncaught errors #2393 failure class on the channel‑list path).[channel, client]and cleans up via theunsubscribehandle returned byclient.on(). It previously depended on[muted], so it re‑subscribed on its own output and could capture a stale channel.#2441 —
fix(ChannelList): do not query unfiltered channels on notification.message_newallowNewMessagesFromUnfilteredChannelsguard before thegetChannel()call inhandleNotificationMessageNew, so the flag actually suppresses the per‑eventwatch()instead of querying first and throwing the result away. Behaviour with the flag enabled is unchanged.#2393 —
fix(Channel): guard channel methods against a disconnected clientchannel.lastRead()inuseCreateChannelStateContextwith!channel.disconnectedso a disconnect while the channel is mounted does not crash the render.loadMorenow early‑returns whenchannel.disconnected, mirroring the existingmarkReadguard, to avoid a doomed pagination query.try/catchbackstops on the async paths remain); this removes the uncaught render‑time crash, which is the part with no safety net.Tests — added test‑first (each test was confirmed to fail before the fix):
useIsChannelMuteddoes not throw on an uninitialized channel, nor on an initialized‑but‑disconnected channel.notification.message_newdoes not callwatch()whenallowNewMessagesFromUnfilteredChannels={false}.loadMoredoes not query when the client is disconnected.notification.removed_from_channelevicts the channel fromclient.activeChannels(so reconnect cannot re‑watch it). This test is red until thestream-chatbump described above — it was verified red→green during development by portalingstream-chatto the stream-chat-js#1788 branch.Verification:
yarn test(ChannelList, ChannelListItem, Channel, MessageList, Thread),yarn types, and ESLint (--max-warnings 0) clean for #2474, #2441 and #2393, which were additionally confirmed in a real browser against the example app. The single #2599 test is expected to fail CI until thestream-chatdependency is bumped to the release containing GetStream/stream-chat-js#1788.🎨 UI Changes
None — these are crash / correctness / rate‑limit fixes in
ChannelListandChannelevent handling, with no visual changes.Summary by CodeRabbit
Bug Fixes
Tests