Description
Checklist
- I have verified that that issue exists against the
master
branch of Django REST framework. - I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
- This is not a usage question. (Those should be directed to the discussion group instead.)
- This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
- I have reduced the issue to the simplest possible case.
- I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)
Steps to reproduce
- Have
ATOMIC_REQUESTS
enabled. - Receive request.
- Save something to the database.
- Respond with
serializer.is_valid(raise_exception=True)
OR
raise Http404
OR
raise PermissionDenied
.
Expected behavior
- Data is saved to the database.
Actual behavior
- Database transaction is rolled back.
This came as something of a surprise!
This doesn't happen with 400
, 403
, or 404
from a return Response
because this logic is in the of the exception_handler
.
I was under the impression that raising these exceptions was an equivalent "shortcut" to manually returning these responses. I was not aware of this side-effect, but it does look as though it was intentionally introduced in 3.1.3
(#2887).
I've discovered that I can change this behaviour by overriding EXCEPTION_HANDLER
, or by using the full form of return Response(serializer.errors, status=400)
(etc), but it feels wrong somehow.
I've asked a few friends, and some of them suggest that rolling back the DB like this is a REST thing, but I'm not convinced of that. Otherwise, wouldn't all 400, 403, and 404 need to be rolled back regardless of if they're raised / returned?