fix(core): sort channels with disposed state last instead of crashing - #2887
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesChannel sorting
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change makes disposed channels sort last instead of causing a crash, with a regression test covering the behavior. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
…listcontroller-local-sort-crashes-on-channels
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2887 +/- ##
=======================================
Coverage 73.96% 73.96%
=======================================
Files 435 435
Lines 28147 28149 +2
=======================================
+ Hits 20818 20821 +3
+ Misses 7329 7328 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Submit a pull request
Linear: FLU-692
CLA
Description of the pull request
StreamChannelListController'svaluesetter sorts the page locally whenever achannelStateSortis active (including when apredefinedFilterresolves aneffectiveSort), using a null assert on the channel state:Channel.dispose()nullsstate, so an in-flightdoInitialLoad/loadMorethat resumes after channels were disposed — a logout/disconnect racing an in-flight pagination, or a channel removed by a deleted/hidden event while a query is in flight — crashes withNull check operator used on a null value. Caught by the nightly e2e run (iOS 18.5,channel_list_test.dart), where the teardown'sclient.dispose()raced a scroll-triggeredloadMore.The sort key is now
it.state?.channelStatewith a nulls-last comparator, so disposed channels sort to the end instead of crashing. No further lifecycle guards are needed: the basePagedValueNotifieralready discards value writes after the controller itself is disposed.Testing: added a regression test (fails with a null-check
_TypeErroron the unfixed code) that mixes a state-less channel into a sorted page and asserts it lands last while live channels keep the sort order.Summary by CodeRabbit