django-restql: Creating a related object when using PATCH method raises a server error

Let’s say I have these models:

class Address(models.Model):
    street = models.CharField()
    city = models.CharField()
    postcode = models.CharField()

class Person(models.Model):
    address = models.ForeignKey('Address', null=True, blank=True, on_delete=models.SET_NULL)
    ... other fields ...

(note that all the fields in the Address model are required)

The address field is a NestedField in my serializer

class PersonSerializer(DynamicFieldsMixin, NestedModelSerializer):
    address = NestedField(AddressSerializer, allow_null=True, required=False)

When I create a Person object with address being null and then want to add the address later using a PATCH method with some of the fields missing, PATCH /api/person/1/

{
    "address": {"street": "Main street 1"}
}

I get a server error.

File "E:\Documents\Work\hriis-crm\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 200, in save
    self.instance = self.update(self.instance, validated_data)
  File "E:\Documents\Work\hriis-crm\backend\.venv\lib\site-packages\django_restql\mixins.py", line 985, in update
    fields["foreignkey_related"]["writable"]
  File "E:\Documents\Work\hriis-crm\backend\.venv\lib\site-packages\django_restql\mixins.py", line 756, in update_writable_foreignkey_related
    obj = serializer.save()
  File "E:\Documents\Work\hriis-crm\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 178, in save
    'You cannot call `.save()` on a serializer with invalid data.'
AssertionError: You cannot call `.save()` on a serializer with invalid data.

When I use a PUT method, I get a normal HTTP 400 response with validation messages.

{
  "address": {
    "city": [
      "This field is required."
    ],
    "postcode": [
      "This field is required."
    ]
  }
}

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 27 (12 by maintainers)

Commits related to this issue

🐛 Fix server error when using PUT/PATCH to create/update related objects - Fixes #211 — committed to yezyilomo/django-restql by yezyilomo 3 years ago
🐛Catch model constraint errors which led to server error - Fixes #211 — committed to yezyilomo/django-restql by yezyilomo 3 years ago

Most upvoted comments

@yezyilomo Just a heads up (If you know about it, then I’m sorry). You can specify a global exception handler in the settings like this:
REST_FRAMEWORK = {
    'EXCEPTION_HANDLER': 'config.utils.helpers.functions.db_exception_handler',
}
and then catch all Integrity errors and return a custom response. Here is what I have done:
from rest_framework.views import exception_handler

def db_exception_handler(exc, context):
    response = exception_handler(exc, context)
    if isinstance(exc, IntegrityError):
        return Response({'detail': str(exc)}, status.HTTP_409_CONFLICT)

    return response

I didn’t know about this, thanks for the heads up, will definitely use it next time.

yezyilomo on Aug 6, 2021

Yes that is the other way we were thinking of handling it.

I have the same thought as you. BUT, and you can try it with the repository I made. If you add raise_exception to the line I told you. I am not sure why, it returns a nice validation error and not the db integrity error.

Proof:

# Post without raise_exception=True
❯ python manage.py runserver
Quit the server with CONTROL-C.
Internal Server Error: /api/products/10/
  File "/Users/gonzaloamadio/.virtualenvs/django-basic/lib/python3.7/site-packages/django_restql/mixins.py", line 843, in bulk_create_many_to_one_related
    obj = serializer.save()
  File "/Users/gonzaloamadio/.virtualenvs/django-basic/lib/python3.7/site-packages/rest_framework/serializers.py", line 178, in save
    'You cannot call `.save()` on a serializer with invalid data.'
AssertionError: You cannot call `.save()` on a serializer with invalid data.
                                                                                                                                                                                                                               

❯ vi /Users/gonzaloamadio/.virtualenvs/django-basic/lib/python3.7/site-packages/django_restql/mixins.py
# here I added raise_exception=True to is_valid()

❯ python manage.py runserver
Bad Request: /api/products/10/
[06/Aug/2021 19:52:28] "PATCH /api/products/10/ HTTP/1.1" 400 79

This is the error returned to the api client

{
    "non_field_errors": [
        "The fields ingredient, product must make a unique set."
    ]
}

gonzaloamadio on Aug 6, 2021

Yes, It would be raised at the DB level, but, assuming devs log server errors, it would be apparent that something went wrong and devs could act accordingly. If a validation error is thrown, then it wouldn’t be so apparent.

I think when API user sends data which violates DB constraint that’s not devs fault, devs job is to make sure when something like this happen they tell API users what they have done wrong so that they can fix their input.

yezyilomo on Aug 7, 2021

I think if that if you run inside atomic requests, that won’t happen. It will be rolled back.

I have confirmed everything is rolled back if one step fails, You can use 'ATOMIC_REQUESTS': True, in DB config

yezyilomo on Aug 7, 2021

If the excpetion is not raised in the serializer validation, wont it be raised somewhere else anyway? At db level… so it does not matter where it is raised, first object would be created anyway?

Yeah, this is true, even if you don’t add raise_exeption=True the exception will be raised the difference is that with raise_exeption=True you get ValidationError, and without it the serializer crushes.

yezyilomo on Aug 6, 2021

Thank you for pointing this out, I’ve managed to reproduce it, it’s definitely a bug, we should be raising validation error and not server error. I’m working on fixing it, if you have any information about the cause feel free to share it.

yezyilomo on May 11, 2021