Alamofire: Feature request: ability to change the request asynchronously in the RequestAdapter

[ricardopereira]

• [x] I've read, understood, and done my best to follow the *CONTRIBUTING guidelines.

What did you do?

As I mentioned here, I want to request a new access token in my RequestAdapter implementation when the token has expired. I know in advance when will the access token expire so, I think it would be nice to spare a request that will probably answer with a 401.

What did you expect to happen?

I was expecting something like:

typealias RequestAdaptCompletion = (_ urlRequest: URLRequest) -> Void

func adapt(_ urlRequest: URLRequest, completion: @escaping RequestAdaptCompletion) throws {
    if userTokenDetails.expires.timeIntervalSince(userTokenDetails.issued) <= 0 {
        reauthorize { newUserTokenDetails in
            urlRequest.setValue("Bearer " + newUserTokenDetails.accessToken, forHTTPHeaderField: "Authorization")
            completion(urlRequest)
        }
    }
    else {
        completion(urlRequest)
    }
}

@ricardopereira It would be much easier to just allow adapter errors to trigger retry behavior. I'm not sure if that would work now, but it might become possible for Alamofire 5. Such a feature likely wouldn't ship until then anyway.

but as @jshier mentioned in #1450, maybe triggering an error in the Adapter and call retry should be enough (at least for my case 😅).

UPDATE:

Message from @cnoon:

This can actually already be done on AF4.x. I know b/c we're doing it in our internal Nike networking library.

How?

We throw custom AdaptError types tailored to our use cases. For example, we have .missingCredential, .expiredCredential, and I believe one other case that we'll throw from adapt. Then in retry, we inspect the error to see if it was an AdaptError. If it was .expired, then we know we need to refresh. This allows us to avoid making the initial request with a credential we already know is expired.

Alamofire Environment

Alamofire version: 4.7.2
Xcode version: 9.3.1
Swift version: 4.0
Platform(s) running Alamofire: iOS
macOS version running Xcode: High Sierra 10.13.4 (17E202)

Demo Project

No demo but I can arrange something if needed.

🤩 Thank you for your amazing work!

[jshier]

While I'm pretty sure this will work as @cnoon described previously, you can also try the alamofire5-core-rewrite branch and see what behavior you get there. AdaptError has been removed, you should check for your adapter errors in the retrier before deciding to retry.

[francos]

Another case to make for this is that sometimes we are using 3rd party libraries for authorization which only provide async methods to get the access token (e.g., Firebase) and we can't use the RequestAdapter to add the access token to all the requests, nor can we change the third party library APIs. This feature would be great for this.

[TwunTee]

@FrancoSabadini how are you sending the firebase token. I am using getIdToken() and if that fails i am using the retrier, do you have any better solution.

[francos]

Hi @TwunTee, I'm using getIDTokenResult(), but it receives a closure, it doesn't return the result synchronously. This is from the Firebase SDK code:

- (void)getIDTokenResultForcingRefresh:(BOOL)forceRefresh
                            completion:(nullable FIRAuthTokenResultCallback)completion

From what I see, getIdToken() also returns the token in a closure:

- (void)getIDTokenWithCompletion:(nullable FIRAuthTokenCallback)completion
    NS_SWIFT_NAME(getIDToken(completion:));

Is the method you are using synchronous? What version of the Firebase SDK are you using?

[TwunTee]

@FrancoSabadini I am using the same method too. I am doing it as part of the retrier which accepts a completion block

[jshier]

This would actually be pretty trivial to implement for Alamofire 5, given the entire request creation process is now async. It just depends on whether the API change to RequestAdapter is worth it.

[francos]

@jshier I think that if the whole request creation is async in version 5 then making this API async would make sense to keep everything consistent, rather than keeping some APIs sync and others async.

[cnoon]

I agree here with @FrancoSabadini that we should look to make the adapter async in AF5. It was always a bit clunky in AF4 to have to force the request to have to fail before you could fetch a new token. It was the only viable approach at the time though given that request creation was synchronous. Now that it is async, the adaptation process could be smoothed out by making it async to match the retry API.

This will make certain cases a bit more complex though, but I think it's worth the tradeoff. Now you'll have to handle the case where you could be managing a queue of adapting requests as well as retrying requests. You'll also have to handle the case where both adapt and retry could be trying to refresh the credential at the same time. However, I don't think this adds a huge amount of complexity, just something to be away of.

[jshier]

Looking into implementing async RequestAdapters now.

[jshier]

Making RequestAdapter async would require changing the function from:

func adapt(_ urlRequest: URLRequest) throws -> URLRequest

to:

func adapt(_ urlRequest: URLRequest, completion: @escaping (_ result: Result<URLRequest) -> Void)

due to the need to preserve the error. This could be rather awkward, but I'll see how it feels.

[jshier]

@ricardopereira @FrancoSabadini Take a look at #2628 and see what you think.

[ricardopereira]

@jshier It looks good to me. Thanks!

[francos]

Looks good to me too @jshier, thanks!! :)

[jshier]

2628 completes this issue.