fix(share_plus): Recover ShareSuccessManager state after error #2817
Conversation
miquelbeltran
changed the title
| @@ -8,72 +8,68 @@ import java.io.IOException | |||
| /** Handles the method calls for the plugin. */ | |||
| internal class MethodCallHandler( | |||
| private val share: Share, | |||
| private val manager: ShareSuccessManager | |||
| private val manager: ShareSuccessManager, | |||
There was a problem hiding this comment.
Added the tailing comma to avoid code formatting to a single line
| expectMapArguments(call) | ||
|
|
There was a problem hiding this comment.
Moved to the top since it is common on all methods
| "share", "shareWithResult" -> { | ||
| expectMapArguments(call) | ||
| if (isWithResult && !manager.setCallback(result)) return | ||
| if (isWithResult && !manager.setCallback(result)) return |
There was a problem hiding this comment.
Moved to the top since it is common for all methods as well.
...plus/share_plus/android/src/main/kotlin/dev/fluttercommunity/plus/share/MethodCallHandler.kt
Outdated
Show resolved
Hide resolved
| private fun success( | ||
| isWithResult: Boolean, | ||
| isResultRequested: Boolean, | ||
| result: MethodChannel.Result | ||
| ) { | ||
| if (!isWithResult) { | ||
| if (isResultRequested) { | ||
| result.success("dev.fluttercommunity.plus/share/unavailable") | ||
| } else { | ||
| result.success(null) |
There was a problem hiding this comment.
Just moved all the result.success() code to a common method to avoid code duplication in the switch
| } catch (e: Throwable) { | ||
| manager.clear() | ||
| result.error("Share failed", e.message, e) | ||
| } |
There was a problem hiding this comment.
Catch all errors from share methods, pass them up to the Flutter layer, but before also call to manager.clear() to avoid the deadlock state in ShareSuccessManager.
This can be tested simply by calling
await Share.shareXFiles(
[XFile('/wrong/file')],
text: text,
);then the user sees something like
E/flutter (29015): [ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: PlatformException(Share failed, /wrong/file: The source file doesn't exist.,
however, they will be able to use the share methods again later on because manager.clear() was called.
|
I believe it also closes #2515 even though the report didn't bother to provide meaningful details requested. |
Description
ShareSuccessManagerwas in a bad state which blocked users from continuing using the plugin.ShareSuccessManagerchecks the state of the current callback state, and fails if a callback was never executed. This is problematic because causes this state to be never recover after an error happened.onActivityResultis never called, so the state is never restored.clear()method that will reset the state, only call to this method in cases we catch an error.MethodCallHandler.shareXFileswith badXFile(wrong paths), which causes an exception. Then I call thesharemethods normally and still works, both with result and without.ShareSuccessManageris in a bad state, we can use the clear method in those edge cases to clean up the state.Related Issues
Checklist
CHANGELOG.mdnor the plugin version inpubspec.yamlfiles.flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
!in the title as explained in Conventional Commits).