gematik / ref-OpenHealthCardKit

Controlling/Use-case framework for accessing smart cards of the telematic infrastructure. API Documentation: https://gematik.github.io/ref-OpenHealthCardKit
Apache License 2.0
15 stars 6 forks source link

operationContinuation?.resume(throwing: ...) leads to fatal error #21

Closed kaaoo closed 3 months ago

kaaoo commented 5 months ago

Hello! I'm using library version 5.6.0. And I having an issue that

    public func tagReaderSession(_: NFCTagReaderSession, didInvalidateWithError error: Swift.Error) {
        DLog("NFC reader session was invalidated: \(error)")
        let coreNFCError = error.asCoreNFCError()
        operationContinuation?.resume(throwing: NFCHealthCardSessionError.coreNFC(coreNFCError))
        operationContinuation = nil
    }

This operationContinuation?.resume causes a fatal error: Fatal error: SWIFT TASK CONTINUATION MISUSE: executeOperation() tried to resume its continuation more than once, throwing coreNFC(NFCCardReaderProvider.CoreNFCError.sessionInvalidated(__C_Synthesized.related decl 'e' for NFCReaderError(_nsError: Error Domain=NFCError Code=202 "Session invalidated unexpectedly" UserInfo={NSLocalizedDescription=Session invalidated unexpectedly})))!

What is happening exactly? I'm trying to scan the card, sometimes the card gets lost and I see that during the scanning operationContinuation?.resume(returning: outcome) is being called once. After the operation is completed I click on cancel button, which fires public func tagReaderSession(_: NFCTagReaderSession, didInvalidateWithError error: Swift.Error) { and then of course I get that fatal error, because resume is supposed to be called only once.

Am I doing something wrong or that issue is a known one?

sigabrtz commented 5 months ago

Hi.

The NFCHealthCardSession should be designed in a way that operationContinuation.resume(xyz) is called only once.

Although after some analysing it seems possible that if the NFCHealthCardSession instance isn't de-initialized after returning an outcome, the user NFC-dialogue is still kept open and the user could tap the "Cancel" button which then indeed would cause operationContinuation?.resume(throwing:) to be called unintentionally.

This shouldn't happen within to provided NFCDemo target (, or does it?).

kaaoo commented 5 months ago

The question is how to execute a many different operations in one tag reader session? We need to have bi-directional messaging. We use version 5.3 right now, and there it's possible to do that - you can save the HealthCardType object in memory once and then execute different operations there in a secure session. Just to give you more inputs - I use this library together with react-native, so we're basically providing an entry points methods to this library, and APDU command that should be executed on the egk comes from the react-native layer. And I don't see a way to do the same thing on version 5.6. And on the NFCDemo code I see only one operation being executed per session.

sigabrtz commented 4 months ago

Thank you for the clarification.

I was able to reproduce the bug. It shows itself when one is holding onto the NFCHealthCardSession object after it's executeOperation() already returned successfully once and then the tag connection is lost after that.

I think it's fixable by inserting a operationContinuation = nil in line 277 into NFCHealthCardSession. I will try to get it through the internal review quickly.

That being said, a NFCHealthCardSession object is not intended to be used the way you describe (holding onto the object in memory to be able to send commands at a later time). We (developers for the e-prescription app) experienced that the NFC-tag connection is too finicky to keep it open during super long living operations. (As you said yourself: "sometimes the card gets lost and I see that during the scanning". And "sometimes" is arguable when talking about day-to-day users.) So in our opinion it's a good idea to limit the time the tag reader session is opened to a minimum.

Our solution to mitigate this real world problem was to design NFCHealthCardSession in that way that everything you want to communicate to the Health Card should be stated within the operation closure of the initializer. So that after executing the operation you either end up successfully with a value (of a defined type) or an error (basically of type NFCHealthCardSessionError). That suggests that every computation/awaiting regarding information for the card should be done if possible beforehand opening the tag reader session. Maybe it's also worth to think about splitting up an otherwise long living NFCHealthCardSession into two separate ones?

sigabrtz commented 4 months ago

I published a new version 5.8.0 that hopefully fixes the reported bug (it fixes at least the peculiarity I could reproduce).

We also set the access level of the objects in the NFCCardReaderProvider module to public as promised in #22. This should aid in the need for further customization.