Alamofire: Request interceptor/adapter call order

[hsousa]

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

What did you do?

• Created session and request interceptors (see snippet below) and used each one when creating the session and request, respectively.
• Called the request

Example:

class MySessionInterceptor: Alamofire.RequestInterceptor
{
    func adapt(_ urlRequest: URLRequest, for session: Session, completion: @escaping (Result<URLRequest, Error>) -> Void) {
        var urlRequest = urlRequest
        urlRequest.setValue("defaultValue", forHTTPHeaderField: "MyCustomHeader")
        completion(.success(urlRequest))
    }
}

class MyRequestInterceptor: Alamofire.RequestInterceptor
{
    func adapt(_ urlRequest: URLRequest, for session: Session, completion: @escaping (Result<URLRequest, Error>) -> Void) {
        var urlRequest = urlRequest
        urlRequest.setValue("newValue", forHTTPHeaderField: "MyCustomHeader")
        completion(.success(urlRequest))
    }
}

What did you expect to happen?

I'd expect the request interceptor to be able to override values set by the session one. I believe the use case is valid, where your app might always send a certain value, but on specific requests you might want to tweak that instead (or modify the request in any other way).

What happened instead?

The request never got modified, because (and the documentation is helpful+clear here) the session interceptor's adapt() method is only called after the request one.

Alamofire Environment

Alamofire version: 5.0.0
Xcode version: 11.3.1
Swift version: 5.1
Platform(s) running Alamofire: macOS
macOS version running Xcode: 10.15.3

Demo Project

I haven't extracted this into a demo project as my issue is more to spark a discussion than to report a bug, since as I stated above the behaviour is actually following the documentation... but happy to do so if needed.

Further Discussion

My sincere apologies if my question makes no sense and/or if there's a very clear reason I'm missing to have these ordered like they are right now. I can certainly find a workaround on my end to achieve this goal, but it seemed to me that it warranted a proper discussion.

Happy to investigate and file a PR with the change if the team agrees with it.

[jshier]

You can find the relevant code here:

func adapter(for request: Request) -> RequestAdapter? {
    if let requestInterceptor = request.interceptor, let sessionInterceptor = interceptor {
        return Interceptor(adapters: [requestInterceptor, sessionInterceptor])
    } else {
        return request.interceptor ?? interceptor
    }
}

func retrier(for request: Request) -> RequestRetrier? {
    if let requestInterceptor = request.interceptor, let sessionInterceptor = interceptor {
        return Interceptor(retriers: [requestInterceptor, sessionInterceptor])
    } else {
        return request.interceptor ?? interceptor
    }
}

As you can see, if a Request has both a Session and Request adapter or retrier, both are used, Request instance first. If one or the other is missing a value, it coalesces in the same order in an attempt to find an instance. This allows the pattern your examples seem to want. However, some care is required in order to prevent the Session instances from overwriting the Request ones, such as checking if a header value has already been set.

If you aren't seeing the behavior, or you aren't seeing your adapters or retriers called at all, there may be a bug. This is where a demo project can help. Feel free to add a link, or the code you're using to make the request, and we can see if there's an issue.

[hsousa]

Hi @jshier! Thanks for getting back to me so quickly!

If you aren't seeing the behavior, or you aren't seeing your adapters or retriers called at all, there may be a bug.

I'm definitely seeing that behaviour. As far as I can tell Alamofire is working as intended, which is what prompted me to spark a discussion here.

However, some care is required in order to prevent the Session instances from overwriting the Request ones, such as checking if a header value has already been set.

What if you need to customize a request to _remove_ a header? With the current logic it's impossible to do so because when the Session interceptor fires it has no way of knowing that a header it normally injects needs to be absent for that specific request.

For example, you might always send some data as a header, but on a specific request you'd like to send that same data as a query parameter instead.

So my question is: am I wrong thinking that a Request interceptor should be able to customize a request after the Session interceptor did whatever it needs to do? (i.e. return Interceptor(adapters: [sessionInterceptor, requestInterceptor]))

Or was there a reason behind having it the other way around? What's the advantage?

[jshier]

You can always inspect the incoming URLRequest to see wether it should be adapted, but in general you're always going to have this issue when you only use one adapter for either side. Either the Session interceptor will have to do some work to only adapt when needed, or the Request interceptor will. My suggestion would be to take advantage of the per-request interceptors and only add particular interceptors to the request that need them, letting the Session interceptor only handle things that are necessary for every request. I usually do this at my request API layer where I know certain requests require authentication or other adaptation. This way the interceptors are more composed together rather than layered on top of each other.

[cnoon]

Great discussion, and I'm with @jshier. Don't put anything in the Session interceptor that wouldn't be set on all requests. It's just like the URLSessionConfiguration headers. Only add what you want on every request. Don't put a header you may want to remove for a request into the Session interceptor.

Then, I'd suggest you take advantage of making separate interceptors (or at least adapters) for splitting up your header add/remove logic. Any request can be "adapted" any number of times at the Request level, and you control the order that they're executed. This should give you everything you need to design any add/remove header logic you would want.

Best of luck! 🍻

[hsousa]

First of all, thanks to you both for taking the time to address my question! ❤️

Then, I'd suggest you take advantage of making separate interceptors (or at least adapters) for splitting up your header add/remove logic. Any request can be "adapted" any number of time at the Request level, and you control the order that they're executed.

You mean more than one interceptor/adapter per request?

If so, is there a way in the API to do this that I'm missing? All I saw was a parameter to pass a single interceptor.

The more I think about it though, the more I agree with you in that I should try to compose instead of layer the interceptors. If there's indeed a simple and clean way to apply multiple interceptors to a given request, I'm sold! 👍

[jshier]

Yes, our Interceptor type can be used to compose multiple interceptors.

[hsousa]

Ah, how did I miss that! 🤪

Thank you so much. Closing this now and hiding in shame 😂 ❤️

[jshier]

No problem, always good to know where we might need to improve our documentation.

[cnoon]

Hopefully this issue will be seen by many that run into a similar challenge.

[ileitch]

Yes, our Interceptor type can be used to compose multiple interceptors.

I think this is worth mentioning in the advanced usage doc and also the migration guide. I wish I'd known about this earlier, now I need to undo my work where I squashed my retryer & adapter into a single class 🙃