-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Returning different types of 400 exceptions #101
Comments
Hey! absolutely! depending on what you want to achieve there are multiple ways.
beware that i just merged the "pro-mode" PolymorphicProxySerializer #91, thus the "dict" part is not in the pypi release yet), but it is recommended to use the "list" version anyway. 😄 |
As far as I understand though, DRF's APIExcpetion power lies in the fact that a Serializer is not necessary after it is raised in the stack. This is the reason that I was thinking an APIException (or list of them as a number of them can occur) would be beneficial to pass into the responses argument of Just to give you an idea of what we have right now in our manually generated openapi yaml, is the following:
All three of these (40X, 500) responses plus a 200 response would be displayed on one endpoint. I hope Im making my question clearer 😉 |
@andrewgy8 ahh now i understand what you are getting at here. that is a good point. we more or less ignored that that part of DRF until now. indeed, my proposed options do not really fit well here. in theory, every endpoint could raise those errors. however listing all of them for every endpoint would be a bit spammy, so listing them explicitly with this needs some investigating on how to do error handling in a flexible and generic way.
|
Exactly. Id suggest as a first iteration placing it in the
AFAIK, out of the box DRF exception responses contain a standard structure. However there will be complexity involved when the application has a custom exception handler. This can indeed manipulate the standard response structure. I was thinking in that case we could use the custom exception handler somehow to "inspect" how the final format will look like.
I assume always |
Notes from #119 , there are many different responses from DRF core https://github.com/ivlevdenis/drf_pretty_exception_handler#features has a nice summary of the four main quite varied response payloads that default DRF emits as 400 responses. The default exception subclasses also emit 500, 403, 401, 404, 405, 406, 415, and 429 In addition, Http404 is caught and emits a 404 response. This would result in a huge increase in the schema if those codes are each described for each endpoint, so I proposed using the responses:
...
default:
insert_ref_here_to_a_component_broadly_describing_of_all_drf_possible_responses_merged_with_AnyOf And IMO the extension framework should be used, so that the setting |
Document the general error message structure in our generated OpenAPI schema. This uses a technique suggested in <tfranzel/drf-spectacular#119> to augment the schema. In the future an implementation for <tfranzel/drf-spectacular#101> may make this code obsolete. Bug: T268774 Change-Id: I77c3afffc90213d44f394e17314b28938136a325
This is how I implemented it:
If atleast one 4xx response is defined on an endpoint (using Only catch here is it overrides private method from rest_framework import serializers
from drf_spectacular.openapi import AutoSchema
class DummySerializer(serializers.Serializer):
def to_internal_value(self, data):
return data
def to_representation(self, instance):
return instance
def update(self, instance, validated_data):
pass
def create(self, validated_data):
pass
class ValidationErrorSerializer(DummySerializer):
errors = serializers.DictField(child=serializers.ListSerializer(child=serializers.CharField()))
non_field_errors = serializers.ListSerializer(child=serializers.CharField())
class GenericErrorSerializer(DummySerializer):
detail = serializers.CharField()
class UnauthenticatedErrorSerializer(GenericErrorSerializer):
pass
class ForbiddenErrorSerializer(GenericErrorSerializer):
pass
class NotFoundErrorSerializer(GenericErrorSerializer):
pass
class MyAutoSchema(AutoSchema):
def _get_response_bodies(self):
response_bodies = super()._get_response_bodies()
if len(list(filter(lambda _:_.startswith('4'), response_bodies.keys()))):
return response_bodies
add_error_codes = []
if not self.method == 'GET':
add_error_codes.append('400')
if self.get_auth():
add_error_codes.append('401')
add_error_codes.append('403')
if not (self.method == 'GET' and self._is_list_view()):
if len(list(filter(lambda _: _['in'] == 'path', self._get_parameters()))):
add_error_codes.append('404')
self.error_response_bodies = {
'400': self._get_response_for_code(ValidationErrorSerializer, '400'),
'401': self._get_response_for_code(UnauthenticatedErrorSerializer, '401'),
'403': self._get_response_for_code(ForbiddenErrorSerializer, '403'),
'404': self._get_response_for_code(NotFoundErrorSerializer, '404')
}
for code in add_error_codes:
response_bodies[code] = self.error_response_bodies[code]
return response_bodies @tfranzel any better way to handle this (without overriding private method) |
@tiholic pretty nice! don't get discouraged from overriding private methods. the distinction is a bit fuzzy. public methods are "mainly" the methods that get overridden by I have not implemented this because by default there is only the generic and very dynamic error handler and at the end of the day everybody has slightly different requirements. there is likely no "one size fits all". I may look at this again when I have spare time but don't get your hopes up. this particular issue is pretty low prio at the moment. |
@tfranzel I created drf-standardized-errors which is a custom exception handler that returns the same error response format for 4XX and 5XX status codes. The package integrates with drf-spectatcular to automatically generate the error responses schema. By creating a custom exception handler, I avoided the issues linked to "the generic and very dynamic error handler". Now, I'm interested in implementing the automatic error response generation directly in drf-spectacular. That would be useful to anyone happy with drf error format and is not interested in standardizing their error responses. So, I wanted to check with you first if it is still a good idea to do that as part of drf-spectacular, and if so, any advice about the implementation and potential problems to keep in mind. |
I've been looking into documenting my responses properly and came across this problem. In general, how should one document the same HTTP status code with different response schemas and descriptions? Without any custom responses, only within DRF we have ValidationError (400) and Parse Error (400) with different schemas and descriptions. Every endpoint that takes data and is working with a serializer should probably define those two responses, but it's not possible to define different responses for the same status code, eg:
|
@michaelbaisch, that is how responses are structured in OpenAPI. However, you can apply a trick and multiplex responses for one code: @extend_schema(
responses={
400: OpenApiResponse(
description="Parse error or Validation error",
response=PolymorphicProxySerializer(
component_name='Errors400',
serializers=[ValidationErrorSerializer, GenericErrorSerializer],
resource_type_field_name=None
)
)
}
) |
I guess for this to work, there should be serializers for all the DRF error responses, pretty straightforward for most of them (only
since |
Congratulations @michaelbaisch, you discovered the reason this was never implemented. 😄 The |
Hey there!
We have a view which may return many different 40X responses due to validation of the data. They all have the same general structure as they are derived from DRF's API exception class.
Is there a way to include these in the
extend_schema
decorator?I was expecting something like this to work:
Does something like this exist?
The text was updated successfully, but these errors were encountered: