Repository: falconry/falcon

Issue:
Improve docs and checks for sink when using ASGI
Currently there is no documentation or checks in `add_sink` that require an async function when using the asgi app.

This is a follow up of https://github.com/falconry/falcon/pull/1769#discussion_r531449969

cc @vytas7 

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1829?src=pr&el=h1) Report
> Merging [#1829](https://codecov.io/gh/falconry/falcon/pull/1829?src=pr&el=desc) (249b770) into [master](https://codecov.io/gh/falconry/falcon/commit/ba78a52b179e07428039a583f08f3f7e853e0a5d?el=desc) (ba78a52) will **decrease** coverage by `65.55%`.
> The diff coverage is `50.00%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1829/graphs/tree.svg?width=650&height=150&src=pr&token=zOtkfNQ3h6)](https://codecov.io/gh/falconry/falcon/pull/1829?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##            master    #1829       +/-   ##
============================================
- Coverage   100.00%   34.44%   -65.56%     
============================================
  Files           54       54               
  Lines         5128     5135        +7     
  Branches       823      825        +2     
============================================
- Hits          5128     1769     -3359     
- Misses           0     3326     +3326     
- Partials         0       40       +40     
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1829?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/asgi/app.py](https://codecov.io/gh/falconry/falcon/pull/1829/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvYXBwLnB5) | `19.63% <50.00%> (-80.37%)` | :arrow_down: |
| [falcon/util/reader.py](https://codecov.io/gh/falconry/falcon/pull/1829/diff?src=pr&el=tree#diff-ZmFsY29uL3V0aWwvcmVhZGVyLnB5) | `12.62% <0.00%> (-87.38%)` | :arrow_down: |
| [falcon/asgi/stream.py](https://codecov.io/gh/falconry/falcon/pull/1829/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvc3RyZWFtLnB5) | `13.58% <0.00%> (-86.42%)` | :arrow_down: |
| [falcon/asgi/structures.py](https://codecov.io/gh/falconry/falcon/pull/1829/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvc3RydWN0dXJlcy5weQ==) | `14.58% <0.00%> (-85.42%)` | :arrow_down: |
| [falcon/middleware.py](https://codecov.io/gh/falconry/falcon/pull/1829/diff?src=pr&el=tree#diff-ZmFsY29uL21pZGRsZXdhcmUucHk=) | `14.89% <0.00%> (-85.11%)` | :arrow_down: |
| [falcon/asgi/reader.py](https://codecov.io/gh/falconry/falcon/pull/1829/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVhZGVyLnB5) | `16.00% <0.00%> (-84.00%)` | :arrow_down: |
| [falcon/testing/helpers.py](https://codecov.io/gh/falconry/falcon/pull/1829/diff?src=pr&el=tree#diff-ZmFsY29uL3Rlc3RpbmcvaGVscGVycy5weQ==) | `18.60% <0.00%> (-81.40%)` | :arrow_down: |
| [falcon/asgi/ws.py](https://codecov.io/gh/falconry/falcon/pull/1829/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvd3MucHk=) | `22.66% <0.00%> (-77.34%)` | :arrow_down: |
| [falcon/asgi/request.py](https://codecov.io/gh/falconry/falcon/pull/1829/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVxdWVzdC5weQ==) | `23.69% <0.00%> (-76.31%)` | :arrow_down: |
| ... and [37 more](https://codecov.io/gh/falconry/falcon/pull/1829/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1829?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1829?src=pr&el=footer). Last update [ba78a52...249b770](https://codecov.io/gh/falconry/falcon/pull/1829?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/falcon/app.py b/falcon/app.py
index 2ee701e..8f639cb 100644
--- a/falcon/app.py
+++ b/falcon/app.py
@@ -21,7 +21,7 @@ import traceback
 
 from falcon import app_helpers as helpers, routing
 import falcon.constants
-from falcon.errors import HTTPBadRequest
+from falcon.errors import CompatibilityError, HTTPBadRequest
 from falcon.http_error import HTTPError
 from falcon.http_status import HTTPStatus
 from falcon.middleware import CORSMiddleware
@@ -505,6 +505,10 @@ class App:
                 corresponding request handlers, and Falcon will do the right
                 thing.
 
+                Note:
+                    When using an async version of the ``App``, all request
+                    handlers must be awaitable coroutine functions.
+
         Keyword Args:
             suffix (str): Optional responder name suffix for this route. If
                 a suffix is provided, Falcon will map GET requests to
@@ -637,6 +641,12 @@ class App:
 
         """
 
+        if not self._ASGI and iscoroutinefunction(sink):
+            raise CompatibilityError(
+                'The sink method must be a regular synchronous function '
+                'in order to be used with a WSGI app.'
+            )
+
         if not hasattr(prefix, 'match'):
             # Assume it is a string
             prefix = re.compile(prefix)
diff --git a/falcon/asgi/app.py b/falcon/asgi/app.py
index 2115577..e0ff2dc 100644
--- a/falcon/asgi/app.py
+++ b/falcon/asgi/app.py
@@ -34,7 +34,12 @@ from falcon.http_status import HTTPStatus
 from falcon.media.multipart import MultipartFormHandler
 import falcon.routing
 from falcon.util.misc import http_status_to_code, is_python_func
-from falcon.util.sync import _wrap_non_coroutine_unsafe, get_running_loop
+from falcon.util.sync import (
+    _should_wrap_non_coroutines,
+    _wrap_non_coroutine_unsafe,
+    get_running_loop,
+    wrap_sync_to_async,
+)
 from .multipart import MultipartForm
 from .request import Request
 from .response import Response
@@ -682,6 +687,20 @@ class App(falcon.app.App):
 
     add_route.__doc__ = falcon.app.App.add_route.__doc__
 
+    def add_sink(self, sink, prefix=r'/'):
+        if not iscoroutinefunction(sink) and is_python_func(sink):
+            if _should_wrap_non_coroutines():
+                sink = wrap_sync_to_async(sink)
+            else:
+                raise CompatibilityError(
+                    'The sink method must be an awaitable coroutine function '
+                    'in order to be used safely with an ASGI app.'
+                )
+
+        super().add_sink(sink, prefix=prefix)
+
+    add_sink.__doc__ = falcon.app.App.add_sink.__doc__
+
     def add_error_handler(self, exception, handler=None):
         """Register a handler for one or more exception types.
 


Test patch:
diff --git a/tests/test_inspect.py b/tests/test_inspect.py
index 575547b..4dbe24a 100644
--- a/tests/test_inspect.py
+++ b/tests/test_inspect.py
@@ -146,10 +146,12 @@ class TestInspectApp:
         assert all(isinstance(s, inspect.SinkInfo) for s in sinks)
         assert sinks[-1].prefix == '/sink_fn'
         assert sinks[-1].name == 'sinkFn'
-        assert '_inspect_fixture.py' in sinks[-1].source_info
+        if not asgi:
+            assert '_inspect_fixture.py' in sinks[-1].source_info
         assert sinks[-2].prefix == '/sink_cls'
         assert sinks[-2].name == 'SinkClass'
-        assert '_inspect_fixture.py' in sinks[-2].source_info
+        if not asgi:
+            assert '_inspect_fixture.py' in sinks[-2].source_info
 
     @pytest.mark.skipif(sys.version_info < (3, 6), reason='dict order is not stable')
     def test_error_handler(self, asgi):
diff --git a/tests/test_sinks.py b/tests/test_sinks.py
index f151533..1694ea8 100644
--- a/tests/test_sinks.py
+++ b/tests/test_sinks.py
@@ -5,7 +5,7 @@ import pytest
 import falcon
 import falcon.testing as testing
 
-from _util import create_app  # NOQA
+from _util import create_app, disable_asgi_non_coroutine_wrapping  # NOQA
 
 
 class Proxy:
@@ -27,6 +27,14 @@ class SinkAsync(Sink):
         super().__call__(req, resp, **kwargs)
 
 
+def kitchen_sink(req, resp, **kwargs):
+    resp.set_header('X-Missing-Feature', 'kitchen-sink')
+
+
+async def async_kitchen_sink(req, resp, **kwargs):
+    kitchen_sink(req, resp, **kwargs)
+
+
 class BookCollection(testing.SimpleTestResource):
     pass
 
@@ -137,3 +145,32 @@ class TestDefaultRouting:
         response = client.simulate_request(path='/books/123')
         assert resource.called
         assert response.status == falcon.HTTP_200
+
+
+class TestSinkMethodCompatibility:
+
+    def _verify_kitchen_sink(self, client):
+        resp = client.simulate_request('BREW', '/features')
+        assert resp.status_code == 200
+        assert resp.headers.get('X-Missing-Feature') == 'kitchen-sink'
+
+    def test_add_async_sink(self, client, asgi):
+        if not asgi:
+            with pytest.raises(falcon.CompatibilityError):
+                client.app.add_sink(async_kitchen_sink)
+        else:
+            client.app.add_sink(async_kitchen_sink, '/features')
+            self._verify_kitchen_sink(client)
+
+    def test_add_sync_sink(self, client, asgi):
+        if asgi:
+            with disable_asgi_non_coroutine_wrapping():
+                with pytest.raises(falcon.CompatibilityError):
+                    client.app.add_sink(kitchen_sink)
+        else:
+            client.app.add_sink(kitchen_sink, '/features')
+            self._verify_kitchen_sink(client)
+
+    def test_add_sync_sink_with_wrapping(self, client, asgi):
+        client.app.add_sink(kitchen_sink, '/features')
+        self._verify_kitchen_sink(client)