Moya: How to handle HTTP errors when using Alamofire automatic validation?

[mrtj]

I am using Moya 8.0.2 configured with OAuth2 authentication handling with Alamofire RequestRetrier as suggested in the docs, outlined in this Alamofire issue and implemented by p2/OAuth2.

To make it working in Moya, I implemented the validate getter in my Target returning true for the requests that need validation.

Everything works like a charm, the installed Alamofire RequestRetrier intercepts the 401 Unauthorized responses, fires the OAuth flow and then resumes the original request signing it. My problem is that this configuration, in particular that Moya issues the validate() request to Alamofire without the possibility of configuring it, shades the response body from the server for non-2xx status codes. For example when issuing a request to a non-existing resource, the automatic validation that I need for the authorization makes failing also this request, returning a underlying(Alamofire.AFError.responseValidationFailed(Alamofire.AFError.ResponseValidationFailureReason.unacceptableStatusCode(404))) error but I have no access to the original response from the server.

Are there any ways to selectively validate the Alamofire responses or otherwise make the non-401 HTTP errors succeed the request?

[BasThomas]

Maybe @scottrhoyt can help you with this one.

[scottrhoyt]

@mrtj unfortunately I don't have any easy answers for you on this one. Using Alamofire's validation takes a lot of control out of Moya's hands. The way I see it, you have a couple of options, but they aren't simple.

1. Modify Alamofire's validation error to also expose the response. This could be via filling an issue/PR with them or creating your own fork.
2. Modify Moya's TargetType.validate property to return an optional integer range to validate with. A nil value indicates no validation while a range is forwarded to Alamofire's validate(statusCode:) that would go around this section. This would require you to fork Moya.
3. Build request retrying functionality into Moya.

Option 1 would probably be valuable to the wider Alamorefire community, but I think it might be hard to get a breaking change like that through in any reasonable timeline for you. You would also have to maintain separate forks of both Alamofire and Moya.

Option 2 is the quickest and easiest for you, and you could submit a PR back to us for consideration. However, I don't personally love the solution for Moya in general because it's going to help propagate this pattern of spreading out error handling between Alamofire, Moya, and the dependent application. It feels like a bad separation of concerns to me, but other's may feel differently about that.

Option 3 would be the hardest but of possibly the most value to the Moya community. We go through some lengths to hide Alamofire from end users. If we can eliminate one more use case--and a big one at that--for users to dig into Alamofire and deal with the attendant consequences, I think that would be a big win.

[mrtj]

@scottrhoyt thank you for the suggestions. Based on your suggestion in (2.) I actually debugged how Moya processes the response and found that the response data actually arrives from Alamofire to the level of Moya and it is Moya that shades the data. More exactly the second case in the switch in convertResponseToResult simple ignores the data in Alamofire response before converting it to an error result. Could this be the place where we could add the data to the result?

[scottrhoyt]

I'm sorry @mrtj, I mistakenly assumed that at that point Alamofire was just returning a AFError. responseValidationFailed(reason: . unacceptableStatusCode(Int)). If it is also returning a response with data then indeed you could modify MoyaError.underlyingError to have an optional Response parameter that can be forwarded to custom error handling.

Why don't you give that a try and report back your findings? We can open up a PR as well, but since this would be a breaking change, it might be a bit before we can include it upstream.

[BasThomas]

FWIW, I think in this case it should definitely be corrected to also include data in Moya, and we should consider releasing a 9.0 for it. What would be the downsides of doing it this quick after 8.0?

[scottrhoyt]

I don't see there being a ton @BasThomas. If this were a bigger breaking change I would say the downside would be forcing user's to deal with breaking changes to receive future fixes, but in this case the fix would be pretty minor and maybe not even affect a large portion of users. If we go that route I would propose we start a pre-release cycle for 9.0 so that @mrtj and others with this use case can get support quickly while we work out what other things--if any--should go in 9.0. How about we open another issue to discuss?

[BasThomas]

👍 great idea

[gperdomor]

Conclusion, in 9.0 will be possible get the response from the error? I need that :D

[scottrhoyt]

@mrtj @BasThomas, I just created the 9.0.0-dev branch so we can accept PRs there. @mrtj, if you have gotten the solution above to work, would you mind submitting a PR to that branch so we can get your work included in the next major release of Moya? 👍

[gperdomor]

Solved in #993 so this can be closed