braze-inc / braze-swift-sdk

https://www.braze.com
Other
47 stars 19 forks source link

[Bug]: Calling id() on Braze user throws continuation leaked warning when Braze enabled is false #119

Closed morluna closed 1 month ago

morluna commented 3 months ago

Platform

iOS

Platform Version

17.4

Braze SDK Version

8.2.1

Xcode Version

15.3

Computer Processor

Apple (M1)

Repro Rate

100%

Steps To Reproduce

This isn't an error right now so no rush, but I know upcoming Swift versions will begin enforcing async warnings like this.

We are in the process of integrating Braze with our iOS app. In an effort to reduce the events we send in our staging environment and minimize our bill, we added some code like this:

func setUserID(_ userID: String) {
    Task(priority: .low) {
        let currentBrazeUserID = await braze.user.id()

        guard currentBrazeUserID != userID else { return }

        braze.changeUser(userId: userID)
    }
}

When braze.enabled = true, this code works with no warnings. However, if braze.enabled = false, this throws the following error:

Braze SDK disabled: Cannot read key path \State.user.<computed 0x0000000106a56314 (Optional<String>)>
SWIFT TASK CONTINUATION MISUSE: id() leaked its continuation!

Expected Behavior

The expected behavior is that this doesn't throw any warnings from the system. I put together a snippet of what I'm assuming your code looks like under the hood:

final class BrazeUser {
    var enabled: Bool = true

    private var id: String?

    func id(queue: DispatchQueue = .main, completion: @escaping (String?) -> Void) {
        guard enabled else {
            return
        }

        completion(id)
    }

    func id() async -> String? {
        await withCheckedContinuation { continuation in
            id { userID in
                continuation.resume(returning: userID)
            }
        }
    }

    func setUserID(_ newValue: String) {
        id = newValue
    }
}

Task {
    // Enabled
    let user = BrazeUser()
    user.enabled = true

    let enabledUserNilId = await user.id()

    print(enabledUserNilId) // nil

    user.setUserID("1234")

    let enabledUserId = await user.id()

    print(enabledUserId) // 1234

    // Disabled
    user.enabled = false

    let disabledUserId = await user.id() // SWIFT TASK CONTINUATION MISUSE: id() leaked its continuation!

    print(disabledUserId)
}

The error here is that if you're using the completion block version within the async one, you must always make sure to call the completion block, even in the return case. In the example above, the warning can be fixed by updating the code to:

func id(queue: DispatchQueue = .main, completion: @escaping (String?) -> Void) {
        guard enabled else {
            completion(nil)
            return
        }

        completion(id)
    }

Actual Incorrect Behavior

More info pasted above.

Verbose Logs

No response

Additional Information

No response

hokstuff commented 3 months ago

Hi @morluna,

Thanks for filing this issue with information around this continuation leak warning when using the [id() API](https://braze-inc.github.io/braze-swift-sdk/documentation/brazekit/braze/user-swift.class/id()), which calls Apple's API withCheckedContinuation. We will look into our implementation to attempt to resolve this warning. For debugging purposes, do you see the warning SWIFT TASK CONTINUATION MISUSE [...] get printed out directly in Xcode, or is it when using an additional debugging tool?


In the mean time, there are a couple things that you can do to prevent the warning for your use case:

  1. You do not need to add in a check prior to calling changeUser, because if you call changeUser to the same exact "id", the SDK will simply log a warning and no event will be sent to the server (meaning no data point consumption)
    let currentBrazeUserID = await braze.user.id()
    guard currentBrazeUserID != userID else { return } // This check isn't needed
    braze.changeUser(userId: userID)
  2. You can use the API id(queue:completion:) which doesn't support async/await but doesn't use that Apple API which is causing the error. This should alleviate the warning

Thanks!

morluna commented 3 months ago

@hokstuff yes you can see this in the Xcode debugger directly!

And thanks for the clarification on point #1. We're onboarding with Braze and our Braze rep wasn't sure about this so we thought we'd be super safe.

hokstuff commented 1 month ago

I am closing out this issue since it seems like the alternative API addresses the desired use case. Thanks!