Skip to content

fix(share_plus): Recover ShareSuccessManager state after error #2817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 7, 2024
Merged

fix(share_plus): Recover ShareSuccessManager state after error #2817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
022ec98
fix: recover state of ShareSuccessManager after errors
miquelbeltran Apr 5, 2024
b0e0d76
fix deadlock in ShareSuccessManager
miquelbeltran Apr 5, 2024
90c42ad
refactor
miquelbeltran Apr 5, 2024
5defc45
catch Throwable and pass error details up
miquelbeltran Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 42 additions & 46 deletions ...s/share_plus/android/src/main/kotlin/dev/fluttercommunity/plus/share/MethodCallHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the tailing comma to avoid code formatting to a single line

) : MethodChannel.MethodCallHandler {

override fun onMethodCall(call: MethodCall, result: MethodChannel.Result) {
expectMapArguments(call)

Comment on lines +15 to +16
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the top since it is common on all methods

// The user used a *WithResult method
val isResultRequested = call.method.endsWith("WithResult")
// We don't attempt to return a result if the current API version doesn't support it
val isWithResult = isResultRequested && Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1
val isWithResult =
isResultRequested && Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1

when (call.method) {
"shareUri" -> {
expectMapArguments(call)
share.share(
call.argument<Any>("uri") as String,
subject = null,
withResult= false
)
if (!isWithResult) {
result.success(null)
}
}
"share", "shareWithResult" -> {
expectMapArguments(call)
if (isWithResult && !manager.setCallback(result)) return
if (isWithResult && !manager.setCallback(result)) return
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the top since it is common for all methods as well.


// Android does not support showing the share sheet at a particular point on screen.
share.share(
call.argument<Any>("text") as String,
call.argument<Any>("subject") as String?,
isWithResult,
)
try {
when (call.method) {
"shareUri" -> {
share.share(
call.argument<Any>("uri") as String, subject = null, withResult = false
)
success(isWithResult, isResultRequested, result)
}

if (!isWithResult) {
if (isResultRequested) {
result.success("dev.fluttercommunity.plus/share/unavailable")
} else {
result.success(null)
}
"share", "shareWithResult" -> {
share.share(
call.argument<Any>("text") as String,
call.argument<Any>("subject") as String?,
isWithResult,
)
success(isWithResult, isResultRequested, result)
}
}
"shareFiles", "shareFilesWithResult" -> {
expectMapArguments(call)
if (isWithResult && !manager.setCallback(result)) return

// Android does not support showing the share sheet at a particular point on screen.
try {
"shareFiles", "shareFilesWithResult" -> {
share.shareFiles(
call.argument<List<String>>("paths")!!,
call.argument<List<String>?>("mimeTypes"),
call.argument<String?>("text"),
call.argument<String?>("subject"),
isWithResult,
)

if (!isWithResult) {
if (isResultRequested) {
result.success("dev.fluttercommunity.plus/share/unavailable")
} else {
result.success(null)
}
}
} catch (e: IOException) {
result.error("Share failed", e.message, null)
success(isWithResult, isResultRequested, result)
}

else -> result.notImplemented()
}
} catch (e: Throwable) {
manager.clear()
result.error("Share failed", e.message, e)
}
Comment on lines +56 to +59
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

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)
Comment on lines +62 to +71
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just moved all the result.success() code to a common method to avoid code duplication in the switch

}
else -> result.notImplemented()
}
}

Expand Down
8 changes: 8 additions & 0 deletions ...share_plus/android/src/main/kotlin/dev/fluttercommunity/plus/share/ShareSuccessManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ internal class ShareSuccessManager(private val context: Context) : ActivityResul
returnResult(RESULT_UNAVAILABLE)
}

/**
* Must be called on error to avoid deadlocking.
*/
fun clear() {
isCalledBack.set(true)
callback = null
}

/**
* Send the result to flutter by invoking the previously set callback.
*/
Expand Down