From 2f0194616c31110d01a4efc8b94ce2aa7a25cdbf Mon Sep 17 00:00:00 2001 From: vahidlazio <692343+vahidlazio@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:09:42 +0200 Subject: [PATCH] fix!: reply exactly once, on the platform thread, from async channel cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android's fetchAndActivate, activateAndFetchAsync and readAllFlags replied from inside coroutineScope.launch on Dispatchers.IO. A throw in the suspending body sent no reply at all, so the Dart future never completed — readAllFlags being the most reachable, since a corrupt flag cache makes Json.decodeFromString throw. Replies also have to be made on the platform thread. iOS had the Task {} equivalent of the threading problem. Each async body is now wrapped so every path replies exactly once, through a per-platform MainThreadResult that marshals to the main thread and drops any reply after the first. BREAKING CHANGE: iOS previously caught fetch/activate failures, logged them, and replied success. It now replies with a FlutterError, matching Android. `await fetchAndActivate()` and `await activateAndFetchAsync()` can therefore throw a PlatformException on iOS where they previously resolved silently. An app that cannot fetch flags should not be told it succeeded, but callers that relied on the silent-success behaviour need a catch. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ConfidenceFlutterSdkPlugin.kt | 66 ++++++++++++++++--- .../ConfidenceFlutterSdkPlugin.swift | 57 ++++++++++++++-- ...dence_flutter_sdk_method_channel_test.dart | 52 +++++++++++++++ 3 files changed, 160 insertions(+), 15 deletions(-) diff --git a/android/src/main/kotlin/com/example/confidence_flutter_sdk/ConfidenceFlutterSdkPlugin.kt b/android/src/main/kotlin/com/example/confidence_flutter_sdk/ConfidenceFlutterSdkPlugin.kt index e4e19bb..e321cd7 100644 --- a/android/src/main/kotlin/com/example/confidence_flutter_sdk/ConfidenceFlutterSdkPlugin.kt +++ b/android/src/main/kotlin/com/example/confidence_flutter_sdk/ConfidenceFlutterSdkPlugin.kt @@ -1,6 +1,9 @@ package com.example.confidence_flutter_sdk import android.content.Context +import android.os.Handler +import android.os.Looper +import android.util.Log import com.spotify.confidence.Confidence import com.spotify.confidence.ConfidenceFactory import com.spotify.confidence.ConfidenceValue @@ -19,6 +22,33 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.serialization.json.Json import java.io.File +import java.util.concurrent.atomic.AtomicBoolean + +private const val TAG = "ConfidenceFlutterSdk" + +/** + * Replies to a [Result] exactly once, on the platform (main) thread. + * + * Flutter requires channel replies on the main thread, and a second reply + * throws. The async cases in this plugin complete on [Dispatchers.IO], so + * neither guarantee holds at the call site — both are enforced here. + */ +private class MainThreadResult(private val delegate: Result) { + private val replied = AtomicBoolean(false) + + fun success(value: Any?) = replyOnce { delegate.success(value) } + + fun error(code: String, message: String?) = replyOnce { delegate.error(code, message, null) } + + private fun replyOnce(reply: () -> Unit) { + if (!replied.compareAndSet(false, true)) return + if (Looper.myLooper() == Looper.getMainLooper()) { + reply() + } else { + Handler(Looper.getMainLooper()).post(reply) + } + } +} /** ConfidenceFlutterSdkPlugin */ class ConfidenceFlutterSdkPlugin: FlutterPlugin, MethodCallHandler, ActivityAware { @@ -68,16 +98,28 @@ class ConfidenceFlutterSdkPlugin: FlutterPlugin, MethodCallHandler, ActivityAwar result.success(null) } "fetchAndActivate" -> { + val reply = MainThreadResult(result) coroutineScope.launch { - confidence.fetchAndActivate() - result.success(null) + try { + confidence.fetchAndActivate() + reply.success(null) + } catch (e: Throwable) { + Log.e(TAG, "fetchAndActivate failed", e) + reply.error("FETCH_AND_ACTIVATE_FAILED", e.message) + } } } "activateAndFetchAsync" -> { + val reply = MainThreadResult(result) coroutineScope.launch { - confidence.activate() - confidence.asyncFetch() - result.success(null) + try { + confidence.activate() + confidence.asyncFetch() + reply.success(null) + } catch (e: Throwable) { + Log.e(TAG, "activateAndFetchAsync failed", e) + reply.error("ACTIVATE_AND_FETCH_ASYNC_FAILED", e.message) + } } } "isStorageEmpty" -> { @@ -116,10 +158,18 @@ class ConfidenceFlutterSdkPlugin: FlutterPlugin, MethodCallHandler, ActivityAwar result.success(Json.encodeToString(NetworkConfidenceValueSerializer, value)) } "readAllFlags" -> { + val reply = MainThreadResult(result) coroutineScope.launch { - val flags = readAllFlags() - val map = flags.flags.associateBy({ it.flag }, { ConfidenceValue.Struct(it.value) }) - result.success(Json.encodeToString(NetworkConfidenceValueSerializer, ConfidenceValue.Struct(map))) + try { + val flags = readAllFlags() + val map = flags.flags.associateBy({ it.flag }, { ConfidenceValue.Struct(it.value) }) + reply.success(Json.encodeToString(NetworkConfidenceValueSerializer, ConfidenceValue.Struct(map))) + } catch (e: Throwable) { + // A corrupt or partially written cache file makes decoding throw; + // without a reply the Dart future would never complete. + Log.e(TAG, "readAllFlags failed", e) + reply.error("READ_ALL_FLAGS_FAILED", e.message) + } } } "putContext" -> { diff --git a/ios/confidence_flutter_sdk/Sources/confidence_flutter_sdk/ConfidenceFlutterSdkPlugin.swift b/ios/confidence_flutter_sdk/Sources/confidence_flutter_sdk/ConfidenceFlutterSdkPlugin.swift index e5a8c0b..b88255c 100644 --- a/ios/confidence_flutter_sdk/Sources/confidence_flutter_sdk/ConfidenceFlutterSdkPlugin.swift +++ b/ios/confidence_flutter_sdk/Sources/confidence_flutter_sdk/ConfidenceFlutterSdkPlugin.swift @@ -1,6 +1,35 @@ import Flutter import UIKit +/// Replies to a `FlutterResult` exactly once, on the main thread. +/// +/// Flutter requires channel replies on the main thread, and a second reply +/// traps. The `Task {}` cases in this plugin resume on an arbitrary executor, +/// so neither guarantee holds at the call site — both are enforced here. +final class MainThreadResult { + private let delegate: FlutterResult + private var replied = false + private let lock = NSLock() + + init(_ delegate: @escaping FlutterResult) { + self.delegate = delegate + } + + func reply(_ value: Any?) { + lock.lock() + let alreadyReplied = replied + replied = true + lock.unlock() + if alreadyReplied { return } + + if Thread.isMainThread { + delegate(value) + } else { + DispatchQueue.main.async { self.delegate(value) } + } + } +} + public class ConfidenceFlutterSdkPlugin: NSObject, FlutterPlugin { public static func register(with registrar: FlutterPluginRegistrar) { let channel = FlutterMethodChannel(name: "confidence_flutter_sdk", binaryMessenger: registrar.messenger()) @@ -59,35 +88,49 @@ public class ConfidenceFlutterSdkPlugin: NSObject, FlutterPlugin { result(confidence.isStorageEmpty()) break; case "fetchAndActivate": + let fetchReply = MainThreadResult(result) Task { guard let confidence = self.confidence else { - result("") + fetchReply.reply("") return } do { try await confidence.fetchAndActivate() + fetchReply.reply("") } catch { - NSLog("%@", "Confidence SDK: \(error)") + NSLog("%@", "Confidence SDK: fetchAndActivate failed: \(error)") + fetchReply.reply( + FlutterError( + code: "FETCH_AND_ACTIVATE_FAILED", + message: "\(error)", + details: nil)) } - result("") - return } break; case "activateAndFetchAsync": + let activateReply = MainThreadResult(result) Task { guard let confidence = self.confidence else { - result("") + activateReply.reply("") return } do { try confidence.activate() } catch { - NSLog("%@", "Confidence SDK: \(error)") + NSLog("%@", "Confidence SDK: activate failed: \(error)") + activateReply.reply( + FlutterError( + code: "ACTIVATE_AND_FETCH_ASYNC_FAILED", + message: "\(error)", + details: nil)) + return } + // Deliberately not awaited: asyncFetch refreshes in the + // background and its outcome is not part of this reply. Task { await confidence.asyncFetch() } - result("") + activateReply.reply("") } break; case "putContext": diff --git a/test/confidence_flutter_sdk_method_channel_test.dart b/test/confidence_flutter_sdk_method_channel_test.dart index 3f128a2..ea04abc 100644 --- a/test/confidence_flutter_sdk_method_channel_test.dart +++ b/test/confidence_flutter_sdk_method_channel_test.dart @@ -154,4 +154,56 @@ void main() { expect((data['d'] as Map)['type'], 'double'); }); }); + + // The native change here (try/catch plus a main-thread reply in the Android + // and iOS plugins) cannot be exercised from Dart, because these tests mock + // the platform side entirely. What IS pinned below is the Dart contract the + // native fix depends on: these futures must COMPLETE when native replies + // with an error, rather than swallowing it or hanging forever. If anyone + // adds an error-swallowing `.catchError` to these methods, or drops the + // `await`, these tests fail. + group('async platform methods surface native errors', () { + void mockNativeError(String code) { + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(platform.methodChannel, (methodCall) async { + methodCalls.add(methodCall); + throw PlatformException(code: code, message: 'native failed'); + }); + } + + test('fetchAndActivate completes with an error rather than hanging', + () async { + mockNativeError('FETCH_AND_ACTIVATE_FAILED'); + + await expectLater( + platform.fetchAndActivate(), + throwsA(isA() + .having((e) => e.code, 'code', 'FETCH_AND_ACTIVATE_FAILED')), + ); + expect(methodCalls.single.method, 'fetchAndActivate'); + }); + + test('activateAndFetchAsync completes with an error rather than hanging', + () async { + mockNativeError('ACTIVATE_AND_FETCH_ASYNC_FAILED'); + + await expectLater( + platform.activateAndFetchAsync(), + throwsA(isA() + .having((e) => e.code, 'code', 'ACTIVATE_AND_FETCH_ASYNC_FAILED')), + ); + expect(methodCalls.single.method, 'activateAndFetchAsync'); + }); + + test('readAllFlags completes with an error rather than hanging', () async { + mockNativeError('READ_ALL_FLAGS_FAILED'); + + await expectLater( + platform.readAllFlags(), + throwsA(isA() + .having((e) => e.code, 'code', 'READ_ALL_FLAGS_FAILED')), + ); + expect(methodCalls.single.method, 'readAllFlags'); + }); + }); }