Improve error handling when accessing request.data in get_serializer_class

[DeadLemon]

Hi there!
I'm trying to specify different serializers depend on user type or request data.

class UserViewSet(serializers.ModelSerializer):
    ...
    def get_serializer_class(self):
        serializers_map = {}
        if self.request.user.is_authenticated:
            pass
        else:
            serializers_map.update({'create': serializers.CreateSerializerForAnonymousUser})
        return serializers_map.get(view.action, None)

So, I get an error:
[{'detail': 'JSON parse error - Expecting value: line 1 column 1 (char 0)', 'source': {'pointer': '/data'}, 'status': '400'}]
Here is my test code:

class UserEndpointCreateTest(APITestCase):
    def setUp(self):
        self.path = reverse('user-list')
        self.data = {
            'data': {
                'type': format_resource_type('user'),
                'attributes': {
                    'phone_number': '+79000000001',
                    'password': 'password'
                }
            }
        }

    def test_anonymous_user(self):
        response = self.client.post(reverse('user-list'), json.dumps(self.data), content_type='application/vnd.api+json')
        print(response.data)

Also, I tried this code with default rest_framework serializer, renderers and parsers - it works properly. Note that this problem exposes only when I'm trying to access request.user or request.data

[sliverc]

Could it be that this is a simply test setup issue and would be solved with changes of #396?

[sliverc]

As there hasn't been any feedback I assume this has been solved with changes in #396

Closing but if there is still a question concerning this issue please comment and we can reopen.

[aradbar]

@sliverc I just ran into this issue as well. From a quick look it seems that JSONParser uses get_serializer_class() as part of trying to determine the resource_name. So when the view implements a custom get_serializer_class that includes a reference to request.data this error occurs.

https://github.com/django-json-api/django-rest-framework-json-api/blob/master/rest_framework_json_api/parsers.py#L121
https://github.com/django-json-api/django-rest-framework-json-api/blob/master/rest_framework_json_api/utils.py#L55

[sliverc]

@aradbar Thanks for the additional information and the pointer. I see that accessing request.data in get_serializer_class causes issues although in the initial example of this issue it actually accesses request.user which in my opinion should work.

Anyhow I do not really see how this can be fixed while DJA continues supporting polymorphic serializers seamlessly. Or do you have any ideas?

A workaround which would work would be to define resource_name on the view which overwrites get_serializer_class. This might be good enough for affected users who might stumble upon this issue.

[aradbar]

@sliverc I agree with what you said but I think it's problematic for DJA to use get_serializer_class internally, as it is a DRF hook and can contain unexpected logic.
I would expect DJA to use serializer_class like it does here:

django-rest-framework-json-api/rest_framework_json_api/parsers.py

Line 143 in 2033bc5

serializer_class = getattr(view, 'serializer_class', None)

With an assertion error that tells the user do define resource_name in case serializer_class is not defined

[sliverc]

@aradbar I think this makes the situation worse for users not using polymorphic serializers as with such a change every user of DJA which has get_serializer_class overwritten will have an error. In the current state only users which access request.data within get_serializer_class do and I would argue accessing request.data is rare as it doesn't sound very resty to me.

Anyhow thinking about it again the error message is confusing. I think it would be good to improve it so states something in the lines that an parser error occurred while calling get_serializer_class as most likely request.data has been access and that overwriting resource_name could solve this issue.

So reoping this issue and marking so this can be addressed.

[avacaru]

I have a similar issue in DJA 6.0.0, but I'm not sure if they are related or I should create a new issue.

Basically the exception handler is calling get_serializer() which together with get_serializer_class() and get_serializer_context() can raise different exceptions.

For example, if you try to add a related object to the serializer context, but that object doesn't exist, a Http404 is raised. The code goes through the exception handler which when it reaches the DJA format_drf_errors() function, the exception is raised again, but never caught.

def format_drf_errors(response, context, exc):
    errors = []
    # handle generic errors. ValidationError('test') in a view for example
    if isinstance(response.data, list):
        for message in response.data:
            errors.extend(format_error_object(message, "/data", response))
    # handle all errors thrown from serializers
    else:
        # Avoid circular deps
        from rest_framework import generics

        has_serializer = isinstance(context["view"], generics.GenericAPIView)
        if has_serializer:
            serializer = context["view"].get_serializer()
            fields = get_serializer_fields(serializer) or dict()
            relationship_fields = [
                format_field_name(name)
                for name, field in fields.items()
                if is_relationship_field(field)
            ]

        for field, error in response.data.items():
            field = format_field_name(field)
            pointer = None
            # pointer can be determined only if there's a serializer.
            if has_serializer:
                rel = "relationships" if field in relationship_fields else "attributes"
                pointer = f"/data/{rel}/{field}"
            if isinstance(exc, Http404) and isinstance(error, str):
                # 404 errors don't have a pointer
                errors.extend(format_error_object(error, None, response))
            elif isinstance(error, str):
                classes = inspect.getmembers(exceptions, inspect.isclass)
                # DRF sets the `field` to 'detail' for its own exceptions
                if isinstance(exc, tuple(x[1] for x in classes)):
                    pointer = "/data"
                errors.extend(format_error_object(error, pointer, response))
            elif isinstance(error, list):
                errors.extend(format_error_object(error, pointer, response))
            else:
                errors.extend(format_error_object(error, pointer, response))

    context["view"].resource_name = "errors"
    response.data = errors

    return response

[sliverc]

@avacaru No this issue is specifically about accessing request.data in get_serializer_class. Your issue is actually related to #1140 and when implemented as mentioned in #1140 (comment) your issue should be fixed as well.