swagger_codegen icon indicating copy to clipboard operation
swagger_codegen copied to clipboard

KeyError on expected headers

Open coderfromhere opened this issue 5 years ago • 3 comments

Hi!

I have a test case of a server-side validation error where the server responds with the most minimal set of headers, like:

{'Transfer-Encoding': 'chunked', 'Date': 'Mon, 24 Aug 2020 13:21:06 GMT', 'Server': 'Warp/3.3.13'}

The body of a validation error message may be just a bytestring of:

b'Error in $.userId: invalid UUID'

Here's how the currently autogenerated client fails with these responses:

test_service.py::test_service FAILED                                     [100%]
tests/myapp_with_client/test_service.py:5 (test_service)
tests/myapp_with_client/test_service.py:14: in test_service
    problem=Problem("{}"),
src/my_service/myapp_with_client/service.py:83: in map
    userId=user_id,
.venv/lib/python3.8/site-packages/autogenerated_client/apis/api/post__link_problem.py:57: in make_request
    }, m)
.venv/lib/python3.8/site-packages/autogenerated_client/lib/base.py:41: in make_request
    result = self._client.call_api(api_request)
.venv/lib/python3.8/site-packages/autogenerated_client/lib/client.py:21: in call_api
    return self._adapter.call(method)
.venv/lib/python3.8/site-packages/autogenerated_client/lib/adapter/requests.py:35: in call
    return methods[api_request.method](api_request=api_request)
.venv/lib/python3.8/site-packages/autogenerated_client/lib/adapter/requests.py:60: in _write
    return self._response(make_request(**params))
.venv/lib/python3.8/site-packages/autogenerated_client/lib/adapter/requests.py:73: in _response
    content_type=response.headers["Content-Type"],
.venv/lib/python3.8/site-packages/requests/structures.py:54: in __getitem__
    return self._store[key.lower()][1]
E   KeyError: 'content-type'

While it's a reasonable solution in a dynamic language, I think it would be beneficial to not to hide the actual bytestring body of the error response, or probably handle missing headers more gracefully. I acknowledge there's not much reason for the JSON api to do so, as it won't be able to recover from a text response anyways, but I think it can help to reveal the content of the response body in a traceback. The content-type header in this particular case could be explicitly set to text/plain, as it's the protocol default (at least, according to https://www.w3.org/Protocols/rfc1341/4_Content-Type.html).

coderfromhere avatar Aug 24 '20 13:08 coderfromhere

Hi!

The W3C Protocols page states:

Now that both HTTP extensions and HTTP/1.1 are stable specifications (RFC2616 at that time), W3C has closed the HTTP Activity.

So I assume that RFC2616 takes preference over RFC1341.

RFC2616 states that default media type should be application/octet-stream

Any HTTP/1.1 message containing an entity-body SHOULD include a Content-Type header field defining the media type of that body. If and only if the media type is not given by a Content-Type field, the recipient MAY attempt to guess the media type via inspection of its content and/or the name extension(s) of the URI used to identify the resource. If the media type remains unknown, the recipient SHOULD treat it as type "application/octet-stream".

What do you think?

asyncee avatar Aug 25 '20 06:08 asyncee

Also, aiohttp client uses RFC2616 to set default content type if it is not specified so i think it's best to provide same behaviour between clients.

asyncee avatar Aug 25 '20 06:08 asyncee

Sure, my only goal regarding this issue is to make tracebacks clearer than just KeyError, the actual implementation of a safer alternative to content_type=response.headers["Content-Type"] can be borrowed from either spec/library.

coderfromhere avatar Aug 25 '20 14:08 coderfromhere