tfranzel / drf-spectacular

Sane and flexible OpenAPI 3 schema generation for Django REST framework.
https://drf-spectacular.readthedocs.io
BSD 3-Clause "New" or "Revised" License
2.39k stars 265 forks source link

Failure on SpectacularAPIView when unexpected mime type provided #41

Closed jayvdb closed 4 years ago

jayvdb commented 4 years ago

I am trying to get https://github.com/dhcode/openapi-ui working, but it uses Accept: application/json, application/yaml, which causes the following backtrace as a result, and it also displays only "untagged" but will be a separate issue.

[10:46:14][ERROR] django.request log.py:log_response:228 | Internal Server Error: /schema/
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 145, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 143, in _get_response
    response = response.render()
  File "/usr/local/lib/python3.6/site-packages/django/template/response.py", line 106, in render
    self.content = self.rendered_content
  File "/usr/local/lib/python3.6/site-packages/rest_framework/response.py", line 70, in rendered_content
    ret = renderer.render(self.data, accepted_media_type, context)
  File "/usr/local/src/drf-spectacular/drf_spectacular/renderers.py", line 14, in render
    return yaml.dump(data, default_flow_style=False, sort_keys=False, Dumper=Dumper).encode('utf-8')
  File "/usr/local/lib/python3.6/site-packages/yaml/__init__.py", line 290, in dump
    return dump_all([data], stream, Dumper=Dumper, **kwds)
  File "/usr/local/lib/python3.6/site-packages/yaml/__init__.py", line 278, in dump_all
    dumper.represent(data)
  File "/usr/local/lib/python3.6/site-packages/yaml/representer.py", line 27, in represent
    node = self.represent_data(data)
  File "/usr/local/lib/python3.6/site-packages/yaml/representer.py", line 48, in represent_data
    node = self.yaml_representers[data_types[0]](self, data)
  File "/usr/local/lib/python3.6/site-packages/yaml/representer.py", line 207, in represent_dict
    return self.represent_mapping('tag:yaml.org,2002:map', data)
  File "/usr/local/lib/python3.6/site-packages/yaml/representer.py", line 118, in represent_mapping
    node_value = self.represent_data(item_value)
  File "/usr/local/lib/python3.6/site-packages/yaml/representer.py", line 58, in represent_data
    node = self.yaml_representers[None](self, data)
  File "/usr/local/lib/python3.6/site-packages/yaml/representer.py", line 231, in represent_undefined
    raise RepresenterError("cannot represent an object", data)
yaml.representer.RepresenterError: ('cannot represent an object', ErrorDetail(string='Could not satisfy the request Accept header.', code='not_acceptable'))
[10:46:14][ERROR] django.server basehttp.py:log_message:154 | "GET /schema/ HTTP/1.1" 500 133266
tfranzel commented 4 years ago

i suppose the view could be a bit more forgiving regarding accepts. it should work though if you do not provide any accept or the vendor ones. i'll have a look.

on a different note, the stacktrace looks more like a non-serializable object snuk into the schema and the yaml encoder choked on it: raise RepresenterError("cannot represent an object", data)

tfranzel commented 4 years ago

improved accept handling: 266904c

i left out the error handling for now. i still believe your error is unrelated to the accept header. happened to me before, where there was a datetime object or something that could not be serialized. error looked similar.

jayvdb commented 4 years ago

Yes of course the stacktrace is an unserializable object. But it isnt data from the schema - it is an error because there is no handler for any of the Accept requested. That is explained fairly clearly here and in my PR https://github.com/tfranzel/drf-spectacular/pull/42 .

data is {'detail', ErrorDetail(string='Could not satisfy the request Accept header.', code='not_acceptable')}

I dont understand why this error is being given to the "first" renderer given that DRF already knows the renderer doesnt respond with an acceptable media type. It is possible that this type of data shouldnt be given to any renderer - maybe it can be blocked sooner.

Maybe the problem is that there is no registered error handler in DRF for yaml.

I can dig into it a bit more, and happy to build test cases, but if you know a better different solution then go for it. I dont want to be wasting my time working on something that you want to fix yourself.

tfranzel commented 4 years ago

still a bit puzzled how that can happen. i'll try to write a minimal test to get to the bottom of this.

tfranzel commented 4 years ago

so i had a look and it is an unfortunate situation. ideally the error would be in json and the schema in yaml each with correct media types.

which leaves us out of options to do it proper. all we can do is handle the error in yaml which i did with 5f4fa7d. this is acceptable i believe because this should be a very rare thing and the response is informative in any case.

jayvdb commented 4 years ago

in theory one could influence the response content type, but then it would not have the correct content type which is even worse.

I dont get this. Responding with content with the wrong content type is extremely bad. If the accept header was "openapi yaml", responding any other type of yaml is wrong. That is why there is a dedicated status code for this.

Another approach is to also respond with a Location: which could give more information.

btw, https://tools.ietf.org/html/rfc7807 may be of interest for this and other error problems. (and pypi drf-problems is also of interest but I havent tried it yet) I'm not sure of a yaml equivalent, but a vendor-ish yaml content type could be created for it.

tfranzel commented 4 years ago

I dont get this. Responding with content with the wrong content type is extremely bad.

that is why i said it is even worse.

is there a problem here? code supports vendor types, application/json and application/yaml

https://github.com/tfranzel/drf-spectacular/blob/97ce18e0a405b8a23d5171dba17e3963fdec61a4/tests/test_view.py#L64-L70

response = APIClient().get('/api/v1/schema/', HTTP_ACCEPT='application/unknown, application/json') 

responds with json as expected. "openapi yaml" is not an official MIME type. afaik DRF would normally bail there anyway.

jayvdb commented 4 years ago

Accept: application/vnd.oai.openapi should respond with only valid "openapi yaml". Official or not.

Accept: application/vnd.oai.openapi should respond with only valid "openapi json" - im not sure the error body in the response complies with this.

tfranzel commented 4 years ago

i see what you mean and i agree. i see no (good) way to influence the the accept error handling. all this handling happens in the framework. the renderer is basically just a glorified json.dumps(data).

i also tried hooking into content negotiations with no luck. available_renderers is ignored and again default is chosen.

class ContentNegotiation(DefaultContentNegotiation):
    """ fall back to JSON if negotiations fail """
    def select_renderer(self, request, renderers, format_suffix=None):
        try:
            super().select_renderer(request, renderers, format_suffix)
        except NotAcceptable as e:
            raise NotAcceptable(available_renderers=[JSONRenderer])

do you have a better solution except making application/yaml the first renderer? strictly speaking that would have its own problems as it does not officially exist. some say text/yaml, some say application/vnd.yaml and so forth.

feel free to provide a non-hacky, non-intrusive solution. sry, but i will not invest more time in this non-critical issue.