Moya: Stubbed Responses Don't Call Alamofire Validation

Created on 31 Mar 2020  路  7Comments  路  Source: Moya/Moya

Using Moya 14, I'm attempting to stub responses with a 401 and some body data. That part works fine. However, I'm also attempting to use an Alamofire EventMonitor to trigger actions based on Alamofire events and it appears Moya breaks events around validation. For example:

// MyAPI is a MoyaProvider subclass.
MyAPI(endpointClosure: MyAPI.authorizationFailure,
      stubClosure: MyAPI.testStub) // .testStub just returns .immediate

// MyAPI internally uses a custom Alamofire Session with the following EventMonitor to send Notifications.

final class Notifier: EventMonitor {
    private let notifications: NotificationCenter
    init(notificationCenter: NotificationCenter = .default) {
        notifications = notificationCenter
    }

    func request(_ request: DataRequest,
                 didValidateRequest urlRequest: URLRequest?,
                 response: HTTPURLResponse,
                 data: Data?,
                 withResult result: Request.ValidationResult) {
        guard response.statusCode == 401 else { return }

        notifications.post(Notification(name: .notification))
    }
}

This event is never issued, as stubbed responses don't use Alamofire's validation logic, despite what #1593 intended. This is especially confusing as, without stubbing, the validation error returned is Alamofire's, whereas with stubbing, the error returned is Moya's. This makes it very difficult to have a common code path between stubbed and unstubbed requests.

Ideally Moya would never interfere with expected Alamofire events. As EventMonitor grows in capability this will become more and more important. In the meantime, I think this is a bug worth addressing if you agree.

bug?

Most helpful comment

Removing the stale label on this. I briefly read through the thread but can't offer any input here as of now. I don't think this is something we want closed automatically though

All 7 comments

I've created this compact coalescing of errors to find the code, but I'd rather not:

guard let code = ((error as? MoyaError)?.response?.statusCode) ??
                 (error.asAFError?.responseCode),
                 code == 401 else { return }

Hello @jshier :)

Moya's stubbing completely bypass Alamofire, either to build the request or to handle the response. I believe #1593 never intended to use Alamofire's validation, but only to provide "a" way to validate response's status code.
As Alamofire is bypassed, it easily explains why the EventMonitor is not triggered when you validate the stubbed response.

I will have to go deeper into Alamofire to get to know how we could create an Alamofire's request, set a stubbed response into it and use it as if the request was actually sent. But provided you raised the issue I suppose there is a way to do that.
A question I ask to @Moya/contributors : if we update Moya like that, should we provide users the ability to still use the "legacy" behavior and completely bypass Alamofire ?

If the intention of stubbing is to bypass Alamofire rather than just the network, I suppose that makes sense. (I hadn鈥檛 investigated how it works.) Such a boundary does make using Alamofire features more awkward, which is unfortunate for any feature that has no counterpart in Moya.

Without stubbing in Alamofire directly, URLProtocol is the only way to skip the network. But it鈥檚 certainly something we鈥檝e been thinking about.

Hey :)

A question I ask to @Moya/contributors : if we update Moya like that, should we provide users the ability to still use the "legacy" behavior and completely bypass Alamofire ?

@amaurydavid As far as I understand I don't see maintaining the "legacy" behavior as a bad thing, but I also think that it could be removed without causing many problems from the user. As @jshier mentioned most of the cases they probably already have a code that maps Alamofire error -> Moya Error for the non-stub code ... but I'm +1 in allowing both just to let the user choose.

I will have to go deeper into Alamofire to get to know how we could create an Alamofire's request, set a stubbed response into it and use it as if the request was actually sent. But provided you raised the issue I suppose there is a way to do that.

My main question would be more in that same direction, how to implement stubbing not in the Moya side but in Alamofire, which is what as far as I understand should be done to make this work in the way we should always call Amofire, but when stubbing, make Alamofire return the stubbed response right?

cc @sunshinejr

I've given this a bit of thought over the last few hours and I've come up with a few goals for such a feature in Alamofire. These are just raw thoughts, a real feature would need to be refined on our GitHub.

Alamofire stubs should be:

  • Invisible to the user when they aren't stubbing.
  • Invisible to Alamofire, except at a point connecting a particular Request to a particular stub.
  • Injectable
  • Mapped for particular Requests, like Moya's, rather than global stubs, like other stubbing libraries.
  • Powerful enough to replace Moya's.

To those ends, there seem to be a few implementation details that need to be met:

  • Must be URLProtocol based, so as to realistically replicate the network environment.
  • At least initially, a closure-based approach on Session seems feasible, with the right convenience API.
  • A per-Request API might be possible, though I'm wary of continually overloading those top level methods.
  • Should allow for Data, String, and Encodable mocking.
  • Should have convenience API building HTTPURLResponses and such.
  • Should have errors handling around unstubbed requests, or perhaps make it impossible to not stub requests.

Those are just some thoughts, so let me know what you think. This likely won't happen "soon", as we've got major features planned for Alamofire 5.2 (Combine) and 5.3 (URLSessionWebSocketTask) already, and 5.1 should ship this weekend (DataStreamRequest).

This issue has been marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Removing the stale label on this. I briefly read through the thread but can't offer any input here as of now. I don't think this is something we want closed automatically though

Was this page helpful?
0 / 5 - 0 ratings

Related issues

JianweiWangs picture JianweiWangs  路  3Comments

cocoatoucher picture cocoatoucher  路  3Comments

PlutusCat picture PlutusCat  路  3Comments

kamwysoc picture kamwysoc  路  3Comments

fenixsolorzano picture fenixsolorzano  路  3Comments