Middleware breaks raising APIException from ViewSet

[FinnGu]

When raising a subclass of APIException (for example ValidationError or ParseError) from a ViewSet, I get a very cryptic error message that suggests that some sort function is broken. When I disable the middleware, everything works as expected.

Minimal code example:

# views.py
class MyViewSet(viewsets.GenericViewSet):
    ...

    @action(methods=['post'], detail=False, url_path='bulk-create')
    def bulk_create(self, request, *args, **kwargs):
        # Some assertion about data structure        
        if True:
            raise ParseError("Expected the data to be in list format.")

        # Actually process incoming data
        ...

Error log:

Traceback (most recent call last):
2022-05-17T12:23:45.366216553Z   File "/opt/venv/lib/python3.9/site-packages/django/core/handlers/exception.py", line 47, in inner
2022-05-17T12:23:45.366221178Z     response = get_response(request)
2022-05-17T12:23:45.366223345Z   File "/opt/venv/lib/python3.9/site-packages/debug_toolbar/middleware.py", line 67, in __call__
2022-05-17T12:23:45.366225470Z     panel.generate_stats(request, response)
2022-05-17T12:23:45.366238220Z   File "/opt/venv/lib/python3.9/site-packages/debug_toolbar/panels/request.py", line 30, in generate_stats
2022-05-17T12:23:45.366240470Z     "post": get_sorted_request_variable(request.POST),
2022-05-17T12:23:45.366346803Z   File "/opt/venv/lib/python3.9/site-packages/debug_toolbar/utils.py", line 227, in get_sorted_request_variable
2022-05-17T12:23:45.366352511Z     return [(k, variable.getlist(k)) for k in sorted(variable)]
2022-05-17T12:23:45.366354803Z TypeError: '<' not supported between instances of 'dict' and 'dict'

I have also posted this question on StackOverflow: https://stackoverflow.com/questions/72274329/cannot-raise-apiexception-from-django-viewset

Thank you for your help in advance.

[matthiask]

I think we had a problem like this in the past. We should probably add a fallback somewhere if sorting the data in request.POST fails instead of crashing and burning. It's definitely unexpected that request.POST contains anything else than string keys and string values though.

Are you using django-rest-framework or something similar by chance? Does DRF modify Django's standard request.POST object?

[FinnGu]

Are you using django-rest-framework or something similar by chance? Does DRF modify Django's standard request.POST object?

Yes, I am using DRF version 3.12.4.

I just logged request.POST and only got <QueryDict: {}>. Does that help?

[tim-schilling]

Can you log/print variable for the exception flow? Both QueryDict and a dict can have sorted called on them. However, calling sorted([{"a": "b"}, {"c": "d"}]) will raise the error you get. And in your SO question, you made a statement about request.data being a list or not.

[tim-schilling]

@matthiask it sounds like somehow request.POST isn't actually a dictionary for this case.

[matthiask]

@tim-schilling Yes, it seems like that. I'm confused by <QueryDict: {}> though if it isn't actually a QueryDict instance.

@FinnGu What are you POSTing exactly? A JSON document with a list as its outer element? DRF seems to set request.POST (see https://github.com/encode/django-rest-framework/blob/3.12.4/rest_framework/request.py) (well, request._post but that's basically the same) but only if the content type of the request matches a form media type, and application/json shouldn't trigger this behavior. Maybe you're POSTing a JSON document with a form media type? I have a hard time imaging what goes wrong, but the exact request you're sending might help with constructing a test case for django-debug-toolbar. I wouldn't want DDT to produce meaningful output if the request is in fact not well formed but DDT shouldn't crash.

[FinnGu]

@matthiask Here is the JSON, I am POSTing with Content-Type: application/json:

[
	{
		"shop_reference_id": "891",
		"date": "2021-09-01T20:49:01.0000Z",
		"donations": [
			{
				"amount": "1.23",
				"cooperation": 14,
				"donor_type": "PERSONAL"
			},
			{
				"amount": "0.10",
				"cooperation": 14,
				"donor_type": "BUSINESS"
			}
		]
	},
	{
		"shop_reference_id": "1343",
		"date": "2021-11-20T21:03:22.0000Z",
		"donations":[
			{
				"amount": "0.15",
				"cooperation": 14,
				"donor_type": "BUSINESS"
			}
		]
	}
]

I just tested your assumption and you are correct. It is enough to pass an array with any kind of object in it, to crash the DDT. Is it bad practice to use arrays in RESTful design? Because it is valid JSON.

[FinnGu]

@tim-schilling Which variable did you want me to print? I am confused.

[matthiask]

Great, thanks!

I just tested your assumption and you are correct. It is enough to pass an array with any kind of object in it, to crash the DDT. Is it bad practice to use arrays in RESTful design? Because it is valid JSON.

I'm not sure. Django's JsonResponse doesn't like arrays as top-level objects (see https://docs.djangoproject.com/en/4.0/ref/request-response/#serializing-non-dictionary-objects). Using arrays makes it hard to extend the API later, e.g. you cannot easily add additional fields such as maybe a version field or something. I would recommend to always use dictionaries / objects, e.g. {"payments": [... your data ...]} even if you control both the Frontend and Backend code but that's just my opinion (which is informed by experience, but still)

Do what works for you. I'd say that the real problem here is that some library (possibly DRF) changes request.POST to an unexpected data structure.

Here's a draft pull request with a proposed fix for this issue: #1624

[FinnGu]

Using arrays makes it hard to extend the API later, e.g. you cannot easily add additional fields such as maybe a version field or something. I would recommend to always use dictionaries / objects, e.g. {"payments": [... your data ...]} even if you control both the Frontend and Backend code but that's just my opinion (which is informed by experience, but still)

Your PR solved the issue perfectly, thanks! Also very good advice on using arrays as top-level structure in a REST API. I will rethink my design for that route.

EDIT: Apparently Django's testing framework has similar issues with top-level arrays. You just convinced me to wrap everything in an object....