The callback execution thread is not consistent

lionhylra commented 1 year ago

Checklist

[X] The issue can be reproduced in the Auth0.swift sample app (or N/A).
[X] I have looked into the Readme, Examples, and FAQ and have not found a suitable solution or answer.
[X] I have looked into the API documentation and have not found a suitable solution or answer.
[X] I have searched the issues and have not found a suitable solution or answer.
[X] I have searched the Auth0 Community forums and have not found a suitable solution or answer.
[X] I agree to the terms within the Auth0 Code of Conduct.

Description

When calling start on webAuth in below example:

Auth0
    .webAuth()
    .parameters(["screen_hint": "signup"])
    .start { result in
        switch result {
        case .success(let credentials):
            print("Obtained credentials: \(credentials)")
        case .failure(let error):
            print("Failed with: \(error)")
        }
    }

The executing thread of callback is not consistent. When credentials returns, the callback is executed on main thread, because JWTValidator dispatch the callback to main queue here. When there is an error, the callback is executed on background thread here.

The documentation and example didn't mention whether the client needs to dispatch code to main queue or not. If there is an error, app usually need to update UI to show an error message.

Reproduction

Go through webAuth process and trigger an error.

Additional context

No response

Auth0.swift version

2.5.0

Platform

iOS

Platform version(s)

16.4

Xcode version

14.3.1

Package manager

Cocoapods

Widcket commented 1 year ago

Hi @lionhylra, thanks for raising this.

I'm unable to reproduce this issue; both error and success results are being delivered on the main thread. Which specific error are you triggering?

lionhylra commented 1 year ago

I did web auth on a client who's authentication method is POST. This will trigger .pkceNotAllowed error, but the callback was executed on a background queue. As I mentioned in the issue description, it was this line of code: https://github.com/auth0/Auth0.swift/blob/d621132e983195a6094322ab0c2140bde746fecb/Auth0/OAuth2Grant.swift#L84-L84

Widcket commented 1 year ago

Thanks for the clarification. The long-standing convention on Apple plaftorms is to execute callbacks in whatever queue the last operation was executed. Following this, as you mentioned, we don't specify in our docs that the callbacks will be called in any specific queue. However, I agree that given the need to deal with UI updates shortly after in case of error, calling it on the main thread would improve the developer experience.

I don't have an ETA for this, but I'll leave this open for future planning and reference.

Widcket commented 1 year ago

Hi, given that:

Addressing this on the current major could lead to unnecessarily dispatching to the main queue twice (once by Auth0.swift, and then again by the current application code)
Even though always delivering the result on the main thread would improve DX, it's a nice-to-have
The current behavior follows a long-standing convention

We'll defer this improvement to the next (future) major. I've already made a note of this, so I'll go ahead and close this issue.

Thanks for bringing this to our attention!

auth0 / Auth0.swift