Merge pull request #489 from ably/more-safe-channel-wrapper

feat: protect from multiple `MethodChannel.Result` invocation
ably · Oct 25, 2023 · dbdef15 · dbdef15
2 parents 801eb34 + 19f7307
commit dbdef15
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 35 deletions.
diff --git a/android/src/main/java/io/ably/flutter/plugin/AblyMethodCallHandler.java b/android/src/main/java/io/ably/flutter/plugin/AblyMethodCallHandler.java
@@ -133,46 +133,13 @@ public AblyMethodCallHandler(final MethodChannel methodChannel,
     _map.put(PlatformConstants.PlatformMethod.cryptoGenerateRandomKey, this::cryptoGenerateRandomKey);
   }
 
-  // MethodChannel.Result wrapper that responds on the platform thread.
-  //
-  // Plugins crash with "Methods marked with @UiThread must be executed on the main thread."
-  // This happens while making network calls in thread other than main thread
-  //
-  // https://github.com/flutter/flutter/issues/34993#issue-459900986
-  // https://github.com/aloisdeniel/flutter_geocoder/commit/bc34cfe473bfd1934fe098bb7053248b75200241
-  private static class MethodResultWrapper implements MethodChannel.Result {
-    private final MethodChannel.Result methodResult;
-    private final Handler handler;
-
-    MethodResultWrapper(MethodChannel.Result result) {
-      methodResult = result;
-      handler = new Handler(Looper.getMainLooper());
-    }
-
-    @Override
-    public void success(final Object result) {
-      handler.post(() -> methodResult.success(result));
-    }
-
-    @Override
-    public void error(
-        final String errorCode, final String errorMessage, final Object errorDetails) {
-      handler.post(() -> methodResult.error(errorCode, errorMessage, errorDetails));
-    }
-
-    @Override
-    public void notImplemented() {
-      handler.post(methodResult::notImplemented);
-    }
-  }
-
   private void handleAblyException(@NonNull MethodChannel.Result result, @NonNull AblyException e) {
     result.error(Integer.toString(e.errorInfo.code), e.getMessage(), e.errorInfo);
   }
 
   @Override
   public void onMethodCall(@NonNull MethodCall call, @NonNull MethodChannel.Result rawResult) {
-    final MethodChannel.Result result = new MethodResultWrapper(rawResult);
+    final MethodChannel.Result result = new MainThreadMethodResult(call.method, rawResult);
     Log.v(TAG, String.format("onMethodCall: Ably Flutter platform method \"%s\" invoked.", call.method));
     final BiConsumer<MethodCall, MethodChannel.Result> handler = _map.get(call.method);
     if (null == handler) {
@@ -183,7 +150,7 @@ public void onMethodCall(@NonNull MethodCall call, @NonNull MethodChannel.Result
         // We have a handler for a method with this name so delegate to it.
         handler.accept(call, result);
       } catch (Exception e) {
-        Log.e(TAG, String.format("\"%s\" platform method received error during invocation", call.method), e);
+        Log.e(TAG, String.format("\"%s\" platform method received error during invocation, caused by: %s", call.method, e.getMessage()), e);
       }
     }
   }

diff --git a/android/src/main/java/io/ably/flutter/plugin/MainThreadMethodResult.java b/android/src/main/java/io/ably/flutter/plugin/MainThreadMethodResult.java
@@ -0,0 +1,56 @@
+package io.ably.flutter.plugin;
+
+import android.os.Handler;
+import android.os.Looper;
+
+import io.ably.lib.util.Log;
+import io.flutter.plugin.common.MethodChannel;
+
+// MethodChannel.Result wrapper that responds on the platform thread.
+//
+// Plugins crash with "Methods marked with @UiThread must be executed on the main thread."
+// This happens while making network calls in thread other than main thread
+//
+// https://github.com/flutter/flutter/issues/34993#issue-459900986
+// https://github.com/aloisdeniel/flutter_geocoder/commit/bc34cfe473bfd1934fe098bb7053248b75200241
+public class MainThreadMethodResult implements MethodChannel.Result {
+    private static final String TAG = MainThreadMethodResult.class.getName();
+    private final String methodName;
+
+    private final MethodChannel.Result methodResult;
+    private final Handler handler;
+
+    MainThreadMethodResult(String methodName, MethodChannel.Result result) {
+        this.methodName = methodName;
+        methodResult = result;
+        handler = new Handler(Looper.getMainLooper());
+    }
+
+    @Override
+    public void success(final Object result) {
+        handler.post(() -> {
+            try {
+                methodResult.success(result);
+            } catch (Exception e) {
+                Log.e(TAG, String.format("\"%s\" platform method received error during invocation, caused by: %s", methodName, e.getMessage()), e);
+            }
+        });
+    }
+
+    @Override
+    public void error(
+            final String errorCode, final String errorMessage, final Object errorDetails) {
+        handler.post(() -> {
+            try {
+                methodResult.error(errorCode, errorMessage, errorDetails);
+            } catch (Exception e) {
+                Log.e(TAG, String.format("\"%s\" platform method received error during invocation, caused by: %s", methodName, e.getMessage()), e);
+            }
+        });
+    }
+
+    @Override
+    public void notImplemented() {
+        handler.post(methodResult::notImplemented);
+    }
+}