Browse Source

fix(camera): catch RuntimeError in TLS proxy forwarders for uvloop

  The bidirectional forwarders inside create_tls_proxy._handle catch
  (ConnectionError, OSError, asyncio.CancelledError) on writes, but
  uvloop's UVStream.write raises a plain RuntimeError from
  UVHandle._ensure_alive when the underlying handle is already closed.
  asyncio's default selector loop reports the same situation as
  ConnectionResetError, so the bug only surfaced on uvloop — and only at
  the moment ffmpeg (or a snapshot-capture subprocess) dropped its socket
  while the proxy was mid-flush.

  The RuntimeError slipped past the except tuple, escaped the forwarder
  coroutine, and asyncio's client_connected_cb task-exception handler
  logged a noisy multi-line traceback ending in:

      RuntimeError: unable to perform operation on
                    <TCPTransport closed=True ...>; the handler is closed

  Adds RuntimeError to the except tuple in both _fwd_to_server and
  _fwd_to_client (the latter is the actual frame from the bug report —
  server→client is where buffered TLS chunks land after the client has
  gone). The forwarders are intentionally fire-and-forget on tear-down;
  the existing dst.close() in the finally block already handles cleanup.

  No functional regression possible — the connection is already dead by
  the time the exception fires; this only changes whether asyncio logs an
  "Unhandled exception" trace for it.

  2 new regression contract tests in test_camera_tls_proxy.py use
  inspect.getsource to assert both forwarder closures' except clauses
  include RuntimeError. Source-level rather than a runtime test because
  the forwarders are nested closures inside _handle and extracting them
  just for testability would require a pure-cosmetic refactor.

  Latent since 0feed83c (Fix P2S camera TLS compatibility via OpenSSL
  proxy, #661, 2026-03-15) — only commit that ever touched these
  forwarders.
maziggy 1 month ago
parent
commit
60d0c33172

+ 12 - 2
backend/app/services/camera.py

@@ -162,6 +162,16 @@ async def create_tls_proxy(target_host: str, target_port: int) -> tuple[int, "as
             proxy_url = f"rtsp://127.0.0.1:{_local_port[0]}".encode()
             proxy_url = f"rtsp://127.0.0.1:{_local_port[0]}".encode()
             real_url = f"rtsps://{target_host}:{target_port}".encode()
             real_url = f"rtsps://{target_host}:{target_port}".encode()
 
 
+            # Note on the broad except below: dst.write() raises RuntimeError
+            # under uvloop when the underlying handle has already been torn
+            # down (uvloop.loop.UVHandle._ensure_alive). asyncio's default
+            # selector loop reports the same situation as ConnectionResetError
+            # / OSError, so a tuple that doesn't include RuntimeError leaks the
+            # uvloop variant up to asyncio's unhandled-exception logger
+            # ("Unhandled exception in client_connected_cb"). The forwarders
+            # are intentionally fire-and-forget on tear-down — once either
+            # peer drops, both halves of the proxy should exit quietly.
+
             async def _fwd_to_server(src: asyncio.StreamReader, dst: asyncio.StreamWriter):
             async def _fwd_to_server(src: asyncio.StreamReader, dst: asyncio.StreamWriter):
                 """Forward client→server, rewriting RTSP request-line URLs only."""
                 """Forward client→server, rewriting RTSP request-line URLs only."""
                 try:
                 try:
@@ -172,7 +182,7 @@ async def create_tls_proxy(target_host: str, target_port: int) -> tuple[int, "as
                         data = rewrite_rtsp_request_url(data, proxy_url, real_url)
                         data = rewrite_rtsp_request_url(data, proxy_url, real_url)
                         dst.write(data)
                         dst.write(data)
                         await dst.drain()
                         await dst.drain()
-                except (ConnectionError, OSError, asyncio.CancelledError):
+                except (ConnectionError, OSError, asyncio.CancelledError, RuntimeError):
                     pass
                     pass
                 finally:
                 finally:
                     if not dst.is_closing():
                     if not dst.is_closing():
@@ -190,7 +200,7 @@ async def create_tls_proxy(target_host: str, target_port: int) -> tuple[int, "as
                             break
                             break
                         dst.write(data)
                         dst.write(data)
                         await dst.drain()
                         await dst.drain()
-                except (ConnectionError, OSError, asyncio.CancelledError):
+                except (ConnectionError, OSError, asyncio.CancelledError, RuntimeError):
                     pass
                     pass
                 finally:
                 finally:
                     if not dst.is_closing():
                     if not dst.is_closing():

+ 48 - 0
backend/tests/unit/services/test_camera_tls_proxy.py

@@ -153,3 +153,51 @@ class TestCreateTlsProxy:
         await server.wait_closed()
         await server.wait_closed()
 
 
         assert not server.is_serving()
         assert not server.is_serving()
+
+
+class TestForwardersCatchRuntimeError:
+    """Regression contract: the bidirectional forwarders inside ``_handle``
+    must catch ``RuntimeError``, not just the connection-error tuple.
+
+    asyncio's default selector event loop reports a write-to-closed-handle as
+    ``ConnectionResetError`` / ``OSError``. uvloop (which is what runs under
+    uvicorn's ``--loop uvloop`` / when ``uvloop`` is installed) raises a plain
+    ``RuntimeError`` from ``UVHandle._ensure_alive``. If the except clause
+    drops ``RuntimeError`` the handler escapes the forwarder, asyncio's
+    ``client_connected_cb`` task-exception handler logs an "Unhandled
+    exception" stack, and the user sees noise like:
+
+        ERROR [asyncio] Unhandled exception in client_connected_cb
+        ...
+        RuntimeError: unable to perform operation on
+                      <TCPTransport closed=True ...>; the handler is closed
+
+    Regression guard for that path. Source-level check rather than a runtime
+    test because the forwarders are nested closures inside ``_handle`` and
+    extracting them just for testability would require a pure-cosmetic
+    refactor of the proxy.
+    """
+
+    def test_fwd_to_server_catches_runtime_error(self):
+        import inspect
+
+        src = inspect.getsource(create_tls_proxy)
+        fwd_section = src.split("async def _fwd_to_server")[1].split("async def _fwd_to_client")[0]
+        assert "RuntimeError" in fwd_section, (
+            "_fwd_to_server must catch RuntimeError to absorb uvloop's "
+            "write-to-closed-handle error; otherwise it leaks to "
+            "asyncio.client_connected_cb's unhandled-exception logger."
+        )
+
+    def test_fwd_to_client_catches_runtime_error(self):
+        import inspect
+
+        src = inspect.getsource(create_tls_proxy)
+        # Slice from `_fwd_to_client` to `await asyncio.gather` so we only
+        # inspect that closure's body.
+        fwd_section = src.split("async def _fwd_to_client")[1].split("await asyncio.gather")[0]
+        assert "RuntimeError" in fwd_section, (
+            "_fwd_to_client must catch RuntimeError — that's the actual frame "
+            "in the original bug report (camera.py:191 dst.write(data) under "
+            "uvloop)."
+        )