Improve catch_response feature (was previous: Remove catch_response feature from HttpBrowser)

[heyman]

To me - even though I was in favor of it when we implemented it - the catch_response feature is slightly odd, and I'm not sure it should be a feature in Locust. A web app should not return 200 OK if the request in fact failed. I also think it would be a mistake to keep locust features that are focused toward detecting errors in the app that should have been spotted by the app's own unit/integration tests. Simply because it's better to focus on one thing (in our case load testing and user simulation) and kick ass at that :).

If someone is wandering I'm talking about the following syntax:

from locust import ResponseError

with self.client.get("/inbox", catch_response=True) as response:
    if response.data == "fail":
        raise ResponseError("Request failed")

Part of the reason I'm bringing this up is that I'm currently working on a branch where I'm replacing Locust's built in HttpBrowser with python requests lib (http://python-requests.org). I have now fully replaced the old client with requests, except for the catch_response feature, and not having to port that feature would save time and code lines :).

Does anyone use the catch_response feature extensively? Would anyone miss it :)?

[Jahaja]

Even though I agree with you in principle. I reckon it may be needed to support badly designed, or just old, web apps.

How much of a hassle is it to add support for this?

[Jahaja]

Forgot to add that it's nice to hear that you are working on using the requests lib! It will be a great improvement.

[heyman]

The HTTP client's get/post/put/etc methods are now actually returning python-requests' Response objects. This is nice since users get to work with a familiar API, but it makes the catch_response feature slightly complicated to implement, since python-request's Response objects aren't context managers (which is a requirement for the with-statement).

I've now come up with a solution which makes the feature work, but with a changed (and maybe slightly odd?) API:

# make a normal request
response = client.get("/page")
print response.status_code
print response.content

# make a request but do not immediately mark it as successful
with client.get("/page", catch_response=True) as catched:
    print catched.response.status_code
    print catched.response.content

[heyman]

Actually this got me thinking if maybe we could use the above solution to also solve the allow_http_error use cases.

If the API looked something like this:

# normal request
client.get("/page")

# equivalent of the above
with client.get("/page", catch_response=True): pass

# the "catch_response" use case
# this one will be reported as failure if the content is empty or if the response code is anything else than 2XX
with client.get("/page", catch_response=True) as catched:
    if catched.response.content = "":
        catched.failure("Empty content")

# the allow_http_error use case
# this one will only be reported as success if the response code was 404
with client.get("/page404", catch_response=True) as catched:
    if catched.response.status_code = 404:
        catched.success()
    elif catched.response.status_code = 200:
        catched.failure("Expected 404")

# and if someone want to mark a request as successful no matter what
# i.e equivalent to client.get("/page", allow_http_error=True)
with client.get("/page") as catched:
    catched.success()

But then again maybe it's weird to wrap the real response object in some other CatchedResponse object, but I do like the fact that it would also solve the use cases where one would want to use the allow_http_error argument.

What do you think?

[cgbystrom]

@heyman Do you monkey patch the Response object for this to work?
Should also add I dig requests a lot and catch_response is ugly. Hacks like that should belong in end-user test code if needed. Feels awkward that the Locust API has to deal with this.

While separate from this issue, it would be neater if Locust cared about being notified about the HTTP request you make (status code, size, response time etc) rather than handing over a ready-made client. Having Locust providing the client rather than the other way around gives certain limitations (such as only being able to test a single host at a time). It could also solve the above problem as well since it would put the end-user in control of the HTTP client, thus making any dirty monkey patch possible.

[heyman]

Nope, no monkey patching needed for the above solution. I agree that it's slightly ugly, but at the same time @Jahaja has a point that it can be useful when dealing with old/badly designed systems. Also, the above solution does also allow us to solve the use cases for the allow_http_error argument, which felt a little bit like a hack, in a more general way.

The client is now implemented as a thin wrapper around python-request's Session class (http://requests.readthedocs.org/en/latest/api/#requests.Session). It's now actually possible to make requests to different hosts, however the UI would need to be updated to reflect this as well (right now you wouldn't see that different requests went to different servers, unless you used the name attribute everywhere where you specified another host).

Also, locust actually do get notified about HTTP requests (status code, size, etc.) through the request_success and request_failure events, so it wouldn't be very hard to hack your own client that would still populate the statistics by firing those events. One thing we could improve to make this even easier, is to make the request_success and request_failure events not take a Response object as argument, but rather extract the data that is needed and pass that instead (I think it might actually only be the content-length header).

[heyman]

Fixed by #40