`discoverReaders` cancel callback does not actually wait for cancel to complete

[nprail]

Summary

If discoverReaders is in-progress and you call the cancel method, the callback on cancel does not actually wait for discoverReaders to complete. So if you promptly then call discoverReaders again (e.g. to switch discovery methods), it fails with a "SDK is busy" error.

Code to reproduce

The code sample here is a little complex as we are using the capacitor-stripe-terminal plugin (of which I am the maintainer).

In this example scenario, cancelDiscoverReaders RESOLVE COMPLETE is logged before discoverReaders-1 RESOLVE is logged and then the second discoverReaders fails with Could not execute discoverReaders because the SDK is busy with another command: discoverReaders.

JavaScript Code:

// assume discoverReaders is already in progress
await stripeTerminal.cancelDiscoverReaders();

stripeTerminal
  .discoverReaders({
    discoveryMethod: DiscoveryMethod.LocalMobile,
  })
  .subscribe({
    next: (readers) => {},
    error: (err) => {},
    complete: () => {}
  });

Swift Code (https://github.com/eventOneHQ/capacitor-stripe-terminal/blob/master/ios/Plugin/Plugin.swift):

@objc func discoverReaders(_ call: CAPPluginCall) {
    discoverReadersCount += 1

    let simulated = call.getBool("simulated") ?? true
    let method = UInt(call.getInt("discoveryMethod") ?? 0)
    let locationId = call.getString("locationId") ?? nil

    let discoveryMethod = StripeTerminalUtils.translateDiscoveryMethod(method: method)

    let config = DiscoveryConfiguration(
        discoveryMethod: discoveryMethod,
        locationId: locationId,
        simulated: simulated
    )

    print("discoverReaders-\(discoverReadersCount) START")
    self.pendingDiscoverReaders = Terminal.shared.discoverReaders(config, delegate: self) { error in
        if let error = error {
            print("discoverReaders-\(discoverReadersCount) ERROR")
            call.reject(error.localizedDescription, nil, error)
            self.pendingDiscoverReaders = nil
        } else {
            print("discoverReaders-\(discoverReadersCount) RESOLVE")
            call.resolve()
            self.pendingDiscoverReaders = nil
        }
    }
}

@objc func cancelDiscoverReaders(_ call: CAPPluginCall? = nil) {
    print("cancelDiscoverReaders START")

    guard let cancelable = pendingDiscoverReaders else {
        call?.resolve()
        print("cancelDiscoverReaders RESOLVE NOT ACTIVE")
        return
    }

    cancelable.cancel() { error in
        if let error = error as NSError? {
            print("cancelDiscoverReaders REJECT")
            call?.reject(error.localizedDescription, nil, error)
            self.pendingDiscoverReaders = nil
        } else {
            print("cancelDiscoverReaders RESOLVE COMPLETE")
            call?.resolve()
            self.pendingDiscoverReaders = nil
        }
    }

}

In this example, the first discoverReaders never resolves causing the second to fail.

discoverReaders-1 START
cancelDiscoverReaders START
cancelDiscoverReaders RESOLVE COMPLETE
discoverReaders-2 START
discoverReaders-2 RESOLVE
discoverReaders-2 ERROR
{"errorMessage":"Could not execute discoverReaders because the SDK is busy with another command: discoverReaders."}

If I add a 100 ms sleep between the cancel and second discoverReaders, the first discoverReaders resolves correctly and so the second starts.

discoverReaders-1 START
cancelDiscoverReaders START
cancelDiscoverReaders RESOLVE COMPLETE
sleep 100
discoverReaders-1 RESOLVE
discoverReaders-2 START

iOS version

16.3.1

Installation method

CocoaPods

SDK version

2.17.1

Other information

[bric-stripe]

Hi, thanks for filing the detailed report. Can confirm this is the current behavior and agree its not ideal that the cancel completes before the command that is being cancelled is fully completed. This is something on our backlog. Can't provide a timeline for when it will be fixed but this issue will be referenced in the changelog when it does and should be closed.

The recommendation though is to use the command's completion block as the source of truth for when the command actually completes. When you cancel the command the Terminal.shared.discoverReaders(config, delegate: self) { error in completion will be called with an error of SCPErrorCanceled. And when the cancel request errors the command may still be running and that would get the state out of sync. So my suggestions for the code you provided would be to not alter pendingDiscoverReaders in the cancelable callback and instead rely on the discoverReaders completion always being called and look for the canceled error explicitly. Something like:

    self.pendingDiscoverReaders = Terminal.shared.discoverReaders(config, delegate: self) { error in
        if let error = error,
           error.code.rawValue != .canceled { // I'm not 100% sure I have the syntax right here, but hopefully the idea makes sense (treat cancelled as success - though maybe you'd still want to reject that? not 100% sure which makes sense for your use case)
            print("discoverReaders-\(discoverReadersCount) ERROR")
            call.reject(error.localizedDescription, nil, error)
            self.pendingDiscoverReaders = nil
        } else {
            print("discoverReaders-\(discoverReadersCount) RESOLVE")
            call.resolve()
            self.pendingDiscoverReaders = nil
        }
    }

....

    cancelable.cancel() { error in
        if let error = error as NSError? {
            print("cancelDiscoverReaders REJECT")
            call?.reject(error.localizedDescription, nil, error)
            // self.pendingDiscoverReaders = nil  - rely on discovery completion block to clear
        } else {
            print("cancelDiscoverReaders RESOLVE COMPLETE")
            call?.resolve()
            // self.pendingDiscoverReaders = nil - rely on discovery completion block to clear
        }
    }

[bric-stripe]

👋 we were reviewing old issues and this one came up. Closing this out as this should now be behaving in a more reasonable way in the 3.x SDK. In 3.x although the cancelable's completion may be called before the command's completion is completed you're able to send new commands to the SDK without getting back an error. The SDK will queue up the command and start it as soon as it calls the completion on the previous command. Please re-open though if I missed anything though.