feat(asgi): Require hooks to be coroutine functions

falcon/hooks.py

@@ -20,7 +20,7 @@
 
 from falcon import COMBINED_METHODS
 from falcon.util.misc import get_argnames
-from falcon.util.sync import set_coroutine_flag
+from falcon.util.sync import _wrap_non_coroutine_unsafe
 
 
 _DECORABLE_METHOD_NAME = re.compile(r'^on_({})(_\w+)?$'.format(
@@ -149,20 +149,16 @@ def _wrap_with_after(responder, action, action_args, action_kwargs):
     responder_argnames = get_argnames(responder)
     extra_argnames = responder_argnames[2:]  # Skip req, resp
 
-    set_coroutine_flag(action)
-
     if iscoroutinefunction(responder):
+        action = _wrap_non_coroutine_unsafe(action)
+
         @wraps(responder)
         async def do_after(self, req, resp, *args, **kwargs):
             if args:
                 _merge_responder_args(args, kwargs, extra_argnames)
 
             await responder(self, req, resp, **kwargs)
-
-            if action._coroutine:
-                await action(req, resp, self, *action_args, **action_kwargs)
-            else:
-                action(req, resp, self, *action_args, **action_kwargs)
+            await action(req, resp, self, *action_args, **action_kwargs)
     else:
         @wraps(responder)
         def do_after(self, req, resp, *args, **kwargs):
@@ -189,19 +185,15 @@ def _wrap_with_before(responder, action, action_args, action_kwargs):
     responder_argnames = get_argnames(responder)
     extra_argnames = responder_argnames[2:]  # Skip req, resp
 
-    set_coroutine_flag(action)
-
     if iscoroutinefunction(responder):
+        action = _wrap_non_coroutine_unsafe(action)
+
         @wraps(responder)
         async def do_before(self, req, resp, *args, **kwargs):
             if args:
                 _merge_responder_args(args, kwargs, extra_argnames)
 
-            if action._coroutine:
-                await action(req, resp, self, kwargs, *action_args, **action_kwargs)
-            else:
-                action(req, resp, self, kwargs, *action_args, **action_kwargs)
-
+            await action(req, resp, self, kwargs, *action_args, **action_kwargs)
             await responder(self, req, resp, **kwargs)
     else:
         @wraps(responder)

falcon/routing/compiled.py

@@ -248,7 +248,7 @@ def _check_coroutine_responders(self, method_map):
         #   for WSGI and ASGI. It must be used with care to avoid
         #   false-positive mismatch issues (for example, do not use
         #   an async hook with a synchronous responder.)
-        should_wrap = 'FALCON_ASGI_WRAP_RESPONDERS' in os.environ
+        should_wrap = 'FALCON_ASGI_WRAP_NON_COROUTINES' in os.environ
 
         for method, responder in method_map.items():
             # NOTE(kgriffs): We don't simply wrap non-async functions

falcon/util/sync.py

@@ -1,11 +1,17 @@
+from functools import wraps
 import inspect
+import os
 
 
-__all__ = ['set_coroutine_flag']
+__all__ = [
+    'set_coroutine_flag',
+    'sync_to_async_unsafe',
+    '_wrap_non_coroutine_unsafe',
+]
 
 
-def set_coroutine_flag(func_or_meth):
-    """Set a _coroutine attribute on the given func or method.
+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
@@ -15,9 +21,57 @@ def set_coroutine_flag(func_or_meth):
     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_or_meth):
-        func_or_meth.__func__._coroutine = inspect.iscoroutinefunction(func_or_meth)
+    if inspect.ismethod(func):
+        func.__func__._coroutine = inspect.iscoroutinefunction(func)
     else:
-        func_or_meth._coroutine = inspect.iscoroutinefunction(func_or_meth)
+        func._coroutine = inspect.iscoroutinefunction(func)
+
+
+def sync_to_async_unsafe(func):
+    """Wrap a callable in a coroutine that executes the callable directly.
+
+    This helper is only to be used for functions that do not perform any
+    blocking I/O or lengthy CPU-bound operations, since the entire async
+    loop will be blocked while the wrapped function is executed.
+
+    For a safer, non-blocking alternative that runs the function in a
+    thread pool executor, use :func:~.sync_to_async instead.
+
+    Arguments:
+        func (callable): Function, method, or other callable to wrap
+    """
+    @wraps(func)
+    async def wrapper(*args, **kwargs):
+        return func(*args, **kwargs)
+
+    return wrapper
+
+
+def _wrap_non_coroutine_unsafe(func):
+    """Wrap a coroutine using ``sync_to_async_unsafe()`` for internal test cases.
+
+    This method is intended for Falcon's own test sweet and should not be
+    used by apps themselves. It provides a convenient way to reuse sync
+    methods for ASGI test cases when it is safe to do so.
+
+    Arguments:
+        func (callable): Function, method, or other callable to wrap
+
+    Returns:
+        When not in test mode, this function simply returns the callable
+        unchanged. Otherwise, if the callable is not a coroutine function,
+        it will be wrapped using ``sync_to_async_unsafe()``.
+    """
+
+    if 'FALCON_ASGI_WRAP_NON_COROUTINES' not in os.environ:
+        return func
+
+    if inspect.iscoroutinefunction(func):
+        return func
+
+    return sync_to_async_unsafe(func)

tests/_util.py

@@ -1,3 +1,6 @@
+from contextlib import contextmanager
+import os
+
 import pytest
 
 import falcon
@@ -71,3 +74,15 @@ async def wrapper(*args, **kwargs):
 def skipif_asgi_unsupported():
     if not ASGI_SUPPORTED:
         pytest.skip('ASGI requires CPython or PyPy 3.6+')
+
+
+@contextmanager
+def disable_asgi_non_coroutine_wrapping():
+    should_wrap = 'FALCON_ASGI_WRAP_NON_COROUTINES' in os.environ
+    if should_wrap:
+        del os.environ['FALCON_ASGI_WRAP_NON_COROUTINES']
+
+    yield
+
+    if should_wrap:
+        os.environ['FALCON_ASGI_WRAP_NON_COROUTINES'] = 'Y'

tests/asgi/test_hello_asgi.py

@@ -1,12 +1,13 @@
 import io
-import os
 
 import pytest
 
 import falcon
 from falcon import testing
 import falcon.asgi
 
+from _util import disable_asgi_non_coroutine_wrapping  # NOQA
+
 
 @pytest.fixture
 def client():
@@ -252,14 +253,8 @@ def test_status_not_set(self, client):
         assert result.status_code == 200
 
     def test_coroutine_required(self, client):
-        should_wrap = 'FALCON_ASGI_WRAP_RESPONDERS' in os.environ
-        if should_wrap:
-            del os.environ['FALCON_ASGI_WRAP_RESPONDERS']
-
-        with pytest.raises(TypeError) as exinfo:
-            client.app.add_route('/', NonCoroutineResource())
-
-        assert 'responder must be a non-blocking async coroutine' in str(exinfo.value)
+        with disable_asgi_non_coroutine_wrapping():
+            with pytest.raises(TypeError) as exinfo:
+                client.app.add_route('/', NonCoroutineResource())
 
-        if should_wrap:
-            os.environ['FALCON_ASGI_WRAP_RESPONDERS'] = 'Y'
+            assert 'responder must be a non-blocking async coroutine' in str(exinfo.value)

tests/conftest.py

@@ -15,12 +15,6 @@ def asgi(request):
     return is_asgi
 
 
-@pytest.fixture(autouse=True, scope='session')
-def set_env_variables():
-    os.environ['FALCON_TESTING_SESSION'] = 'Y'
-    os.environ['FALCON_ASGI_WRAP_RESPONDERS'] = 'Y'
-
-
 # NOTE(kgriffs): Some modules actually run a wsgiref server, so
 # to ensure we reset the detection for the other modules, we just
 # run this fixture before each one is tested.

tests/test_before_hooks.py

@@ -7,7 +7,7 @@
 import falcon
 import falcon.testing as testing
 
-from _util import create_app, create_resp  # NOQA
+from _util import create_app, create_resp, disable_asgi_non_coroutine_wrapping  # NOQA
 
 
 def validate(req, resp, resource, params):
@@ -25,6 +25,10 @@ def validate_param(req, resp, resource, params, param_name, maxval=100):
         raise falcon.HTTPBadRequest('Out of Range', msg)
 
 
+async def validate_param_async(*args, **kwargs):
+    validate_param(*args, **kwargs)
+
+
 class ResourceAwareValidateParam:
     def __call__(self, req, resp, resource, params):
         assert resource
@@ -134,16 +138,6 @@ def on_get(self, req, resp, doc=None):
         self.doc = doc
 
 
-class WrappedRespondersBodyParserAsyncResource:
-
-    @falcon.before(validate_param, 'limit', 100)
-    @falcon.before(parse_body_async)
-    async def on_get(self, req, resp, doc=None):
-        self.req = req
-        self.resp = resp
-        self.doc = doc
-
-
 @falcon.before(bunnies)
 class WrappedClassResource:
 
@@ -360,6 +354,15 @@ def test_parser_sync(body, doc):
     ]
 )
 def test_parser_async(body, doc):
+    with disable_asgi_non_coroutine_wrapping():
+        class WrappedRespondersBodyParserAsyncResource:
+            @falcon.before(validate_param_async, 'limit', 100)
+            @falcon.before(parse_body_async)
+            async def on_get(self, req, resp, doc=None):
+                self.req = req
+                self.resp = resp
+                self.doc = doc
+
     app = create_app(asgi=True)
 
     resource = WrappedRespondersBodyParserAsyncResource()

tests/test_validators.py

@@ -183,10 +183,10 @@ def test_both_schemas_validation_failure(asgi):
     should_wrap = 'FALCON_ASGI_WRAP_RESPONDERS' in os.environ
 
     if should_wrap:
-        del os.environ['FALCON_ASGI_WRAP_RESPONDERS']
+        del os.environ['FALCON_ASGI_WRAP_NON_COROUTINES']
     client.app.add_route('/test', resource)
     if should_wrap:
-        os.environ['FALCON_ASGI_WRAP_RESPONDERS'] = 'y'
+        os.environ['FALCON_ASGI_WRAP_NON_COROUTINES'] = 'y'
 
     result = client.simulate_put('/test', json=_INVALID_MEDIA)
     assert result.status_code == 400

tox.ini

@@ -25,6 +25,8 @@ envlist = py38,
 [testenv]
 setenv =
     PIP_CONFIG_FILE={toxinidir}/pip.conf
+    FALCON_ASGI_WRAP_NON_COROUTINES=Y
+    FALCON_TESTING_SESSION=Y
 deps = -r{toxinidir}/requirements/tests
 commands = {toxinidir}/tools/clean.sh {toxinidir}/falcon
            pytest tests []