fix(api): Don't block subscription events during WebSocket reconnect - #7282
fix(api): Don't block subscription events during WebSocket reconnect#7282VarshithaPamisetty wants to merge 7 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7282 +/- ##
=======================================
Coverage 40.98% 40.98%
=======================================
Files 121 121
Lines 8273 8273
Branches 3598 3598
=======================================
Hits 3391 3391
Misses 4882 4882 🚀 New features to boost your workflow:
|
| late WebSocketState _currentState; | ||
|
|
||
| /// The in-flight reconnection, if any. Ensures at most one runs at a time. | ||
| Future<void>? _reconnectOperation; |
There was a problem hiding this comment.
Don't we need to cancel or await this? E.g. in _close?
There was a problem hiding this comment.
Tested awaiting it in _close() and it hangs shutdown (breaks the shutdown-during-reconnect test), and a Future can't be cancelled. Instead, _isShuttingDown makes the reconnect a no-op once the bloc closes.
| _reconnectOperation ??= _performReconnect( | ||
| _currentState, | ||
| ).whenComplete(() => _reconnectOperation = null); |
There was a problem hiding this comment.
Should we handle errors that aren't Exception (https://api.dart.dev/dart-core/#exceptions)? _performReconnect catches Exception only (} on Exception catch (e, st) ).
There was a problem hiding this comment.
on Exception is intentional here. Per Effective Dart we shouldn't swallow Errors, and the one that actually surfaced in testing, add-after-close, is already prevented by the _isShuttingDown/_safeAdd guards. So leaving it as-is would be better.
3 participants
Description
The WebSocket reconnect ran on the serial event queue (
asyncExpand), so a slow reconnect blocked every event behind it: subscription data and keep-alives were received but never processed, freezing active subscriptions on a healthy socket. This moves reconnect off the queue so events keep flowing, and guards against shutdown mid-reconnect.Testing
analyze/formatclean (Dart 3.11 and 3.13).Fixes #7001