Repository: falconry/falcon

Issue:
Websocket handler fails in some cases of HTTPError/HTTPStatus
Hi,

Writing ASGI falcon using websockets, using the ```process_request_ws``` middleware handler and ```app.add_error_handler(HTTPError, my_error_handler)``` error handler.

When raising a ```falcon.HTTPError``` in the middleware its picked up by the error handler via the ```ex``` param. 
The object is not none, however this is my stacktrace: <br/>

I raise the "falcon.http_error.HTTPError: <HTTPError: 403 Forbidden>" in the middleware, and then raise the "falcon.http_error.HTTPError: <HTTPError: 500>" in the error handler, which results in the final traceback (falcon internal).


```bash
'''
[2023-03-29 13:39:47 +0200] [82450] [INFO] ('127.0.0.1', 57224) - "WebSocket /message" 403
[2023-03-29 13:39:47 +0200] [82450] [ERROR] Exception in ASGI application
Traceback (most recent call last):
  File ".pyenv/versions/wm/lib/python3.11/site-packages/falcon/asgi/app.py", line 1000, in _handle_websocket
    await process_request_ws(req, web_socket)
  File "Desktop/mirror/whatsapp-service/whatsapp_service/middleware/basic_ws_middleware.py", line 106, in process_request_ws
    raise HTTPError(status=falcon.HTTP_403, title="Invalid header")
falcon.http_error.HTTPError: <HTTPError: 403 Forbidden>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File ".pyenv/versions/wm/lib/python3.11/site-packages/falcon/asgi/app.py", line 1095, in _handle_exception
    await err_handler(req, resp, ex, params, **kwargs)
  File "Desktop/mirror/whatsapp-service/whatsapp_service/utils/handlers/error_handlers.py", line 24, in my_error_handler
    raise HTTPError(status=500, title='Custom Websocket Error')
falcon.http_error.HTTPError: <HTTPError: 500>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File ".pyenv/versions/wm/lib/python3.11/site-packages/uvicorn/protocols/websockets/websockets_impl.py", line 238, in run_asgi
    result = await self.app(self.scope, self.asgi_receive, self.asgi_send)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".pyenv/versions/wm/lib/python3.11/site-packages/uvicorn/middleware/proxy_headers.py", line 78, in __call__
    return await self.app(scope, receive, send)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".pyenv/versions/wm/lib/python3.11/site-packages/falcon/asgi/app.py", line 312, in __call__
    await self._handle_websocket(spec_version, scope, receive, send)
  File ".pyenv/versions/wm/lib/python3.11/site-packages/falcon/asgi/app.py", line 1018, in _handle_websocket
    if not await self._handle_exception(req, None, ex, params, ws=web_socket):
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".pyenv/versions/wm/lib/python3.11/site-packages/falcon/asgi/app.py", line 1100, in _handle_exception
    self._compose_error_response(req, resp, error)
  File "falcon/app.py", line 975, in falcon.app.App._compose_error_response
'''

```
The usage of the method resolution order is incorrect here as the ```type(exc)``` invocation returns type not the exception: 
https://github.com/falconry/falcon/blob/18503813059e648c693d03064adf2acdbe68d654/falcon/app.py#L1001



Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/falconry/falcon/pull/2150?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry) Report
> Merging [#2150](https://app.codecov.io/gh/falconry/falcon/pull/2150?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry) (85de46b) into [master](https://app.codecov.io/gh/falconry/falcon/commit/d2981f684c91598f229e6c2fa2408a3a857da0da?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry) (d2981f6) will **decrease** coverage by `0.03%`.
> The diff coverage is `100.00%`.

```diff
@@             Coverage Diff             @@
##            master    #2150      +/-   ##
===========================================
- Coverage   100.00%   99.97%   -0.03%     
===========================================
  Files           62       62              
  Lines         6789     6797       +8     
  Branches      1095     1094       -1     
===========================================
+ Hits          6789     6795       +6     
- Misses           0        2       +2     
```


| [Impacted Files](https://app.codecov.io/gh/falconry/falcon/pull/2150?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry) | Coverage Δ | |
|---|---|---|
| [falcon/asgi/app.py](https://app.codecov.io/gh/falconry/falcon/pull/2150?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry#diff-ZmFsY29uL2FzZ2kvYXBwLnB5) | `100.00% <100.00%> (ø)` | |
| [falcon/asgi/ws.py](https://app.codecov.io/gh/falconry/falcon/pull/2150?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry#diff-ZmFsY29uL2FzZ2kvd3MucHk=) | `100.00% <100.00%> (ø)` | |
| [falcon/util/misc.py](https://app.codecov.io/gh/falconry/falcon/pull/2150?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry#diff-ZmFsY29uL3V0aWwvbWlzYy5weQ==) | `98.62% <100.00%> (-1.38%)` | :arrow_down: |


CaselIT: thanks. I'll look into adding it and also by the coverage is not happy.
CaselIT: thanks for fixing the coverage, I guess the old code was covering that part by chance

Patch:
diff --git a/docs/_newsfragments/2146.bugfix.rst b/docs/_newsfragments/2146.bugfix.rst
new file mode 100644
index 0000000..ab27e52
--- /dev/null
+++ b/docs/_newsfragments/2146.bugfix.rst
@@ -0,0 +1,6 @@
+
+:ref:`WebSocket <ws>` implementation has been fixed to properly handle
+:class:`~falcon.HTTPError` and :class:`~falcon.HTTPStatus` exceptions raised by
+custom :func:`error handlers <falcon.asgi.App.add_error_handler>`.
+The WebSocket connection is now correctly closed with an appropriate code
+instead of bubbling up an unhandled error to the application server.
diff --git a/falcon/asgi/app.py b/falcon/asgi/app.py
index a4cf656..b7dbc94 100644
--- a/falcon/asgi/app.py
+++ b/falcon/asgi/app.py
@@ -44,6 +44,7 @@ from .multipart import MultipartForm
 from .request import Request
 from .response import Response
 from .structures import SSEvent
+from .ws import http_status_to_ws_code
 from .ws import WebSocket
 from .ws import WebSocketOptions
 
@@ -1025,30 +1026,45 @@ class App(falcon.app.App):
             asgi=True,
         )
 
-    async def _http_status_handler(self, req, resp, status, params):
-        self._compose_status_response(req, resp, status)
+    async def _http_status_handler(self, req, resp, status, params, ws=None):
+        if resp:
+            self._compose_status_response(req, resp, status)
+        elif ws:
+            code = http_status_to_ws_code(status.status)
+            falcon._logger.error(
+                '[FALCON] HTTPStatus %s raised while handling WebSocket. '
+                'Closing with code %s',
+                status,
+                code,
+            )
+            await ws.close(code)
+        else:
+            raise NotImplementedError('resp or ws expected')
 
     async def _http_error_handler(self, req, resp, error, params, ws=None):
         if resp:
             self._compose_error_response(req, resp, error)
-
-        if ws:
+        elif ws:
+            code = http_status_to_ws_code(error.status_code)
             falcon._logger.error(
-                '[FALCON] WebSocket handshake rejected due to raised HTTP error: %s',
+                '[FALCON] HTTPError %s raised while handling WebSocket. '
+                'Closing with code %s',
                 error,
+                code,
             )
-
-            code = 3000 + error.status_code
             await ws.close(code)
+        else:
+            raise NotImplementedError('resp or ws expected')
 
     async def _python_error_handler(self, req, resp, error, params, ws=None):
         falcon._logger.error('[FALCON] Unhandled exception in ASGI app', exc_info=error)
 
         if resp:
             self._compose_error_response(req, resp, falcon.HTTPInternalServerError())
-
-        if ws:
+        elif ws:
             await self._ws_cleanup_on_error(ws)
+        else:
+            raise NotImplementedError('resp or ws expected')
 
     async def _ws_disconnected_error_handler(self, req, resp, error, params, ws):
         falcon._logger.debug(
@@ -1093,9 +1109,9 @@ class App(falcon.app.App):
                 await err_handler(req, resp, ex, params, **kwargs)
 
             except HTTPStatus as status:
-                self._compose_status_response(req, resp, status)
+                await self._http_status_handler(req, resp, status, params, ws=ws)
             except HTTPError as error:
-                self._compose_error_response(req, resp, error)
+                await self._http_error_handler(req, resp, error, params, ws=ws)
 
             return True
 
diff --git a/falcon/asgi/ws.py b/falcon/asgi/ws.py
index 19dd2f2..e7db3d5 100644
--- a/falcon/asgi/ws.py
+++ b/falcon/asgi/ws.py
@@ -701,3 +701,8 @@ class _BufferedReceiver:
             if self._pop_message_waiter is not None:
                 self._pop_message_waiter.set_result(None)
                 self._pop_message_waiter = None
+
+
+def http_status_to_ws_code(http_status: int) -> int:
+    """Convert the provided http status to a websocket close code by adding 3000."""
+    return http_status + 3000
diff --git a/falcon/util/misc.py b/falcon/util/misc.py
index 6422a81..d20fd45 100644
--- a/falcon/util/misc.py
+++ b/falcon/util/misc.py
@@ -458,10 +458,10 @@ def code_to_http_status(status):
 
     try:
         code = int(status)
-        if not 100 <= code <= 999:
-            raise ValueError('{} is not a valid status code'.format(status))
     except (ValueError, TypeError):
         raise ValueError('{!r} is not a valid status code'.format(status))
+    if not 100 <= code <= 999:
+        raise ValueError('{} is not a valid status code'.format(status))
 
     try:
         # NOTE(kgriffs): We do this instead of using http.HTTPStatus since


Test patch:
diff --git a/tests/asgi/test_misc.py b/tests/asgi/test_misc.py
new file mode 100644
index 0000000..1a609db
--- /dev/null
+++ b/tests/asgi/test_misc.py
@@ -0,0 +1,32 @@
+# misc test for 100% coverage
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from falcon.asgi import App
+from falcon.http_error import HTTPError
+from falcon.http_status import HTTPStatus
+
+
+@pytest.mark.asyncio
+async def test_http_status_not_impl():
+    app = App()
+    with pytest.raises(NotImplementedError):
+        await app._http_status_handler(MagicMock(), None, HTTPStatus(200), {}, None)
+
+
+@pytest.mark.asyncio
+async def test_http_error_not_impl():
+    app = App()
+    with pytest.raises(NotImplementedError):
+        await app._http_error_handler(MagicMock(), None, HTTPError(400), {}, None)
+
+
+@pytest.mark.asyncio
+async def test_python_error_not_impl():
+    app = App()
+    with pytest.raises(NotImplementedError):
+        await app._python_error_handler(
+            MagicMock(), None, ValueError('error'), {}, None
+        )
diff --git a/tests/asgi/test_ws.py b/tests/asgi/test_ws.py
index bcce2cf..c96ed3e 100644
--- a/tests/asgi/test_ws.py
+++ b/tests/asgi/test_ws.py
@@ -1092,3 +1092,126 @@ def test_msgpack_missing():
 
     with pytest.raises(RuntimeError):
         handler.deserialize(b'{}')
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize('status', [200, 500, 422, 400])
+@pytest.mark.parametrize('thing', [falcon.HTTPStatus, falcon.HTTPError])
+@pytest.mark.parametrize('accept', [True, False])
+async def test_ws_http_error_or_status_response(conductor, status, thing, accept):
+    class Resource:
+        async def on_websocket(self, req, ws):
+            if accept:
+                await ws.accept()
+            raise thing(status)
+
+    conductor.app.add_route('/', Resource())
+    exp_code = 3000 + status
+
+    async with conductor as c:
+        if accept:
+            async with c.simulate_ws() as ws:
+                assert ws.closed
+                assert ws.close_code == exp_code
+        else:
+            with pytest.raises(falcon.WebSocketDisconnected) as err:
+                async with c.simulate_ws():
+                    pass
+            assert err.value.code == exp_code
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize('status', [200, 500, 422, 400])
+@pytest.mark.parametrize(
+    'thing',
+    [
+        falcon.HTTPStatus,
+        falcon.HTTPError,
+    ],
+)
+@pytest.mark.parametrize('place', ['request', 'resource'])
+async def test_ws_http_error_or_status_middleware(conductor, status, thing, place):
+    called = False
+
+    class Resource:
+        async def on_websocket(self, req, ws):
+            nonlocal called
+            called = True
+
+    class Middleware:
+        async def process_request_ws(self, req, ws):
+            if place == 'request':
+                raise thing(status)
+
+        async def process_resource_ws(self, req, ws, res, params):
+            if place == 'resource':
+                raise thing(status)
+
+    conductor.app.add_route('/', Resource())
+    conductor.app.add_middleware(Middleware())
+    exp_code = 3000 + status
+
+    async with conductor as c:
+        with pytest.raises(falcon.WebSocketDisconnected) as err:
+            async with c.simulate_ws():
+                pass
+        assert err.value.code == exp_code
+    assert not called
+
+
+class FooBarError(Exception):
+    pass
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize('status', [200, 500, 422, 400])
+@pytest.mark.parametrize('thing', [falcon.HTTPStatus, falcon.HTTPError])
+@pytest.mark.parametrize(
+    'place', ['request', 'resource', 'ws_before_accept', 'ws_after_accept']
+)
+@pytest.mark.parametrize('handler_has_ws', [True, False])
+async def test_ws_http_error_or_status_error_handler(
+    conductor, status, thing, place, handler_has_ws
+):
+    class Resource:
+        async def on_websocket(self, req, ws):
+            if place == 'ws_before_accept':
+                raise FooBarError
+            await ws.accept()
+            if place == 'ws_after_accept':
+                raise FooBarError
+
+    class Middleware:
+        async def process_request_ws(self, req, ws):
+            if place == 'request':
+                raise FooBarError
+
+        async def process_resource_ws(self, req, ws, res, params):
+            if place == 'resource':
+                raise FooBarError
+
+    if handler_has_ws:
+
+        async def handle_foobar(req, resp, ex, param, ws=None):   # type: ignore
+            raise thing(status)
+
+    else:
+
+        async def handle_foobar(req, resp, ex, param):  # type: ignore
+            raise thing(status)
+
+    conductor.app.add_route('/', Resource())
+    conductor.app.add_middleware(Middleware())
+    conductor.app.add_error_handler(FooBarError, handle_foobar)
+    exp_code = 3000 + status
+
+    async with conductor as c:
+        if place == 'ws_after_accept':
+            async with c.simulate_ws() as ws:
+                assert ws.closed
+                assert ws.close_code == exp_code
+        else:
+            with pytest.raises(falcon.WebSocketDisconnected) as err:
+                async with c.simulate_ws():
+                    pass
+            assert err.value.code == exp_code
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 3de05c4..83a0e1f 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -522,7 +522,23 @@ class TestFalconUtils:
     def test_code_to_http_status(self, v_in, v_out):
         assert falcon.code_to_http_status(v_in) == v_out
 
-    @pytest.mark.parametrize('v', [0, 13, 99, 1000, 1337.01, -99, -404.3, -404, -404.3])
+    @pytest.mark.parametrize(
+        'v',
+        [
+            0,
+            13,
+            99,
+            1000,
+            1337.01,
+            -99,
+            -404.3,
+            -404,
+            -404.3,
+            'Successful',
+            'Failed',
+            None,
+        ],
+    )
     def test_code_to_http_status_value_error(self, v):
         with pytest.raises(ValueError):
             falcon.code_to_http_status(v)