search

corydolphin / flask-cors

Cross Origin Resource Sharing ( CORS ) support for Flask
https://flask-cors.readthedocs.io/en/latest/index.html
MIT License
879 stars 139 forks source link

CORS headers are not sent with decorator when there is an exception in view function #198

Closed cenkalti closed 6 years ago

cenkalti commented 7 years ago

In other words, intercept_exceptions option is not implemented in the decorator.

cenkalti commented 7 years ago

I can submit a PR with the implementation if you want to accept @corydolphin

corydolphin commented 7 years ago

@cenkalti that is correct. It is not listed in the docs, but it is admittedly not very clear.

I would be very open to such a contribution. I would want to make sure it didn't interrupt standard handling of exceptions in any way in flask, or muddy the stack traces for application developers.

Is your idea to wrap the call in a try/except and re-raise?

Currently the extension registers an error handler https://github.com/corydolphin/flask-cors/blob/master/flask_cors/extension.py#L164 Sadly, Flask handles exceptions differently in debug and production mode, making the current approach not work in debug mode, causing some developer headaches.

cenkalti commented 7 years ago

I was thinking to use the deferred callback pattern described here: http://flask.pocoo.org/docs/0.12/patterns/deferredcallbacks/ but then, I have realized that neither the decorator or the view function does not have access to Flask application instance. I have an ugly solution in my mind that I will try to implement and send PR for it. Feel free to reject if you don't like it.

However, without handling exceptions, the decorator is not very helpful because browsers cannot even read the response code (4xx or 5xx) when there is no Access-Control-Allow-Origin header.

cenkalti commented 7 years ago

After some investigation, I think it is not enough to use only the decorator for proper CORS support because for the application to send CORS headers with 404 responses, a custom error handler needs to be registered. So, using both the extension and the decorator is the correct approach in my opinion. Should we make this clear in docs?

corydolphin commented 7 years ago

Yeah sadly this is tough to support. I wish there was a better way to support this in DEV. When not in Dev, it should work as expected.

corydolphin commented 6 years ago

Merging this into #210