Skip to content

fix: prevent adding events to closed StreamController#978

Open
rwrz wants to merge 1 commit intolivekit:mainfrom
RealLifeGlobal:fix/prevent-event-after-stream-closed
Open

fix: prevent adding events to closed StreamController#978
rwrz wants to merge 1 commit intolivekit:mainfrom
RealLifeGlobal:fix/prevent-event-after-stream-closed

Conversation

@rwrz
Copy link

@rwrz rwrz commented Feb 4, 2026

Add isClosed checks to DataStreamController's write() and error() methods to prevent "Bad state: Cannot add event after closing" exception.

This error occurs when a participant disconnects and validateParticipantHasNoActiveDataStreams() tries to send errors to stream controllers that have already been closed.

Summary by CodeRabbit

  • Bug Fixes
    • Improved stability by preventing errors when operations are attempted on closed data streams. The system now gracefully handles edge cases where write or error operations occur after stream closure, ensuring more reliable behavior without throwing exceptions.

Add isClosed checks to DataStreamController's write() and error() methods
to prevent "Bad state: Cannot add event after closing" exception.

This error occurs when a participant disconnects and
validateParticipantHasNoActiveDataStreams() tries to send errors to
stream controllers that have already been closed.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
Copy link

coderabbitai bot commented Feb 4, 2026

📝 Walkthrough

Walkthrough

The DataStreamController's write() and error() methods now include guards that check if the underlying StreamController is closed before attempting to add chunks or errors, preventing operations on closed controllers.

Changes

Cohort / File(s) Summary
Defensive Checks for Closed Stream Controller
lib/src/types/data_stream.dart
Added isClosed guards in write() and error() methods to prevent operations when the underlying StreamController is closed, replacing unconditional add/addError calls that could throw.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A guardian checks before it leaps,
No write shall wake what deeply sleeps,
The stream controller's door now sealed tight,
With gentle guards we've set things right! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding guards in DataStreamController to prevent operations on closed StreamControllers.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

🧹 Recent nitpick comments
lib/src/types/data_stream.dart (1)

164-174: The fix correctly prevents the exception, but consider adding observability for dropped writes/errors.

The isClosed guards effectively prevent the "Bad state" exception. However, silently discarding chunks and errors could make debugging harder in edge cases where writes are unexpectedly dropped.

Consider one of these optional improvements:

  1. Add debug-level logging when a write/error is ignored
  2. Return a bool to indicate success (though this would be a breaking change)
  3. Document the silent-drop behavior in doc comments
♻️ Optional: Add logging for dropped writes
+import 'package:logging/logging.dart';
+
+final _logger = Logger('DataStreamController');
+
 void write(T chunk) {
-   if (!streamController.isClosed) {
+   if (streamController.isClosed) {
+     _logger.fine('Ignoring write to closed stream: ${info.id}');
+     return;
+   }
-     streamController.add(chunk);
-   }
+   streamController.add(chunk);
 }

 void error(DataStreamError error) {
-   if (!streamController.isClosed) {
+   if (streamController.isClosed) {
+     _logger.fine('Ignoring error on closed stream: ${info.id}');
+     return;
+   }
-     streamController.addError(error);
-   }
+   streamController.addError(error);
 }
📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57c8d42 and 2eaf998.

📒 Files selected for processing (1)
  • lib/src/types/data_stream.dart
⏰ 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). (7)
  • GitHub Check: Build for Flutter Windows
  • GitHub Check: Build for Flutter Web
  • GitHub Check: Build for Flutter Linux
  • GitHub Check: Build for Flutter iOS
  • GitHub Check: Build for Flutter macOS
  • GitHub Check: Build for Flutter Web WASM
  • GitHub Check: Build for Flutter Android

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants