Custom Response formatting

[Marlon Bailey]

Currently if an exception in thrown in DRF the error formatting is fixed, i.e.:

{ 'detail': 'Failure...' }

Our internal API specs return errors in this format:

{'errors': [{'message': 'Failure...', 'code': ''} ...]}

This is fine when we catch it in code for things like serializer errors, however in exceptions outside of our code path  or exceptions we don't catch we end up back on 

{'detail': 'Failure...'}

so now we have a mix of error formats, which drives our API consumers batty.  I'd like to propose being able to set a custom Response class, which inherits from Response, on each APIView/ViewSet class, when nothing is set it defaults back to the standard Response class.  This is so people can not only do things like format error messages to their error spec, but also do whatever other pre-processing they may need to do such as logging to Sentry.etc..

So a line from handle_exception()  would go from:

            return Response({'detail': exc.detail},
                            status=exc.status_code,
                            exception=True)
to           
            return cls.ResponseClass({'detail': exc.detail},
                            status=exc.status_code,
                            exception=True)

I originally thought of just making a specific call for formatting of errors but then I realize it may not have as much functionality.  Thanks for looking, I can do the patch I'd just like opinions, maybe there is a better way to do this, or already functionality for it.

_Marlon_

[Marlon Bailey]

Any comments on this? =/

[Alan Justino (alanjds)]

I would like this format too. Is this already possible or need changes on DRF code? If possible, can you please clarify how, expanding the example?

[Marlon Bailey]

Sure, what do you want me to expand?  I'd expect the class definition to looking like this

from rest_framework import viewsets

from rest_framework.response import Response

class CustomResponse(Response):

    def __init__(*args, **kwargs):

         # change formatting

class FooView(viewsets.ViewSet):

    response_class = CustomResponse

    def ..

Let me know if that's not clear enough.

_Marlon_

[Tom Christie]

Hi Marlon,

  There's a pending pull request that addresses at least some of this.

What that PR would allow was setting an exception handler function that deals with translating exceptions into responses.  The default is to handle any subclass of APIException, plus Django's built in Http404 and PermissionDenied exceptions.  (Anything else bubbles up and will normally raise a 500 error)

This'd make it a lot easier to customize how exception responses are formatted.

One thing it *doesn't* deal with is how regularly returned responses are formatted.  So for example, the 400 responses that are returned by the generic views in response to invalid request data won't be handled by the exception handler.   I'm not sure if or how we'd want to deal with customizing those responses, although you mentioned this which makes me think that may not be an issue for you.

> This is fine when we catch it in code for things like serializer errors, however in exceptions outside of our code path  or exceptions we don't catch we end up back on 

So, question: Does the intent of the PR meet your use-case and do you think it's sufficient?

Cheers,

   Tom

[Marlon Bailey]

Hi Tom,

   Thanks for the response.  First i think the idea of targeting the Response class is much more flexible, and has the added benefit of not having a ton of code of forcing convention.

class SomeView(APIView):

    response_class = CustomResponse

seems way simpler and more pythonic to me, than what is going on in the pull request.  The current Response object already understands when it's being handed an exception, and it's being sent the status, so this could also handle the PR's initial reasoning

class SomeResponse(Response):

   def __init__(self, *args, **kwargs);

    if kwargs['status'] >= 400;

       raise Exception('some exception')

Unless i"m missing the point of PR.  So I'd still rather do things that related to the Response in the Response class.  The separation of concerns seems better served there.  I can create a pull request so you can take a look if you need more.  I don't always know if I'm being clear when I'm trying to get this done at work = )

_Marlon_

[Tom Christie]

Hi Marlon,

  I'm not sure.  Being able to modify the response class *is* very flexible, but I'm not convinced if that's a good thing.

What I don't want to do is be writing code in the generic views that I wouldn't be happy to see in user code.

It's entirely obvious what this is doing:

    return Response(...)

This has an extra indirection that obfuscates the intent.

    return self.response_cls(...)

What I like about a customizable exception handler is that it's conceptually separate from the view code.

I don't consider that PR complete yet, and there would need to be some simplification of the default error handler,

so that overriding it doesn't require lots of boilerplate, but it's def still my preferred option at the moment.

[Marlon Bailey]

I have to respectfully disagree, that self.response_class(), is obfuscated or indirect.  It's a standard python idiom. 

We could rewrite it to:

    ResponseClass = gettatr(self, response_class)

    return ResponseClass(...)

  Either way, I'm having this problem because the default exception handler is hard-coded to only return Response.  I'd like to have control over what exits my view, it's a little strange that I don't right now.

  Django gives me this control right now.  I don't believe hard-coding a single Response class is going to be enough in the long run.  It seems anti-robust.

_Marlon_

[Tom Christie]

Hey Marlon,

  To clarify, I don't think that having a `response_cls` attribute on the view is a bad idea - I'd considered it before we had this conversation, and it's not unreasonable, I just don't think it quite fits.  I've spent a lot of effort trying to slim down the generic views API, in particular making a couple of tweaks that take it away from the Django GCBV implementation.  Introducing an extra attribute specifying the response class may seem like a small thing, but it's not the direction I want the API to go.  In any case the core problem that you've hit is not that users can't set the class of the response, but that the data that populates the response is in a fixed style.  I'd rather see API that enables changing that style, rather than subclassing the response class in order to kludge it.  That's not to say that I might not change my mind some point later down the road, but I'm trying to be very strict about introducing new API points, and it's not something I want to compromise on right now.  I hope the above helps explain the decision to some extent.

Cheers,

  Tom

[Marlon Bailey]

Hey Tom,

    Sorry for the insanely delayed response, I have a major delivery in under a month, and I'm on the wrong side of time. =/  First off, I want to thank you for the email, it means a lot.  It also makes me feel great that we're using Django Rest Framework, because you're obviously dedicated.  I agree with your decision to keep the API strict right now until a definitive needs comes up for giving an override to the Response class.   That being said, you're right, I definitely need a way to control the return data's format.  I'm the lead architect of a new platform that is rolling out and our team exclusively uses Django Rest Framework for APIs, it would look poor if we couldn't keep all out output to the spec that we defined. = )  If you have any direction on this(i.e. a modification you'd be happy with), I'll gladly code up a pull request for you if it'll solve our problem.  Also, as an aside, any chance you can make access to Django's standard request.session public, or is there a different way of getting this data? I've had to use the _request attribute, I'd rather not do that, but we need to get access to that for our access tokens we use some of our DRF views to call external oauth2 apis.

_Marlon_

--
You received this message because you are subscribed to a topic in the Google Groups "Django REST framework" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-rest-framework/ndgmIU8kfOY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-rest-fram...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Tom Christie]

Hi Marlon

> If you have any direction on this(i.e. a modification you'd be happy with), I'll gladly code up a pull request for you if it'll solve our problem

There are two things that need to happen here:

1. Customizable exception handling.

2. Appropriate hooks to allow customizing of non-exception responses.

Let's deal with these in turn:

The first needs an `EXCEPTION_HANDLER` setting, that takes an exception and returns an appropriate reponse (or None if it cannot be handled and should 500 error).  I've done a little prep work to make fixing this easier - see this commit:

https://github.com/tomchristie/django-rest-framework/commit/b430503fa657330b606a9c632ea0decc4254163e

The exception handler now just needs to be set via an `EXCEPTION_HANDLER` setting that defaults to `'rest_framework.view.exception_handler'`.  Plus of course a test and docs.  You'll see that some logic is still handled in the view, which leaves a fairly small stub to override if you want to customize the style of exception responses.

The second I think needs a `get_success_response(data=None, created=False)` and `get_failure_response` hooks in the GenericAPIView.  I think we should deprecate `get_success_headers` and move all the 200/201/204 logic into the `get_success_response` method.

    def get_success_response(data=None, created=False):

        status = HTTP_200_OK

        headers = {}

        if created:

            status = HTTP_201_CREATED

            headers = ...  # same as existing `get_success_headers`

        if data is None:

            status = HTTP_204_NO_CONTENT

        return Response(data, status=status, headers=headers)

    def get_failure_response(errors):

        return Response(status.HTTP_400_BAD_REQUEST)

 

That'd make it easy enough to customise non-exception responses (either the status codes, headers, or the response body), by creating a mixin and using that throughout, or by customizing them on a per-view basis.

Sound okay?... Presumably those two sets of changes would allow you to support your use case?

> any chance you can make access to Django's standard request.session public

It already is (or should be).  This is implicitly noted in the 'Standard HTTPRequest attributes' section:

http://django-rest-framework.org/api-guide/requests.html#standard-httprequest-attributes

but I've just updated the docs to explicitly include `request.session` in that paragraph.  (Not yet up on the site.)

Cheers,

  Tom

To unsubscribe from this group and all its topics, send an email to django-rest-framework+unsub...@googlegroups.com.

[Matt Long]

Hello,

I have a very similar need to customize the format of the body of all kinds of non-successful responses. Including those caused by authentication errors, validation errors, other exceptions...basically anything with a status_code >= 400.

To accomplish this, I'm thought is to write a process_template_response middleware that will analyze response.data, and re-assign it with error messages in my desired format. Middleware seems like the most Django-ish way to accomplish custom error handling barring some sort of error formatting hook being formally added to DRF.

I'm attempting to enumerate all possible formats of the default error responses from DRF and have come up with the following:

{

    "detail": "message caused by an APIException such as failed authentication"

}

{

    "non_field_errors": ["errors causes by my custom serializer validate functions", ...]

}

{

    "<field_name>": ["validation errors related to a particular field", ...]

}

Are there any other hard-coded keys besides "detail" and "non_field_errors" that I could expect to see in a failed response object? Or can I assume that any other key refers to a particular field? I'm very open to suggestions if there might be a better way to do this. After I get something working, I'll be happy to share my solution.

Thanks!

[Eddwin Paz]

http://stackoverflow.com/questions/23642070/how-configure-django-rest-framework-to-return-errors-with-custom-text/23654158#23654158 there you cand find your answer ;)