From 3427b88b9ea9f3b40f0be2a668043e98fe761c4d Mon Sep 17 00:00:00 2001 From: Adir Amsalem Date: Mon, 20 Apr 2026 13:53:28 +0300 Subject: [PATCH 1/6] fix: don't double-wrap WebRTCError raised inside connect() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The blanket `except Exception` handler in connect() was re-wrapping every WebRTCError into another WebRTCError (message=str(original), cause=original) and emitting it through on_error a second time — once from the original raise site (e.g. _handle_error), once from the except block. Catch WebRTCError separately and re-raise as-is so callers see a single, unwrapped error with no redundant cause chain. --- decart/realtime/webrtc_connection.py | 3 ++ tests/test_realtime_unit.py | 54 ++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/decart/realtime/webrtc_connection.py b/decart/realtime/webrtc_connection.py index 12f53ed..596bd16 100644 --- a/decart/realtime/webrtc_connection.py +++ b/decart/realtime/webrtc_connection.py @@ -116,6 +116,9 @@ async def connect( raise TimeoutError("Connection timeout") + except WebRTCError: + await self._set_state("disconnected") + raise except Exception as e: logger.error(f"Connection failed: {e}") await self._set_state("disconnected") diff --git a/tests/test_realtime_unit.py b/tests/test_realtime_unit.py index b748eed..448a96b 100644 --- a/tests/test_realtime_unit.py +++ b/tests/test_realtime_unit.py @@ -1354,3 +1354,57 @@ async def inject_error_soon(): ) finally: injector.cancel() + + +@pytest.mark.asyncio +async def test_connect_does_not_double_wrap_webrtc_error(): + """WebRTCError raised inside connect() re-raises as-is — no nested cause, no duplicate on_error.""" + from decart.realtime.webrtc_connection import WebRTCConnection + from decart.errors import WebRTCError + + errors: list[Exception] = [] + connection = WebRTCConnection(on_error=lambda e: errors.append(e)) + + connection._setup_peer_connection = AsyncMock() # type: ignore[assignment] + connection._create_and_send_offer = AsyncMock() # type: ignore[assignment] + + async def _noop_receive(): + await asyncio.sleep(60) + + connection._receive_messages = _noop_receive # type: ignore[assignment] + + fake_ws = MagicMock() + fake_ws.closed = False + fake_ws.close = AsyncMock() + + mock_session = MagicMock() + mock_session.closed = False + mock_session.close = AsyncMock() + mock_session.ws_connect = AsyncMock(return_value=fake_ws) + + async def inject_error_soon(): + await asyncio.sleep(0.15) + connection._connection_error = "Server at capacity. Please try again later." + + injector = asyncio.create_task(inject_error_soon()) + + try: + with patch( + "decart.realtime.webrtc_connection.aiohttp.ClientSession", + return_value=mock_session, + ): + with pytest.raises(WebRTCError) as exc_info: + await connection.connect( + url="https://example.com/ws", + local_track=None, + timeout=10.0, + ) + finally: + injector.cancel() + + assert exc_info.value.message == "Server at capacity. Please try again later." + assert not isinstance(exc_info.value.cause, WebRTCError) + assert [type(e).__name__ for e in errors] == [], ( + "on_error should not be invoked by the connect() exception handler for WebRTCError; " + f"got {errors!r}" + ) From ef2a09762a19a1909918c0d80a38b4682e308375 Mon Sep 17 00:00:00 2001 From: Adir Amsalem Date: Mon, 20 Apr 2026 13:58:48 +0300 Subject: [PATCH 2/6] fix: preserve on_error for direct-raise paths in connect() The prior fix skipped on_error for all WebRTCError exceptions in connect(), which regressed direct-raise paths (e.g. ack timeouts) that don't flow through _handle_error. Gate the skip on _connection_error being set so only the _handle_error path (which already emitted on_error) is deduped. --- decart/realtime/webrtc_connection.py | 6 +++- tests/test_realtime_unit.py | 50 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/decart/realtime/webrtc_connection.py b/decart/realtime/webrtc_connection.py index 596bd16..bafc2ad 100644 --- a/decart/realtime/webrtc_connection.py +++ b/decart/realtime/webrtc_connection.py @@ -116,8 +116,12 @@ async def connect( raise TimeoutError("Connection timeout") - except WebRTCError: + except WebRTCError as e: await self._set_state("disconnected") + # _handle_error already emitted on_error when it set _connection_error; + # only direct-raise paths (e.g., ack timeouts) still need reporting. + if self._on_error and not self._connection_error: + self._on_error(e) raise except Exception as e: logger.error(f"Connection failed: {e}") diff --git a/tests/test_realtime_unit.py b/tests/test_realtime_unit.py index 448a96b..f790002 100644 --- a/tests/test_realtime_unit.py +++ b/tests/test_realtime_unit.py @@ -1408,3 +1408,53 @@ async def inject_error_soon(): "on_error should not be invoked by the connect() exception handler for WebRTCError; " f"got {errors!r}" ) + + +@pytest.mark.asyncio +async def test_connect_direct_raise_fires_on_error_once(): + """Direct-raise WebRTCError paths (e.g. ack timeouts) must fire on_error exactly once.""" + from decart.realtime.webrtc_connection import WebRTCConnection + from decart.errors import WebRTCError + + errors: list[Exception] = [] + connection = WebRTCConnection(on_error=lambda e: errors.append(e)) + + async def _raise_ack_timeout(prompt, timeout=15.0): + raise WebRTCError("Initial prompt acknowledgment timed out") + + connection._send_initial_prompt_and_wait = _raise_ack_timeout # type: ignore[assignment] + connection._setup_peer_connection = AsyncMock() # type: ignore[assignment] + connection._create_and_send_offer = AsyncMock() # type: ignore[assignment] + + async def _noop_receive(): + await asyncio.sleep(60) + + connection._receive_messages = _noop_receive # type: ignore[assignment] + + fake_ws = MagicMock() + fake_ws.closed = False + fake_ws.close = AsyncMock() + + mock_session = MagicMock() + mock_session.closed = False + mock_session.close = AsyncMock() + mock_session.ws_connect = AsyncMock(return_value=fake_ws) + + with patch( + "decart.realtime.webrtc_connection.aiohttp.ClientSession", + return_value=mock_session, + ): + with pytest.raises(WebRTCError) as exc_info: + await connection.connect( + url="https://example.com/ws", + local_track=None, + timeout=10.0, + initial_prompt={"text": "hello", "enhance": True}, + ) + + assert exc_info.value.message == "Initial prompt acknowledgment timed out" + assert not isinstance(exc_info.value.cause, WebRTCError) + assert len(errors) == 1, ( + f"on_error should fire exactly once for direct-raise paths; got {errors!r}" + ) + assert errors[0] is exc_info.value From 966d7e7c33cb86b75839b923631baf8cbd12d0da Mon Sep 17 00:00:00 2001 From: Adir Amsalem Date: Mon, 20 Apr 2026 14:02:28 +0300 Subject: [PATCH 3/6] refactor: use explicit _on_error_fired flag for on_error dedup _connection_error was doubling as a proxy for "on_error was already fired". That coupling was implicit and would break silently if a future code path set _connection_error without firing on_error. Replace with a dedicated flag that expresses intent directly and is set at each call site. --- decart/realtime/webrtc_connection.py | 11 +++++++---- tests/test_realtime_unit.py | 10 +++++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/decart/realtime/webrtc_connection.py b/decart/realtime/webrtc_connection.py index bafc2ad..d3c7c23 100644 --- a/decart/realtime/webrtc_connection.py +++ b/decart/realtime/webrtc_connection.py @@ -62,6 +62,7 @@ def __init__( self._local_track: Optional[MediaStreamTrack] = None self._model_name: Optional[str] = None self._connection_error: Optional[str] = None + self._on_error_fired: bool = False async def connect( self, @@ -77,6 +78,7 @@ async def connect( self._local_track = local_track self._model_name = model_name self._connection_error = None + self._on_error_fired = False await self._set_state("connecting") @@ -118,15 +120,15 @@ async def connect( except WebRTCError as e: await self._set_state("disconnected") - # _handle_error already emitted on_error when it set _connection_error; - # only direct-raise paths (e.g., ack timeouts) still need reporting. - if self._on_error and not self._connection_error: + if self._on_error and not self._on_error_fired: + self._on_error_fired = True self._on_error(e) raise except Exception as e: logger.error(f"Connection failed: {e}") await self._set_state("disconnected") - if self._on_error: + if self._on_error and not self._on_error_fired: + self._on_error_fired = True self._on_error(e) raise WebRTCError(str(e), cause=e) @@ -401,6 +403,7 @@ def _handle_error(self, message: ErrorMessage) -> None: self._resolve_pending_waits(message.error) if self._on_error: + self._on_error_fired = True self._on_error(error) def register_image_set_wait(self) -> tuple[asyncio.Event, dict]: diff --git a/tests/test_realtime_unit.py b/tests/test_realtime_unit.py index f790002..52bb730 100644 --- a/tests/test_realtime_unit.py +++ b/tests/test_realtime_unit.py @@ -1384,7 +1384,11 @@ async def _noop_receive(): async def inject_error_soon(): await asyncio.sleep(0.15) + # Simulate _handle_error having run: it sets _connection_error, fires + # on_error, and marks _on_error_fired so connect() doesn't double-fire. connection._connection_error = "Server at capacity. Please try again later." + errors.append(WebRTCError("Server at capacity. Please try again later.")) + connection._on_error_fired = True injector = asyncio.create_task(inject_error_soon()) @@ -1404,9 +1408,9 @@ async def inject_error_soon(): assert exc_info.value.message == "Server at capacity. Please try again later." assert not isinstance(exc_info.value.cause, WebRTCError) - assert [type(e).__name__ for e in errors] == [], ( - "on_error should not be invoked by the connect() exception handler for WebRTCError; " - f"got {errors!r}" + assert len(errors) == 1, ( + "connect()'s WebRTCError handler must not fire on_error again when _handle_error " + f"already did; got {errors!r}" ) From f41acab8f908e989c503322a2dc7342f157c6306 Mon Sep 17 00:00:00 2001 From: Adir Amsalem Date: Mon, 20 Apr 2026 14:04:32 +0300 Subject: [PATCH 4/6] docs: explain _on_error_fired dedup contract --- decart/realtime/webrtc_connection.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/decart/realtime/webrtc_connection.py b/decart/realtime/webrtc_connection.py index d3c7c23..ba71284 100644 --- a/decart/realtime/webrtc_connection.py +++ b/decart/realtime/webrtc_connection.py @@ -62,6 +62,9 @@ def __init__( self._local_track: Optional[MediaStreamTrack] = None self._model_name: Optional[str] = None self._connection_error: Optional[str] = None + # Per-connect() dedup: _handle_error and connect()'s except branches both + # may see the same error; whichever fires first flips this to True and the + # other skips. Reset at the top of every connect() call. self._on_error_fired: bool = False async def connect( From e9ab59b6b3209430ff952bd8fd7810af69cfc2a4 Mon Sep 17 00:00:00 2001 From: Adir Amsalem Date: Mon, 20 Apr 2026 14:06:08 +0300 Subject: [PATCH 5/6] style: black format --- tests/test_realtime_unit.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_realtime_unit.py b/tests/test_realtime_unit.py index 52bb730..d13e82d 100644 --- a/tests/test_realtime_unit.py +++ b/tests/test_realtime_unit.py @@ -1458,7 +1458,7 @@ async def _noop_receive(): assert exc_info.value.message == "Initial prompt acknowledgment timed out" assert not isinstance(exc_info.value.cause, WebRTCError) - assert len(errors) == 1, ( - f"on_error should fire exactly once for direct-raise paths; got {errors!r}" - ) + assert ( + len(errors) == 1 + ), f"on_error should fire exactly once for direct-raise paths; got {errors!r}" assert errors[0] is exc_info.value From a40c2ad46c0cf70c685cddb7d09b8f6318ed316a Mon Sep 17 00:00:00 2001 From: Adir Amsalem Date: Mon, 20 Apr 2026 14:15:06 +0300 Subject: [PATCH 6/6] fix: log connect() failures for WebRTCError path too The new except WebRTCError branch was missing the logger.error call that the generic except Exception branch has, so client-side timeout paths (ack timeouts from _send_initial_{prompt,image}_and_wait) had no log entry. Server-originated errors were still logged by _handle_error. --- decart/realtime/webrtc_connection.py | 1 + 1 file changed, 1 insertion(+) diff --git a/decart/realtime/webrtc_connection.py b/decart/realtime/webrtc_connection.py index ba71284..a51e8ed 100644 --- a/decart/realtime/webrtc_connection.py +++ b/decart/realtime/webrtc_connection.py @@ -122,6 +122,7 @@ async def connect( raise TimeoutError("Connection timeout") except WebRTCError as e: + logger.error(f"Connection failed: {e}") await self._set_state("disconnected") if self._on_error and not self._on_error_fired: self._on_error_fired = True