refactor(asgi): Require error handlers to be coroutines when using wi…

…th ASGI

falcon/app.py

@@ -15,6 +15,7 @@
 """Falcon App class."""
 
 from functools import wraps
+from inspect import iscoroutinefunction
 import re
 import traceback
 
@@ -632,9 +633,17 @@ def handle(req, resp, ex, params):
 
         """
         def wrap_old_handler(old_handler):
+            if iscoroutinefunction(old_handler):
+                @wraps(old_handler)
+                async def handler_async(req, resp, ex, params):
+                    await old_handler(ex, req, resp, params)
+
+                return handler_async
+
             @wraps(old_handler)
             def handler(req, resp, ex, params):
                 old_handler(ex, req, resp, params)
+
             return handler
 
         if handler is None:

falcon/asgi/app.py

@@ -20,12 +20,12 @@
 
 from falcon.api_helpers import prepare_middleware
 import falcon.app
-from falcon.errors import UnsupportedScopeError
+from falcon.errors import CompatibilityError, UnsupportedScopeError
 from falcon.http_error import HTTPError
 from falcon.http_status import HTTPStatus
 import falcon.routing
 from falcon.util.misc import http_status_to_code
-from falcon.util.sync import set_coroutine_flag
+from falcon.util.sync import _wrap_non_coroutine_unsafe
 from .request import Request
 from .response import Response
 from .structures import SSEvent
@@ -479,8 +479,13 @@ def add_error_handler(self, exception, handler=None):
                 # NOTE(kgriffs): Delegate to the parent method for error handling.
                 pass
 
-        if handler:
-            set_coroutine_flag(handler)
+        handler = _wrap_non_coroutine_unsafe(handler)
+
+        if handler and not iscoroutinefunction(handler):
+            raise CompatibilityError(
+                'The handler must be an awaitable coroutine function in order '
+                'to be used safely with an ASGI app.'
+            )
 
         super().add_error_handler(exception, handler=handler)
 
@@ -551,7 +556,13 @@ def _prepare_middleware(self, middleware=None, independent_middleware=False):
             asgi=True
         )
 
-    def _python_error_handler(self, req, resp, error, params):
+    async def _http_status_handler(self, req, resp, status, params):
+        super()._http_status_handler(req, resp, status, params)
+
+    async def _http_error_handler(self, req, resp, error, params):
+        super()._http_error_handler(req, resp, error, params)
+
+    async def _python_error_handler(self, req, resp, error, params):
         falcon._logger.error('Unhandled exception in ASGI app', exc_info=error)
         self._compose_error_response(req, resp, falcon.HTTPInternalServerError())
 
@@ -575,10 +586,7 @@ async def _handle_exception(self, req, resp, ex, params):
         for err_type, err_handler in self._error_handlers:
             if isinstance(ex, err_type):
                 try:
-                    if err_handler._coroutine:
-                        await err_handler(req, resp, ex, params)
-                    else:
-                        err_handler(req, resp, ex, params)
+                    await err_handler(req, resp, ex, params)
                 except HTTPStatus as status:
                     self._compose_status_response(req, resp, status)
                 except HTTPError as error:

falcon/errors.py

@@ -47,7 +47,7 @@ class HeaderNotSupported(ValueError):
     """The specified header is not supported by this method."""
 
 
-class CompatibilityError(Exception):
+class CompatibilityError(ValueError):
     """The given method or value is not compatibile."""
 
 

falcon/util/sync.py

@@ -4,34 +4,11 @@
 
 
 __all__ = [
-    'set_coroutine_flag',
     'wrap_sync_to_async_unsafe',
     '_wrap_non_coroutine_unsafe',
 ]
 
 
-def set_coroutine_flag(func):
-    """Set a _coroutine attribute on the given function or method.
-
-    This function will detect whether a function or method is a
-    coroutine (i.e., one declared with async def), and set an
-    attribute named '_coroutine' on that function or method object,
-    accordingly.
-
-    The flag can be used to more quickly check whether the object
-    is a coroutine vs. having to call :py:meth:`inspect.iscoroutinefunction`
-    every time.
-
-    Arguments:
-        func (callable): Function or method object to flag
-    """
-
-    if inspect.ismethod(func):
-        func.__func__._coroutine = inspect.iscoroutinefunction(func)
-    else:
-        func._coroutine = inspect.iscoroutinefunction(func)
-
-
 def wrap_sync_to_async_unsafe(func):
     """Wrap a callable in a coroutine that executes the callable directly.
 

tests/test_middleware.py

@@ -849,6 +849,8 @@ def test_http_status_raised_from_error_handler(self, asgi):
         app.add_route('/', MiddlewareClassResource())
         client = testing.TestClient(app)
 
+        # NOTE(kgriffs): Use the old-style error handler signature to
+        #   ensure our shim for that works as expected.
         def _http_error_handler(error, req, resp, params):
             raise falcon.HTTPStatus(falcon.HTTP_201)