Prevent crash from plugin interference in auth flows#45
Prevent crash from plugin interference in auth flows#45muteeb-shahid1 wants to merge 1 commit intokinde-oss:mainfrom
Conversation
Handle "Reply already submitted" PlatformException gracefully by converting to KindeError instead of reporting crash when third-party plugins consume activity results improperly.
WalkthroughThis pull request adds detection and handling for plugin interference errors on Android. The Android plugin implements ActivityAware lifecycle management without direct result handling, while Flutter-side code detects "Reply already submitted" exceptions and maps them to a new pluginInterference error code. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
android/src/main/kotlin/com/kinde/kinde_flutter_sdk/KindeFlutterSdkPlugin.kt (1)
64-75: Consider clarifying the actual benefit of this ActivityAware implementation.The extensive comments explain that this implementation doesn't handle activity results and that the real protection is Flutter-side error handling. If the stored activity references aren't used and no result listeners are added, what specific benefit does implementing ActivityAware provide?
The comments mention "proper plugin registration order" (line 50), but it's unclear how this implementation affects registration order.
Consider either:
- Adding a comment explaining the concrete benefit (e.g., "ensures proper lifecycle cleanup" or "required for X")
- Simplifying if this is primarily scaffolding for future enhancements
lib/src/error/kinde_error.dart (2)
135-137: Add parentheses for clarity in the compound condition.While the operator precedence is technically correct (since
&&binds tighter than||), the mixed boolean operators on line 137 would benefit from explicit parentheses for improved readability.- if (message.contains("Reply already submitted") || - details.contains("Reply already submitted") || - message.contains("IllegalStateException") && message.contains("Reply")) { + if (message.contains("Reply already submitted") || + details.contains("Reply already submitted") || + (message.contains("IllegalStateException") && message.contains("Reply"))) {
138-144: Consider whether the error message is too technical for end users.The error message includes internal implementation details like "requestCode filtering" and explains the technical cause. While this is helpful for debugging, it might be overwhelming for end users who just need to know authentication failed.
Consider whether a simpler message would be more appropriate:
message: "Authentication failed due to plugin conflict. Please contact support if this persists. " "Technical details: ${platformException.message ?? 'Unknown'}",Alternatively, keep the detailed message if your SDK is primarily used by developers who would benefit from the technical context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
android/src/main/kotlin/com/kinde/kinde_flutter_sdk/KindeFlutterSdkPlugin.kt(2 hunks)lib/src/error/kinde_error.dart(1 hunks)lib/src/error/kinde_error_code.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:110-124
Timestamp: 2025-06-20T10:43:27.820Z
Learning: The PR #27 for the Flutter starter kit example app is designed to work with Kinde Flutter SDK version 1.0.4, where the register() method will return a token if registration is successful. The current build errors are expected since the code targets a future SDK version.
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:79-94
Timestamp: 2025-06-20T10:44:49.809Z
Learning: In the Kinde Flutter SDK, version 1.0.4 will change the login() method to return a token when login is successful, rather than returning void. Code in this repository may be written to anticipate this future API change.
📚 Learning: 2025-06-20T10:43:27.820Z
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:110-124
Timestamp: 2025-06-20T10:43:27.820Z
Learning: The PR #27 for the Flutter starter kit example app is designed to work with Kinde Flutter SDK version 1.0.4, where the register() method will return a token if registration is successful. The current build errors are expected since the code targets a future SDK version.
Applied to files:
lib/src/error/kinde_error.dart
📚 Learning: 2025-06-20T10:44:49.809Z
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:79-94
Timestamp: 2025-06-20T10:44:49.809Z
Learning: In the Kinde Flutter SDK, version 1.0.4 will change the login() method to return a token when login is successful, rather than returning void. Code in this repository may be written to anticipate this future API change.
Applied to files:
android/src/main/kotlin/com/kinde/kinde_flutter_sdk/KindeFlutterSdkPlugin.kt
🔇 Additional comments (5)
android/src/main/kotlin/com/kinde/kinde_flutter_sdk/KindeFlutterSdkPlugin.kt (3)
3-3: LGTM! Clear documentation of the problem.The imports and documentation clearly explain the plugin interference issue being addressed. The ActivityAware implementation provides proper activity lifecycle integration.
Also applies to: 7-8, 14-23
77-90: LGTM! Proper lifecycle management.The lifecycle methods correctly handle activity references during configuration changes and detachment, following Android best practices.
29-30: Activity references are stored but not used—this is intentional per ActivityAware contract.Verification confirms the
activityandactivityPluginBindingfields are assigned in lifecycle methods but never read. However, this is the correct pattern for implementing Flutter'sActivityAwareinterface. The code explicitly documents (lines 68–74) that activity result handling is intentionally avoided here since flutter_appauth handles it. The fields must be managed through the lifecycle even when not directly used—this is part of the contract compliance and ensures proper cleanup during configuration changes.lib/src/error/kinde_error_code.dart (1)
35-37: LGTM! Clear and consistent error code addition.The new
pluginInterferenceerror code is well-documented and follows the naming conventions of other error codes in the file.lib/src/error/kinde_error.dart (1)
130-144: Pragmatic approach to detecting plugin interference, but verify robustness.The string-matching approach is pragmatic for detecting the "Reply already submitted" error. However, it relies on specific error message formats that could change in future versions of Flutter or the underlying libraries.
Consider verifying this handles the actual error formats you're seeing. You may want to add logging during testing to confirm the detection works as expected:
// Temporary logging for verification (remove after testing) if (message.isNotEmpty || details.isNotEmpty) { print("PlatformException - message: $message, details: $details"); }
1 participant
Handle "Reply already submitted" PlatformException gracefully by converting to KindeError instead of reporting crash when third-party plugins consume activity results improperly.
Explain your changes
I’ve hardened the Android login flow so the “Reply already submitted” case no longer report the crashes. It’s now caught and returned as a clean KindeError (pluginInterference), leaving the happy path unchanged.
Code changes:
///CODE LEVEL CHANGES START
In kinde_error_code file
static const pluginInterference = "plugin-interference";
In kinde_error.dart
// Handle "Reply already submitted" error caused by plugin interference
// This occurs when rogue plugins consume activity results without proper requestCode filtering
final message = platformException.message ?? "";
final details = platformException.details?.toString() ?? "";
if (message.contains("Reply already submitted") ||
details.contains("Reply already submitted") ||
message.contains("IllegalStateException") && message.contains("Reply")) {
return KindeError(
code: KindeErrorCode.pluginInterference,
message: "Plugin interference detected. Another plugin is consuming activity results without proper filtering. "
"This is typically caused by plugins that don't filter activity results by requestCode. "
"Original error: ${platformException.message ?? 'Unknown'}",
);
}
in KindeFlutterSDKPlugin file
added activity result handling for these type of exceptions
override fun onAttachedToActivity(binding: ActivityPluginBinding) {
activity = binding.activity
activityPluginBinding = binding
}
override fun onDetachedFromActivityForConfigChanges() {
activity = null
activityPluginBinding = null
}
override fun onReattachedToActivityForConfigChanges(binding: ActivityPluginBinding) {
activity = binding.activity
activityPluginBinding = binding
}
override fun onDetachedFromActivity() {
activity = null
activityPluginBinding = null
}
///END OF CODE CHANGES
Checklist