fix: handle exceptions when FlutterEngine is suspended#595
Merged
Conversation
WalkthroughWraps event-sink dispatch in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/Classes/AblyFlutterStreamHandler.m (1)
132-151:⚠️ Potential issue | 🔴 CriticalUnreachable code:
cancelListeninghas duplicate conditions making cleanup branches dead code.All three
if/else ifconditions on lines 132, 134, and 144 compare againstAblyPlatformMethod_onRealtimeConnectionStateChanged. This means:
- Line 134-143 (channel
off:) is unreachable- Line 144-151 (channel
unsubscribe:) is unreachableThese should check
AblyPlatformMethod_onRealtimeChannelStateChangedandAblyPlatformMethod_onRealtimeChannelMessagerespectively, mirroring thestartListeningmethod structure.This bug is explicitly called out in linked issue
#594as objective#3: "Fix cancelListening logic so each listener branch is reachable and cleaned up."🐛 Proposed fix
if([AblyPlatformMethod_onRealtimeConnectionStateChanged isEqual: eventName]) { [[_instanceStore realtimeFrom:handle].connection off: listener]; - } else if([AblyPlatformMethod_onRealtimeConnectionStateChanged isEqual: eventName]) { + } else if([AblyPlatformMethod_onRealtimeChannelStateChanged isEqual: eventName]) { // Note: this and all other assert statements in this onCancel method are // left as is as there is no way of propagating this error to flutter side NSAssert(eventMessage.message, @"event message is missing"); NSMutableDictionary<NSString *, NSObject *>* eventPayload = eventMessage.message; ARTRealtime *const realtime = [_instanceStore realtimeFrom:handle]; NSString *const channelName = (NSString*) eventPayload[TxTransportKeys_channelName]; ARTRealtimeChannel *const channel = [realtime.channels get:channelName]; [channel off: listener]; - } else if([AblyPlatformMethod_onRealtimeConnectionStateChanged isEqual: eventName]) { + } else if([AblyPlatformMethod_onRealtimeChannelMessage isEqual: eventName]) { NSAssert(eventMessage.message, @"event message is missing"); NSMutableDictionary<NSString *, NSObject *>* eventPayload = eventMessage.message; ARTRealtime *const realtime = [_instanceStore realtimeFrom:handle]; NSString *const channelName = (NSString*) eventPayload[TxTransportKeys_channelName]; ARTRealtimeChannel *const channel = [realtime.channels get:channelName]; [channel unsubscribe: listener]; } else if ([AblyPlatformMethod_onRealtimePresenceMessage isEqual: eventName]) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Classes/AblyFlutterStreamHandler.m` around lines 132 - 151, In cancelListening, the three conditional branches all check AblyPlatformMethod_onRealtimeConnectionStateChanged, making the channel cleanup branches unreachable; update the second condition to check AblyPlatformMethod_onRealtimeChannelStateChanged and the third to check AblyPlatformMethod_onRealtimeChannelMessage so the branches that call [channel off: listener] and [channel unsubscribe: listener] execute; keep the existing eventMessage handling, NSAssert calls, and use the same _instanceStore/realtime/channel retrieval logic to mirror startListening's structure and ensure each listener branch is properly cleaned up.
🧹 Nitpick comments (1)
.github/workflows/check.yaml (1)
55-61: Consider using a more stable simulator destination.The destination
name=iPhone 16may not be available on allmacos-latestrunner images, depending on the Xcode version installed. This could cause intermittent CI failures.Consider using a more generic approach:
♻️ Suggested alternatives
Option 1 - Use a device family that's consistently available:
xcodebuild test \ -workspace example/ios/Runner.xcworkspace \ -scheme ably_flutter-Tests \ - -destination 'platform=iOS Simulator,OS=latest,name=iPhone 16' \ + -destination 'platform=iOS Simulator,OS=latest,name=iPhone 15' \ CODE_SIGNING_ALLOWED=NOOption 2 - Use
any iOS Simulatorto let Xcode pick:xcodebuild test \ -workspace example/ios/Runner.xcworkspace \ -scheme ably_flutter-Tests \ - -destination 'platform=iOS Simulator,OS=latest,name=iPhone 16' \ + -destination 'generic/platform=iOS Simulator' \ CODE_SIGNING_ALLOWED=NO🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/check.yaml around lines 55 - 61, The CI step "Run iOS unit tests" uses xcodebuild test with a fixed destination name ('-destination "platform=iOS Simulator,OS=latest,name=iPhone 16"') which can be missing on some macos-latest images; change the -destination argument in that xcodebuild test invocation to a more stable selector such as a device-family or a generic simulator (e.g. replace the name=... selector with a consistently available device like device=iPhone 8 or use a generic selector like just 'platform=iOS Simulator,OS=latest' / 'platform=iOS Simulator,OS=latest,any' so Xcode can pick an available simulator), updating the command in the "Run iOS unit tests" step accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/Classes/AblyFlutterStreamHandler.m`:
- Around line 122-124: The catch block in onListenWithArguments: currently only
logs exceptions which can leave the Dart stream hanging; update the `@catch` in
AblyFlutterStreamHandler.m to notify Flutter by sending a FlutterError to the
event sink (e.g. call the instance's event sink with a FlutterError created via
+errorWithCode:message:details: including exception.reason) and then rethrow or
return an error so the failure is not silently swallowed; reference the
onListenWithArguments: method and the class AblyFlutterStreamHandler when making
this change.
---
Outside diff comments:
In `@ios/Classes/AblyFlutterStreamHandler.m`:
- Around line 132-151: In cancelListening, the three conditional branches all
check AblyPlatformMethod_onRealtimeConnectionStateChanged, making the channel
cleanup branches unreachable; update the second condition to check
AblyPlatformMethod_onRealtimeChannelStateChanged and the third to check
AblyPlatformMethod_onRealtimeChannelMessage so the branches that call [channel
off: listener] and [channel unsubscribe: listener] execute; keep the existing
eventMessage handling, NSAssert calls, and use the same
_instanceStore/realtime/channel retrieval logic to mirror startListening's
structure and ensure each listener branch is properly cleaned up.
---
Nitpick comments:
In @.github/workflows/check.yaml:
- Around line 55-61: The CI step "Run iOS unit tests" uses xcodebuild test with
a fixed destination name ('-destination "platform=iOS
Simulator,OS=latest,name=iPhone 16"') which can be missing on some macos-latest
images; change the -destination argument in that xcodebuild test invocation to a
more stable selector such as a device-family or a generic simulator (e.g.
replace the name=... selector with a consistently available device like
device=iPhone 8 or use a generic selector like just 'platform=iOS
Simulator,OS=latest' / 'platform=iOS Simulator,OS=latest,any' so Xcode can pick
an available simulator), updating the command in the "Run iOS unit tests" step
accordingly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea46c1ad-64ef-496b-9826-8352887b3809
📒 Files selected for processing (5)
.github/workflows/check.yamlios/Classes/AblyFlutterStreamHandler.mios/Classes/AblyStreamsChannel.mios/Tests/AblyStreamsChannelTests.mios/ably_flutter.podspec
ttypic
force-pushed
the
ECO-5703/fix-errors-because-of-flutter-engine-inactivity
branch
from
fbe6337 to
9787ec7
Compare
maratal
approved these changes
Apr 7, 2026
Collaborator
maratal
left a comment
maratal
left a comment
There was a problem hiding this comment.
LGTM (as a quick fix until more info available)
…mission - Added exception handling in `AblyStreamsChannel` to prevent crashes when FlutterEngine is stopped. - Improved logging to notify dropped events due to engine inactivity. - Added iOS unit tests for regression and normal operation of event sinks. - Configured GitHub Actions to run iOS unit tests on macOS.
ttypic
force-pushed
the
ECO-5703/fix-errors-because-of-flutter-engine-inactivity
branch
from
9787ec7 to
864e0bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ios_unit_tests.yml:
- Around line 22-39: The workflow is using the wrong test scheme name: update
the xcodebuild test invocation in the "Run iOS unit tests" step to use the
scheme generated by the podspec test_spec (change ably_flutter-Unit-Tests to
ably_flutter-Tests) so xcodebuild uses the scheme created by the prior pod lib
lint step (workspace at /tmp/ably_test_build/App.xcworkspace).
In `@ios/Classes/AblyStreamsChannel.m`:
- Around line 112-125: The try block currently wraps both envelope encoding and
sending in stream.sink, causing codec exceptions from _codec
encodeErrorEnvelope: or _codec encodeSuccessEnvelope: to be misreported as
engine inactivity; refactor stream.sink so you first compute the message (handle
FlutterEndOfEventStream, detect FlutterError and call _codec
encodeErrorEnvelope:, otherwise call _codec encodeSuccessEnvelope:) outside the
`@try`, then use a minimal `@try` around only [_messenger sendOnChannel:name
message:message] to catch messenger/engine errors; reference stream.sink,
FlutterEndOfEventStream, _codec encodeErrorEnvelope:/encodeSuccessEnvelope:, and
_messenger sendOnChannel:name message: to locate the changes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5dc6a9a1-90c4-45df-bb33-4b3a747c8298
📒 Files selected for processing (4)
.github/workflows/ios_unit_tests.ymlios/Classes/AblyStreamsChannel.mios/Tests/AblyStreamsChannelTests.mios/ably_flutter.podspec
🚧 Files skipped from review as they are similar to previous changes (1)
- ios/ably_flutter.podspec
ttypic
deleted the
ECO-5703/fix-errors-because-of-flutter-engine-inactivity
branch
Merged
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
2 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.
Resolves #594
AblyStreamsChannelto prevent crashes when FlutterEngine is stopped.Summary by CodeRabbit
Bug Fixes
Tests
Chores