Verbose misses response body

avkonst commented 3 years ago

I have got the following test spec:

tests:
-   name: auth
    verbose: all
    url: /api/sessions
    method: POST
    data: "asdsad"
    status: 200

which has got data not a proper json on purpose. The response results in 400 instead of 200 code. Verbose is set to all, but it still does not print the response body, although detects non empty content:

... #### auth ####
> POST http://localhost:7000/api/sessions
> user-agent: gabbi/1.40.0 (Python urllib3)

< 400 Bad Request
< content-length: 48
< date: Wed, 19 Aug 2020 06:11:24 GMT

✗ gabbi-runner.input_auth

FAIL: gabbi-runner.input_auth
        Traceback (most recent call last):
          File "/usr/lib/python3/dist-packages/gabbi/case.py", line 94, in wrapper
            func(self)
          File "/usr/lib/python3/dist-packages/gabbi/case.py", line 143, in test_request
            self._run_test()
          File "/usr/lib/python3/dist-packages/gabbi/case.py", line 550, in _run_test
            self._assert_response()
          File "/usr/lib/python3/dist-packages/gabbi/case.py", line 188, in _assert_response
            self._test_status(self.test_data['status'], self.response['status'])
          File "/usr/lib/python3/dist-packages/gabbi/case.py", line 591, in _test_status
            self.assert_in_or_print_output(observed_status, statii)
          File "/usr/lib/python3/dist-packages/gabbi/case.py", line 654, in assert_in_or_print_output
            self.assertIn(expected, iterable)
          File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 417, in assertIn
            self.assertThat(haystack, Contains(needle), message)
          File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 498, in assertThat
            raise mismatch_error
        testtools.matchers._impl.MismatchError: '400' not in ['200']
----------------------------------------------------------------------

The expected behavior:

response body is printed to the stdout alongside the response headers.

cdent commented 3 years ago

You're using a version of gabbi that is about 2.5 years old. If you're able to upgrade and try the same test that would be helpful in trying to trace where the problem is. I'm pretty sure the version won't make a difference, but I'd like to remove that variable.

Note that if no content-type is provided, or a content-type for which gabbi does not have a ContentHandler , then gabbi will not send a body. You can see that in your paste above because the request body is empty. If you need to be able to send other content-types there are ways to subclass ContentHandler (see: https://gabbi.readthedocs.io/en/latest/handlers.html )

Also could you tell me more about the server you are making your requests against? I've tried against a few different servers (both python and go-based, using both modern gabbi and the version you're running) and I'm unable to replicate the problem you are seeing.

One thing that would be worth trying is replicating the request using curl in verbose mode to show the output gabbi should be showing. That will help to narrow things down. I think the curl would look something like this (including empty body based on no ContentHandler):

curl -v http://localhost:7000/api/sessions -X POST

I hope we can work this out, but I need a bit more info to make any progress.

avkonst commented 3 years ago

I can not reproduce it with the latest version. PS: I was not sure how to install gabbi-run. I have got ubuntu 20.04 which is recent. I run the gabbi-run command and it suggested 'apt install gabbi-run'. So here I got what I got. Would it be better to update the ubuntu/debian repo with the latest version?

cdent commented 3 years ago

Would it be better to update the ubuntu/debian repo with the latest version?

That's something that's beyond my control. Which version the various linux distros choose to package is up to them and not something I'm involved with. If you'd like to see them be more up to date, opening a bug with them may help.

The linux distros being out of date with regard to Python packages is a fairly common problem, big enough that people tend to install the Python packages they use regularly via pip and not use the version packaged by the distro.

avkonst commented 3 years ago

no worries. thanks

avkonst commented 3 years ago

Sorry, I was wrong. I can reproduce the issue with the latest. As far as I can see it does not print the response body if a server does not set Content-type header.

avkonst commented 3 years ago

It also prints the following error if response content-type is "application/json" but the response body is just plain text:

< 401 Unauthorized
< content-length: 63
< content-type: application/json
< date: Wed, 19 Aug 2020 23:32:07 GMT
E gabbi-runner.input_auth

ERROR: gabbi-runner.input_auth
        Expecting value: line 1 column 1 (char 0)
----------------------------------------------------------------------
Ran 2 tests in 0.006s

avkonst commented 3 years ago

I think it should apply the default content-type if the response does not contain it. It should be UTF-8 string by default, as far as I remember.

cdent commented 3 years ago

Thanks, those are good clues as to what's going wrong.

I think you're right: If it can't figure out what to show it should try to dump a string. I'll look into what it will take to do that. Will try to get to it tomorrow, but it may be a few days.

cdent commented 3 years ago

I've started experimenting with this, and there's two cases that we need to decide how to get the right behavior:

When a content-type is set on the response, and it it is determined to be "binary" then the verbose routines very intentionally to do print the response body, as it is assumed that it will not be readable and will likely be long. I'd prefer that this remain the case. I am able to make so that if no content-type header is set on the response then the body will be printed. Good enough?
The "Expecting value..." error can happens as part of the response decoding when content-type is application/json but the body is not valid JSON. This needs to remain true. What this means is that if the response body is "foo" and verbose is set to all then the changes being implemented will allow that "foo" to be printed in the verbose output but the "Excepting value" error will still happen, because gabbi will still try to decode the response it thinks is JSON. Okay?

avkonst commented 3 years ago

I think about this way (likely similar to yours view):

Verbose printing of a body (by request via a config or command line option) and validation of the body content (as per the test spec) are 2 different things.
Sure, if the test spec expects json and coming body can not be deserialised as json, than it is a test failure. Keep in mind that many servers respond with json without setting the content type at all - and it is perfectly valid, especially for servers with very narrow bandwidth deployment, where extra headers are removed on purpose.
Verbose printing of a body should follow the same principles for requests and responses
Verbose printing of binary data can be omitted (better with a message saying it was omitted) or can be printed as hexbytes text (maybe add a configuration option for this to tune the desired behavior)
Verbose printing of long responses (binary or text) can / should be truncated (maybe add a configuration option for the max length)
Verbose printing should print the body regardless if the content type is set, if it is set correctly, if the body valid or invalid structure or if it corresponds to the content type declaration
Verbose printing should not alter the response body, eg. if a json response is not pretty-printed, it is better to keep it as it was in the response, but a config option could change this behaviour to activate pretty printing of json/yml/etc...

This is how I think about it. Pick what you think is useful.

cdent commented 3 years ago

Thanks for the comments. For the time being I've gone with a relatively simple fix in #285 which will be released as version 2.0.4 here in a few minutes. As things move along it might make sense to incorporate some of your other ideas.

One thing I don't want to do is automatically decode or detect content-type without a content-type header. Gabbi has always been opinionated about using content-type effectively and encouraging people to write APIs that do so. As gabbi has grown to sometimes be used to test things which the tester has no control over, his has proven a bit more difficult, so it might make sense in the future to change this, but I don't want to make that change without due consideration.

cdent / gabbi

Verbose misses response body #282