Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix FlaskApp exception handlers #1788

Merged
merged 2 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions connexion/apps/flask.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import typing as t

import flask
import starlette.exceptions
import werkzeug.exceptions
from a2wsgi import WSGIMiddleware
from flask import Response as FlaskResponse
from starlette.types import Receive, Scope, Send
Expand Down Expand Up @@ -213,7 +215,7 @@ def __init__(
:obj:`security.SECURITY_HANDLERS`
"""
self._middleware_app = FlaskASGIApp(import_name, server_args or {})
self.app = self._middleware_app.app

super().__init__(
import_name,
lifespan=lifespan,
Expand All @@ -233,6 +235,15 @@ def __init__(
security_map=security_map,
)

self.app = self._middleware_app.app
self.app.register_error_handler(
werkzeug.exceptions.HTTPException, self._http_exception
)

def _http_exception(self, exc: werkzeug.exceptions.HTTPException):
"""Reraise werkzeug HTTPExceptions as starlette HTTPExceptions"""
raise starlette.exceptions.HTTPException(exc.code, detail=exc.description)

def add_url_rule(
self, rule, endpoint: str = None, view_func: t.Callable = None, **options
):
Expand All @@ -247,4 +258,4 @@ def add_error_handler(
[ConnexionRequest, Exception], MaybeAwaitable[ConnexionResponse]
],
) -> None:
self.app.register_error_handler(code_or_exception, function)
self.middleware.add_error_handler(code_or_exception, function)
67 changes: 67 additions & 0 deletions connexion/http_facts.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,70 @@
FORM_CONTENT_TYPES = ["application/x-www-form-urlencoded", "multipart/form-data"]

METHODS = {"get", "put", "post", "delete", "options", "head", "patch", "trace"}

HTTP_STATUS_CODES = {
100: "Continue",
101: "Switching Protocols",
102: "Processing",
103: "Early Hints", # see RFC 8297
200: "OK",
201: "Created",
202: "Accepted",
203: "Non Authoritative Information",
204: "No Content",
205: "Reset Content",
206: "Partial Content",
207: "Multi Status",
208: "Already Reported", # see RFC 5842
226: "IM Used", # see RFC 3229
300: "Multiple Choices",
301: "Moved Permanently",
302: "Found",
303: "See Other",
304: "Not Modified",
305: "Use Proxy",
306: "Switch Proxy", # unused
307: "Temporary Redirect",
308: "Permanent Redirect",
400: "Bad Request",
401: "Unauthorized",
402: "Payment Required", # unused
403: "Forbidden",
404: "Not Found",
405: "Method Not Allowed",
406: "Not Acceptable",
407: "Proxy Authentication Required",
408: "Request Timeout",
409: "Conflict",
410: "Gone",
411: "Length Required",
412: "Precondition Failed",
413: "Request Entity Too Large",
414: "Request URI Too Long",
415: "Unsupported Media Type",
416: "Requested Range Not Satisfiable",
417: "Expectation Failed",
418: "I'm a teapot", # see RFC 2324
421: "Misdirected Request", # see RFC 7540
422: "Unprocessable Entity",
423: "Locked",
424: "Failed Dependency",
425: "Too Early", # see RFC 8470
426: "Upgrade Required",
428: "Precondition Required", # see RFC 6585
429: "Too Many Requests",
431: "Request Header Fields Too Large",
449: "Retry With", # proprietary MS extension
451: "Unavailable For Legal Reasons",
500: "Internal Server Error",
501: "Not Implemented",
502: "Bad Gateway",
503: "Service Unavailable",
504: "Gateway Timeout",
505: "HTTP Version Not Supported",
506: "Variant Also Negotiates", # see RFC 2295
507: "Insufficient Storage",
508: "Loop Detected", # see RFC 5842
510: "Not Extended",
511: "Network Authentication Failed",
}
3 changes: 2 additions & 1 deletion connexion/lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,5 +260,6 @@ def __init__(
self.content_type = content_type
self.body = body
self.headers = headers or {}
self.headers.update({"Content-Type": content_type})
if content_type:
self.headers.update({"Content-Type": content_type})
self.is_streamed = is_streamed
28 changes: 7 additions & 21 deletions connexion/middleware/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import asyncio
import functools
import logging
import typing as t

import werkzeug.exceptions
from starlette.concurrency import run_in_threadpool
from starlette.exceptions import HTTPException
from starlette.middleware.exceptions import (
Expand All @@ -12,6 +12,7 @@
from starlette.responses import Response as StarletteResponse
from starlette.types import ASGIApp, Receive, Scope, Send

from connexion import http_facts
from connexion.exceptions import InternalServerError, ProblemException, problem
from connexion.lifecycle import ConnexionRequest, ConnexionResponse
from connexion.types import MaybeAwaitable
Expand All @@ -28,6 +29,7 @@ def connexion_wrapper(
them to the error handler, and translates the returned Connexion responses to
Starlette responses."""

@functools.wraps(handler)
async def wrapper(request: StarletteRequest, exc: Exception) -> StarletteResponse:
request = ConnexionRequest.from_starlette_request(request)

Expand All @@ -36,6 +38,9 @@ async def wrapper(request: StarletteRequest, exc: Exception) -> StarletteRespons
else:
response = await run_in_threadpool(handler, request, exc)

while asyncio.iscoroutine(response):
response = await response

return StarletteResponse(
content=response.body,
status_code=response.status_code,
Expand All @@ -53,9 +58,6 @@ class ExceptionMiddleware(StarletteExceptionMiddleware):
def __init__(self, next_app: ASGIApp):
super().__init__(next_app)
self.add_exception_handler(ProblemException, self.problem_handler) # type: ignore
self.add_exception_handler(
werkzeug.exceptions.HTTPException, self.flask_error_handler
)
self.add_exception_handler(Exception, self.common_error_handler)

def add_exception_handler(
Expand All @@ -81,7 +83,7 @@ def http_exception(
"""Default handler for Starlette HTTPException"""
logger.error("%r", exc)
return problem(
title=exc.detail,
title=http_facts.HTTP_STATUS_CODES.get(exc.status_code),
detail=exc.detail,
status=exc.status_code,
headers=exc.headers,
Expand All @@ -95,21 +97,5 @@ def common_error_handler(
logger.error("%r", exc, exc_info=exc)
return InternalServerError().to_problem()

def flask_error_handler(
self, request: StarletteRequest, exc: werkzeug.exceptions.HTTPException
) -> ConnexionResponse:
"""Default handler for Flask / werkzeug HTTPException"""
# If a handler is registered for the received status_code, call it instead.
# This is only done automatically for Starlette HTTPExceptions
if handler := self._status_handlers.get(exc.code):
starlette_exception = HTTPException(exc.code, detail=exc.description)
return handler(request, starlette_exception)

return problem(
title=exc.name,
detail=exc.description,
status=exc.code,
)

async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
await super().__call__(scope, receive, send)
76 changes: 43 additions & 33 deletions docs/exceptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@ Exception Handling
Connexion allows you to register custom error handlers to convert Python ``Exceptions`` into HTTP
problem responses.

You can register error handlers on:

- The exception class to handle
If this exception class is raised somewhere in your application or the middleware stack,
it will be passed to your handler.
- The HTTP status code to handle
Connexion will raise ``starlette.HTTPException`` errors when it encounters any issues
with a request or response. You can intercept these exceptions with specific status codes
if you want to return custom responses.

.. tab-set::

.. tab-item:: AsyncApp
:sync: AsyncApp

You can register error handlers on:

- The exception class to handle
If this exception class is raised somewhere in your application or the middleware stack,
it will be passed to your handler.
- The HTTP status code to handle
Connexion will raise ``starlette.HTTPException`` errors when it encounters any issues
with a request or response. You can intercept these exceptions with specific status codes
if you want to return custom responses.

.. code-block:: python

from connexion import AsyncApp
Expand All @@ -40,17 +40,6 @@ problem responses.
.. tab-item:: FlaskApp
:sync: FlaskApp

You can register error handlers on:

- The exception class to handle
If this exception class is raised somewhere in your application or the middleware stack,
it will be passed to your handler.
- The HTTP status code to handle
Connexion will raise ``starlette.HTTPException`` errors when it encounters any issues
with a request or response. The underlying Flask application will raise
``werkzeug.HTTPException`` errors. You can intercept both of these exceptions with
specific status codes if you want to return custom responses.

.. code-block:: python

from connexion import FlaskApp
Expand All @@ -69,20 +58,34 @@ problem responses.
.. automethod:: connexion.FlaskApp.add_error_handler
:noindex:

.. tab-item:: ConnexionMiddleware
:sync: ConnexionMiddleware
.. note::

.. warning::

⚠️ **The following is not recommended as it complicates the exception handling logic,**

You can register error handlers on:
You can also register error handlers on the underlying flask application directly.

- The exception class to handle
If this exception class is raised somewhere in your application or the middleware stack,
it will be passed to your handler.
- The HTTP status code to handle
Connexion will raise ``starlette.HTTPException`` errors when it encounters any issues
with a request or response. You can intercept these exceptions with specific status codes
if you want to return custom responses.
Note that this might not catch ``HTTPExceptions`` with the same status code raised by
your wrapped ASGI/WSGI framework.
.. code-block:: python

flask_app = app.app
flask_app.register_error_handler(FileNotFoundError, not_found)
flask_app.register_error_handler(404, not_found)

`Flask documentation`_

Error handlers registered this way:

- Will only intercept exceptions thrown in the application, not in the Connexion
middleware.
- Can intercept exceptions before they reach the error handlers registered on the
connexion app.
- When registered on status code, will intercept only
``werkzeug.exceptions.HTTPException`` thrown by werkzeug / Flask not
``starlette.exceptions.HTTPException``.

.. tab-item:: ConnexionMiddleware
:sync: ConnexionMiddleware

.. code-block:: python

Expand All @@ -105,10 +108,17 @@ problem responses.
.. automethod:: connexion.ConnexionMiddleware.add_error_handler
:noindex:

.. note::

This might not catch ``HTTPExceptions`` with the same status code raised by
your wrapped ASGI/WSGI framework.

.. note::

Error handlers can be ``async`` coroutines as well.

.. _Flask documentation: https://flask.palletsprojects.com/en/latest/errorhandling/#error-handlers

Default Exception Handling
--------------------------
By default connexion exceptions are JSON serialized according to
Expand Down
5 changes: 3 additions & 2 deletions docs/v3.rst
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,9 @@ Smaller breaking changes
has been added to work with Flask's ``MethodView`` specifically.
* Built-in support for uWSGI has been removed. You can re-add this functionality using a custom middleware.
* The request body is now passed through for ``GET``, ``HEAD``, ``DELETE``, ``CONNECT`` and ``OPTIONS`` methods as well.
* Error handlers registered on the on the underlying Flask app directly will be ignored. You
should register them on the Connexion app directly.
* The signature of error handlers has changed and default Flask error handlers are now replaced
with default Connexion error handlers which work the same for ``AsyncApp`` and
``ConnexionMiddleware``.


Non-breaking changes
Expand Down
17 changes: 12 additions & 5 deletions tests/api/test_bootstrap.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import json
from unittest import mock

import jinja2
Expand All @@ -7,6 +8,7 @@
from connexion.exceptions import InvalidSpecification
from connexion.http_facts import METHODS
from connexion.json_schema import ExtendedSafeLoader
from connexion.lifecycle import ConnexionRequest, ConnexionResponse
from connexion.middleware.abstract import AbstractRoutingAPI
from connexion.options import SwaggerUIOptions

Expand Down Expand Up @@ -302,10 +304,15 @@ def test_add_error_handler(app_class, simple_api_spec_dir):
app = app_class(__name__, specification_dir=simple_api_spec_dir)
app.add_api("openapi.yaml")

def custom_error_handler(_request, _exception):
pass
def not_found(request: ConnexionRequest, exc: Exception) -> ConnexionResponse:
return ConnexionResponse(
status_code=404, body=json.dumps({"error": "NotFound"})
)

app.add_error_handler(Exception, custom_error_handler)
app.add_error_handler(500, custom_error_handler)
app.add_error_handler(404, not_found)

app.middleware._build_middleware_stack()
app_client = app.test_client()

response = app_client.get("/does_not_exist")
assert response.status_code == 404
assert response.json()["error"] == "NotFound"
Loading