ashleysommer / sanic-cors

A Sanic extension for handling Cross Origin Resource Sharing (CORS), making cross-origin AJAX possible. Based on flask-cors by Cory Dolphin.
MIT License
159 stars 22 forks source link

Cors disabled for exception handlers #32

Closed jayrbolton closed 4 years ago

jayrbolton commented 5 years ago

I'd like responses to be CORS enabled for exceptions, but if the request is handled with an exception callback (using @app.exception(ErrorClass)), then CORS doesn't work.

The log says: [DEBUG] CORS: CORS was handled in the exception handler, skipping

Seems related to the SANIC_CORS_SKIP_RESPONSE_MIDDLEWARE and also https://github.com/ashleysommer/sanic-cors/issues/18

Am I missing something in the docs?

Example exception handler that disables cors:

@app.exception(sanic.exceptions.NotFound)
async def page_not_found(request, err):
    """Handle 404 as a json response."""
    return sanic.response.json({'error': '404 Not found'}, status=404)
ashleysommer commented 5 years ago

Sorry, for late reply. I saw this issue come through, but forgot to look into it. This definitely should already work. This is a normal use-case for sanic-cors and there are tests in place to ensure this functionality is working.

I will have to look more deeply into this to see if I can find a way of reproducing it.

jayrbolton commented 5 years ago

No worries. I'll try to make a repo that reproduces this

idanbensha commented 5 years ago

I'm experiencing this issue as well. please fix

ashleysommer commented 5 years ago

@idanbensha As mentioned above, this feature is already enabled and working in Sanic-CORS. If you can point me to some code that reproduces the error, I can fix the bug.

idanbensha commented 5 years ago

Figured that it was because the exception handler was an async method (as shown by sanic documentation). Once I removed the async keyword of my exception handler, it worked.

ashleysommer commented 5 years ago

I'm investigating this problem further. Yes, I think sanic can use async exception handlers (as shown in the documentation), but for some reason when combined with sanic-cors it cannot. It's definitely a problem I want to solve, because there have been three issues and one PR opened in the last month with users experiencing the same problem.

ashleysommer commented 4 years ago

Fixed by https://github.com/ashleysommer/sanic-cors/commit/2ad2bb0b699827ac6a63d985b8db8b1362977604