Django 3.1: CORS headers not set on error responses?

[bjmc]

Hello, I'm not sure if there's something I'm misunderstanding, but from my experiments, it seems like django-cors-headers is not setting CORS headers on HTTP 4XX error responses, when used with Django 3.1. I know there's been some changes to the way middleware are handled over the years. Is it possible that django-cors-headers needs to be updated to be fully compatible with newer versions?

As an experiment, I added a process_exception() method to CorsMiddleware, like this:

def process_exception(self, request, exception):
    return self.process_response(request, exception)

And that seems to have resolved my issue. Is this a sensible way to proceed? If so, I'm happy to put together a formal pull request.

Thanks for a useful library!

[adamchainz]

Hi!

Yes we should probably do this.

But does it help you if CORS headers are set on error responses? Was the error expected on the server side, and something you could then process in browser?

*Thanks,
Adam

[bjmc]

In our case, yes, I think we definitely want CORS headers set on error responses. We're using graphene-django to build a GraphQL API. If the client sends us a malformed query, we want to be able to return a sensible error without the browser tripping over CORS restrictions.

I can't say this is universal, but I would guess most people building APIs in Django and setting CORS headers would also want those headers on their error responses. Do you think it's correct to treat process_exception() and process_response() equivalently? The only thing that gives me pause is returning an HTTP response from process_exception() will short-circuit other middleware executions. But the CORS middleware needs to be near the outside of the middleware stack anyway, right?

[marcosalcazar]

@bjmc Hello! did you go with that solution in the end? We are facing this same issue here with our graphene-django API endpoing.

[bjmc]

Hi @marcosalcazar yes, we just subclassed CorsMiddleware and added the process_exception() method as above. We haven't run into any problems that I'm aware of, but it does feel a little bit hacky. I'd like to see a proper upstream fix for this issue.

[marcosalcazar]

Cool, thanks @bjmc for the quick response!

[kbakk]

A "+ 1" for adding support for process_exception from me. The missing process_exception caused some head scratching and debugging, as I couldn't understand why it would work when I used HttpResponse("Bad...", status=400) but not raise BadRequest("Bad...").

The suggested workaround doesn't seem to work for me – when raising BadRequest, exception is an exception object, so that results in AttributeError: 'BadRequest' object has no attribute 'has_header'.

As an fun fact, this can be raised (with the workaround in place) and will get the correct headers:

class StrangeException(HttpResponse, Exception):
    pass
...
raise StrangeException("Invalid response", status=400)

[merwok]

But does it help you if CORS headers are set on error responses? Was the error expected on the server side, and something you could then process in browser?

It would help coworkers if the real error messages in the browser console were not hidden by CORS check errors 🙂