socketry / async-http

MIT License
320 stars 46 forks source link

Better support for WebMock #35

Open ioquatix opened 5 years ago

ioquatix commented 5 years ago

Regarding https://github.com/socketry/async-http/issues/34 and https://github.com/bblimke/webmock/issues/858

It would be great to know how to improve support for WebMock.

Potentially, we could integrate most of the mocking behaviour into async-http and provide some hooks for replacing this in user code. I'm probably not keen to replace constants e.g. Async::HTTP::Client but we could at least provide some new middleware like Async::HTTP::MockClient or something to that end, which can be used to implement mocked requests/responses.

I'm not sure that recording and replaying is within the scope of this gem or not, but we could at least avoid forcing the WebMock gem to reimplement internal implementation details.

bblimke commented 4 years ago

@ioquatix if that's possible, it sounds like a great idea.

ioquatix commented 4 years ago

I thought about this a bit more.

It's super easy to spin up a local server that behaves exactly like any other server (in theory).

One option would be, rather than mocking up the client side, mock up the server side and redirect requests using a client-side proxy designed for mocks.

That way, even thought the requests and responses are mocked, from the client POV, the interface is exactly the same as in production (including streaming responses).

In theory, this could even handle things like WebSockets, if done at the right level.

The way this would work naively would be something like this:

@bblimke what do you think?

bblimke commented 4 years ago

@ioquatix thank you for spending time on this!

There was a project which tried doing something similar https://github.com/rdsubhas/webmock-server

I believe the goal behind the proxy idea is to prevent monkey-patching the http client code. I'm now concerned where that "client-side proxy designed for mocks." would sit. I guess it would not be a proxy at OS level since the behaviour has to be limited only to running code where WebMock with given setup has been loaded and not be global. In that case I think that intercepting the requests at client lib level is the right place. The question is whether each http client lib does allow adding that kind of proxy.

Please also note that WebMock is not only used for returning stubbed responses but also setting expectations on real requests.

ioquatix commented 4 years ago

There are pros and cons of both approaches. The server repo link you give above has a great outline as I'm sure you are aware.

In theory it should be trivial to avoid the need for monkey patching, provided the user can replace the use of Async::HTTP::Client instances, e.g.

client = Async::HTTP::Client.new
mock_client = Async::HTTP::MockClient.new(client)

mock_client is responsible for proxying or mocking requests/responses. However, doing this generally across a large code base requires quite a bit of coordination and is impossible outside of the Ruby sandbox (e.g. external tools).

One option would be to add a method like: Async::HTTP::Client.default which is an alias for Async::HTTP::Client.new - but with a monkey patch could become Async::HTTP::MockClient.new. It won't work for sub-classes.

Taking it a step further, mocking the underlying connection might make more sense. By providing a different connection pool/protocol implementation, some of these issues can be avoided. Rather than being HTTP/1 or HTTP/2 it would be some kind of MOCK protocol. This is the lowest layer before hitting the network, so perhaps the ideal place to do the work.

ioquatix commented 4 years ago

Multiplexing this using a different URL scheme would be very simple to implement... one way to do this explicitly would be to have something like:

mock:http://host/path

or

mock+http://...
bblimke commented 4 years ago

One thing to consider is that WebMock needs to cover all the popular ruby http clients, therefore we would need to make sure that the same is possible across all other adapters, including Net::HTTP or Curb.

I absolutely agree that the current approach has lots of flaws. Streaming, handling large responses, multi-part responses. I foresee that some cases, like multi-part will still be tricky, even if a real http server is used.

There will most likely be new challenges. E.g. how to manage the registered stubs on the server.

WebMock also allows setting expectations on requests, the stubbed requests or real requests, though I'm not sure how popular setting expectations on real requests is, I don't take advantage of it personally.

Perhaps it's worth giving it a try and creating a branch to build a prototype, just for Async::HTTP and see what are the challenges.

ioquatix commented 4 years ago

A server side solution should be client agnostic, which should be a nice property to have and minimise the per-client effort currently required. But yes, it's a different approach, and provides a different set of features, even if there is a lot of overlap.

bblimke commented 4 years ago

A server side solution should be client agnostic

Do you know how to achieve that? How to intercept the request and proxy it to appropriate local server and at the same time make sure it's only in scope of the test suite and not something global, configured at OS level?

ioquatix commented 4 years ago

Do you know how to achieve that?

Just thinking out loud. If it's in a test harness, you should be able to change the hostnames, and there seem like various ways to achieve that. One "simple" one would be to use LD_PRELOAD to replace the DNS resolver... or you could just do it in pure Ruby by replacing the resolver. Depending on your approach, you could rewrite the DNS for any process and/or child process.

ioquatix commented 4 years ago

Probably a better place to do this is simply in the test harness itself, e.g. if running tests, use this other domain name for the test API. In practice that probably propagates much better, e.g. to headless chrome, etc.

bblimke commented 4 years ago

I assume this is something that would only work for URI's with hostname and not for URIs with IP address?

Could you please point me to some docs or example on how resolver can be replaced in Ruby?

E.g in case of Curb, all DNS resolving happens in underlying Curl bindings where we don't have access.

In practice that probably propagates much better, e.g. to headless chrome, etc.

I don't think I follow here. How would headless chrome be of any use here?

The reason why WebMock has been build the way it was built was that I couldn't find any better way to intercept the requests than by intercepting them at http client level. If there is indeed a way to do that, and make sure that all the setup is local to a single ruby test process, it would be awesome.

ioquatix commented 4 years ago

This is an example of using the PRELOAD trick to replace DNS resolution:

Here is a specific example for unit testing:

I'm sure there are others.

I don't think I follow here. How would headless chrome be of any use here?

The point is, if you only replace DNS in Ruby, external tools e.g. headless chrome running acceptance tests, might get the wrong DNS.

So, we'd need to make sure it was launched with the same resolve hack. The only issue would be HTTPS certificates, maybe hard to avoid that one.

ioquatix commented 4 years ago

https://github.com/bblimke/webmock/commit/e3a7d9014fb4b31df8dac9bc4f0d32f33ef3967b just linking to other discussion regarding the original implementation.

tleish commented 4 years ago

FYI, I'm interested in this. I originally built an application with falcon and async-http. However, after hitting roadblock with mocking client requests I ended up switching to goliath and em-synchrony/em-http. I plan to try falcon/async-http again once resolved.

ioquatix commented 4 years ago

@tleish can you explain your problem in more detail?

tleish commented 4 years ago

I have a server where some API endpoints send outbound API requests. After writing the code, I want to be able to have automated tests which calls the API endpoints that processes the 3rd party API response without actually calling the 3rd party API. WebMock is a great solution for testing (and placing boundaries) around 3rd outbound http client testing.

Previously I was unable to successfully mock the API responses from 3rd party sites while testing falcon with async-http.

bblimke commented 4 years ago

@tleish I understand what you want to achieve but I don't understand the issue that you had with async-http and WebMock. You stated that

However, after hitting roadblock with mocking client requests

Can you please explain what was this roadblock? You do know that WebMock supports async-http already, right?

ioquatix commented 4 years ago

I think the problem is that the server itself needed to mock the requests, not the specs. I guess the specs were starting a server, and that server was making requests, and for whatever reason, it wasn't possible to mock those requests. But I don't know why, we probably need. small repro.

tleish commented 4 years ago

@bblimke - I was going to report this issue last November, but found the issue was reported here:

https://github.com/bblimke/webmock/issues/858