diff --git a/lib/main.py b/lib/main.py index 4a4b2198..5bcda118 100644 --- a/lib/main.py +++ b/lib/main.py @@ -1094,7 +1094,7 @@ def _start_recording(self, language_override=None): self._hide_mic_osd() self._stop_audio_level_monitoring() self._notify_zero_volume( - "Realtime backend not connected yet — try again in a moment.", + self._realtime_unavailable_message(), log_level="ERROR", ) # Restore audio if it was ducked @@ -1540,6 +1540,20 @@ def _mic_failure_message(self, fallback: str) -> str: return "Microphone unavailable - input device missing or still initializing - check the connection and try again" return fallback + def _realtime_unavailable_message(self) -> str: + """Explain why realtime couldn't start, based on what recovery actually hit. + + "try again in a moment" is only true while a handshake is in flight; for a + failed connect it invites the user to keep retrying against a dead endpoint. + """ + reason = self.whisper_manager.realtime_connect_failure() + if reason == 'failed': + return "Realtime connection failed — check network or provider status" + if reason == 'cooldown': + # Nothing retries in the background — the next attempt is the user's. + return "Realtime connection failed — try again in a few seconds" + return "Realtime backend not connected yet — try again in a moment." + def _notify_zero_volume(self, message: str, log_level: str = "WARN"): """Log a mic failure, signal waybar, and show a coalesced desktop notification""" # Prevent duplicate handling of the same error within 2 seconds (user diff --git a/lib/src/backends/realtime_ws_backend.py b/lib/src/backends/realtime_ws_backend.py index 8f29dd11..7303344c 100644 --- a/lib/src/backends/realtime_ws_backend.py +++ b/lib/src/backends/realtime_ws_backend.py @@ -45,6 +45,9 @@ class RealtimeWsBackend(TranscriptionBackend): is_local = False reinit_on_resume = True + # Don't rebuild a torn-down client on every keypress while an endpoint is down. + REBUILD_COOLDOWN_SECS = 5.0 + def __init__(self, manager): super().__init__(manager) # Realtime WebSocket client @@ -53,6 +56,10 @@ def __init__(self, manager): # Connection parameters used for reconnect-on-demand. # (Stored in-memory only; do not log API keys.) self._realtime_connect_params = None + # Why the last recovery attempt failed, for an honest user-facing message: + # 'connecting' | 'cooldown' | 'failed' | None + self._last_connect_failure = None + self._last_rebuild_attempt = None @property def _realtime_partial_callback(self): @@ -415,18 +422,19 @@ def get_streaming_callback(self) -> Optional[Callable]: backend = self.config.get_setting('transcription_backend', 'pywhispercpp') backend = normalize_backend(backend) - if backend == 'realtime-ws' and self._realtime_client: - # If the server closed the socket while idle, reconnect on-demand here - # (before we start capturing audio) to avoid dropping the first chunks. - if not self._realtime_client.connected: - if not self._reconnect_realtime_client(): - return None - - # Clear server buffer before starting new recording - self._realtime_client.clear_audio_buffer() - self._clear_realtime_partial_preview() - return self._realtime_streaming_callback - return None + if backend != 'realtime-ws': + return None + + # Recover here — before we start capturing audio — so the first chunks + # aren't dropped, whether the socket went idle or the client was torn + # down entirely by an earlier failure. + if not self._ensure_client(): + return None + + # Clear server buffer before starting new recording + self._realtime_client.clear_audio_buffer() + self._clear_realtime_partial_preview() + return self._realtime_streaming_callback def apply_partial_callback(self, callback: Optional[Callable[[str], None]]) -> None: """Apply the partial-preview callback to the active realtime provider.""" @@ -484,6 +492,53 @@ def _clear_realtime_partial_preview(self) -> None: except Exception as e: print(f'[REALTIME] Failed to clear partial transcript preview: {e}', flush=True) + @property + def last_connect_failure(self) -> Optional[str]: + """Why the last recovery attempt failed: 'connecting', 'cooldown', 'failed', or None.""" + return self._last_connect_failure + + def _ensure_client(self) -> bool: + """Make the realtime client usable for a new recording. + + Single recovery entry point for the three states a client can be in: + connected, disconnected (idle close), or gone entirely — the last one + happens whenever close_realtime_connection() runs on a recording failure + or suspend, and nothing outside the resume path rebuilds it. + """ + if self._realtime_client: + if self._realtime_client.connected: + self._last_connect_failure = None + return True + return self._reconnect_realtime_client() + + now = time.monotonic() + last = self._last_rebuild_attempt + if last is not None and (now - last) < self.REBUILD_COOLDOWN_SECS: + print('[REALTIME] Rebuild failed recently; waiting before retry', flush=True) + self._last_connect_failure = 'cooldown' + return False + + # initialize() can block on the connect timeout, so don't retry it on + # every keypress while the endpoint is down. + self._last_rebuild_attempt = now + print('[REALTIME] Rebuilding client after teardown', flush=True) + try: + rebuilt = self.initialize() + except Exception as e: + print(f'[REALTIME] Rebuild failed: {e}', flush=True) + self._last_connect_failure = 'failed' + return False + + if not rebuilt or not self._realtime_client: + self._last_connect_failure = 'failed' + return False + + # Only failures should hold the cooldown, or a teardown shortly after a + # good rebuild would be stalled by the previous success. + self._last_rebuild_attempt = None + self._last_connect_failure = None + return True + def _reconnect_realtime_client(self) -> bool: """Reconnect realtime client using stored connect params.""" if not self._realtime_client: @@ -501,9 +556,11 @@ def _reconnect_realtime_client(self) -> bool: time.sleep(0.1) if self._realtime_client.connected: print('[REALTIME] In-flight connection landed; proceeding', flush=True) + self._last_connect_failure = None return True if getattr(self._realtime_client, 'connecting', False): print('[REALTIME] Still connecting; try again in a moment', flush=True) + self._last_connect_failure = 'connecting' return False # Attempt finished without connecting; fall through to reconnect. @@ -515,23 +572,33 @@ def _reconnect_realtime_client(self) -> bool: if not (websocket_url and api_key and model_id): print('[REALTIME] Missing connection parameters; cannot reconnect', flush=True) + self._last_connect_failure = 'failed' return False try: - # Best-effort: close any stale connection first + # Best-effort: drop stale socket/thread state first. Use reset() where + # available — close() latches the client shut and would make every + # reconnect from here fail instantly (issue #229). ElevenLabs has no + # reset(); its close() is already a transient teardown. try: - self._realtime_client.close() + teardown = getattr(self._realtime_client, 'reset', None) + if teardown is None: + teardown = self._realtime_client.close + teardown() except Exception: pass if not self._realtime_client.connect(websocket_url, api_key, model_id, instructions): print('[REALTIME] Reconnect failed', flush=True) + self._last_connect_failure = 'failed' return False print('[REALTIME] Reconnected on-demand', flush=True) + self._last_connect_failure = None return True except Exception as e: print(f'[REALTIME] Reconnect failed: {e}', flush=True) + self._last_connect_failure = 'failed' return False def discard_audio(self) -> None: diff --git a/lib/src/realtime_base.py b/lib/src/realtime_base.py index 8ba222e6..b43144c9 100644 --- a/lib/src/realtime_base.py +++ b/lib/src/realtime_base.py @@ -199,6 +199,8 @@ def __init__(self, mode: str = 'transcribe'): self._reconnect_threads = set() self._stop_event = threading.Event() self._closed = False + # Set by the transport callbacks when the in-flight attempt dies. + self._attempt_failed = None self._connection_generation = 0 self._active_generation = 0 @@ -373,12 +375,18 @@ def _connect_internal(self) -> bool: on_close=lambda ws, code, message: self._on_close(ws, code, message, generation), **kwargs, ) + # Lets the transport's error/close callbacks end this attempt's wait + # early instead of burning the full timeout on an endpoint that has + # already refused us. + attempt_failed = threading.Event() + abandon_closed_attempt = False with self.lock: if self._closed or generation != self._active_generation: abandon_closed_attempt = True else: self.ws = attempt_ws + self._attempt_failed = attempt_failed if abandon_closed_attempt: attempt_ws.close() return False @@ -395,6 +403,8 @@ def _connect_internal(self) -> bool: while not self.connected and (time.time() - start_time) < timeout: if self._stop_event.wait(0.1): break + if attempt_failed.is_set(): + break if self.connected: self._log('Connected successfully') @@ -402,7 +412,7 @@ def _connect_internal(self) -> bool: self._on_connect_success() return True else: - self._log('Connection timeout') + self._log('Connection failed' if attempt_failed.is_set() else 'Connection timeout') self._abandon_attempt(attempt_ws) return False @@ -465,8 +475,14 @@ def _on_error(self, ws, error, generation=None): with self.lock: if not self._is_active_connection(ws, generation): return + self._fail_pending_attempt_locked() self._log(f'WebSocket error: {error}') + def _fail_pending_attempt_locked(self): + """Mark the in-flight connect attempt dead (no-op once connected).""" + if not self.connected and self._attempt_failed is not None: + self._attempt_failed.set() + def _on_close(self, ws, close_status_code, _close_msg, generation=None): """Handle WebSocket close""" with self.lock: @@ -476,6 +492,9 @@ def _on_close(self, ws, close_status_code, _close_msg, generation=None): if not self._is_active_connection(ws, generation): self._log(f'Ignoring close from obsolete WebSocket (code: {close_status_code})') return + # A close before the handshake lands is a failed attempt, not an + # idle drop — don't make the caller wait out the connect timeout. + self._fail_pending_attempt_locked() was_connected = self.connected self.connected = False # Stop sender thread on disconnect; it will be restarted on next connect. @@ -528,8 +547,12 @@ def _attempt_reconnect(self): self._log('Reconnection cancelled (closing)') return False - # Abort if the client was closed while we were waiting - if self._closed or not self.receiver_running: + # Abort if the client was closed or torn down while we were waiting. + # Read under the lock: _closed can flip both ways now that an + # explicit connect() reopens a latched client. + with self.lock: + cancelled = self._closed or not self.receiver_running + if cancelled: self._log('Reconnection cancelled (closing)') return False @@ -543,13 +566,33 @@ def _attempt_reconnect(self): self._reconnect_lock.release() def close(self): - """Close WebSocket connection and cleanup""" + """Close WebSocket connection and cleanup (permanent: latches the client shut).""" + self._shutdown(latch=True) + + def reset(self): + """Tear the connection down without latching, so connect() can reopen it. + + close() latches `_closed` so background reconnect loops stand down during + a real shutdown. Reconnect paths want the same teardown *without* that + latch — see issue #229, where closing before reconnecting left the client + permanently unusable. + """ + self._shutdown(latch=False) + + def _shutdown(self, latch: bool): + """Shared teardown. `_closed` is mutated only under self.lock.""" with self.lock: - if self._closed: + if self._closed and latch: return - self._closed = True - self._stop_event.set() + if latch: + self._closed = True + self._stop_event.set() self._active_generation += 1 + # Any in-flight attempt is abandoned by the generation bump, and its + # own finally clause won't clear this flag once generations diverge. + self.connecting = False + # reset() sets no stop signal, so wake a waiting attempt explicitly. + self._fail_pending_attempt_locked() self._sender_running = False self.receiver_running = False self._audio_queue.clear() @@ -579,8 +622,13 @@ def close(self): with self.lock: self.connected = False + if not latch and not self._closed: + # Leave the client usable: no latch, and no lingering stop signal + # for the next attempt's wait loops to trip over. A client that + # was already closed for good keeps its stop signal. + self._stop_event.clear() - self._log('Connection closed') + self._log('Connection closed' if latch else 'Connection reset for reconnect') # ------------------------------------------------------------------ # Transcript assembly / commit diff --git a/lib/src/whisper_manager.py b/lib/src/whisper_manager.py index 517c5ada..fe51d909 100644 --- a/lib/src/whisper_manager.py +++ b/lib/src/whisper_manager.py @@ -147,6 +147,17 @@ def get_realtime_streaming_callback(self) -> Optional[Callable]: return backend.get_streaming_callback() return None + def realtime_connect_failure(self) -> Optional[str]: + """Why realtime recovery last failed ('connecting'/'cooldown'/'failed'), else None. + + Reads the backend directly rather than _active_realtime_backend(), which + excludes exactly the torn-down case we most need to explain. + """ + backend = self._backend + if backend is not None and backend.name == 'realtime-ws': + return getattr(backend, 'last_connect_failure', None) + return None + def _active_realtime_backend(self): """The realtime-ws backend when it has a live client, else None.""" backend = self._backend diff --git a/tests/test_realtime_reconnect_recovery.py b/tests/test_realtime_reconnect_recovery.py new file mode 100644 index 00000000..adf6fd85 --- /dev/null +++ b/tests/test_realtime_reconnect_recovery.py @@ -0,0 +1,308 @@ +"""Recovery from a dead realtime connection (regressions for issue #229). + +Two independent paths used to leave realtime permanently unusable until the +service restarted: + +- close() latched ``_closed`` and ``_connect_internal()`` refused while latched, + so the backend's on-demand reconnect (which tears down stale state first) + could never succeed; +- a recording failure ran ``close_realtime_connection()``, which drops the + client entirely, and nothing outside the suspend/resume path rebuilt it. + +Both are driven here against the real classes over a fake transport. +""" + +import sys +import threading +import time +import types +import unittest +from pathlib import Path +from unittest import mock + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "lib" / "src")) +sys.modules.setdefault("websocket", types.SimpleNamespace(WebSocketApp=object)) + +from backends.realtime_ws_backend import RealtimeWsBackend # noqa: E402 +from realtime_client import RealtimeClient # noqa: E402 +from whisper_manager import WhisperManager # noqa: E402 + + +class FakeWebSocketApp: + """Transport whose run_forever completes the handshake immediately.""" + + def __init__(self, url, on_open=None, on_message=None, on_error=None, on_close=None, **kwargs): + self.url = url + self.on_open = on_open + self.on_close = on_close + self._stop = threading.Event() + + def run_forever(self): + self.on_open(self) + self._stop.wait(5) + + def close(self): + self._stop.set() + + def send(self, payload): + pass + + +class DeadTransport: + """Transport whose socket is refused, so connect() fails immediately. + + Refusing at construction rather than hanging keeps these tests off the + client's 10s connect timeout. + """ + + class WebSocketApp: + def __init__(self, url, **kwargs): + raise ConnectionRefusedError("endpoint down") + + +class RefusedTransport: + """Transport that reports a refused connection through the WS callbacks.""" + + class WebSocketApp: + def __init__(self, url, on_open=None, on_error=None, on_close=None, **kwargs): + self.on_error = on_error + self.on_close = on_close + + def run_forever(self): + self.on_error(self, ConnectionRefusedError("endpoint down")) + self.on_close(self, None, "") + + def close(self): + pass + + +def _client(): + """A RealtimeClient wired to the fake transport, with I/O stubbed out.""" + client = RealtimeClient(mode="transcribe") + client._websocket_transport = types.SimpleNamespace(WebSocketApp=FakeWebSocketApp) + client._send_session_update = lambda *a, **k: None + return client + + +def _connected_client(): + client = _client() + assert client.connect("wss://example.test/rt", "key", "model") + return client + + +def _idle_close(client): + """Simulate the server dropping an idle session.""" + client._last_audio_chunk_time = 0.0 + client._on_close(client.ws, None, "", client._active_generation) + + +class ClientReopenTests(unittest.TestCase): + """close() must not make a client permanently unusable (issue #229).""" + + def test_connect_reopens_a_closed_client(self): + client = _connected_client() + _idle_close(client) + self.assertFalse(client.connected) + + # What the backend's on-demand reconnect used to do. + client.close() + self.assertTrue(client._closed) + + self.assertTrue(client.connect("wss://example.test/rt", "key", "model")) + self.assertTrue(client.connected) + self.assertFalse(client._closed) + + def test_reset_leaves_client_reconnectable(self): + client = _connected_client() + client.reset() + + self.assertFalse(client._closed) + self.assertFalse(client._stop_event.is_set()) + self.assertFalse(client.connected) + self.assertTrue(client._connect_internal()) + self.assertTrue(client.connected) + + def test_close_still_latches_against_internal_reconnect(self): + """Real shutdown must still stand background reconnect loops down.""" + client = _connected_client() + client.close() + + self.assertTrue(client._closed) + self.assertTrue(client._stop_event.is_set()) + self.assertFalse(client._connect_internal()) + self.assertFalse(client.connected) + + def test_reset_does_not_unlatch_a_closed_client(self): + """reset() is teardown, not resurrection — only connect() reopens.""" + client = _connected_client() + client.close() + client.reset() + + self.assertTrue(client._closed) + self.assertTrue(client._stop_event.is_set()) + self.assertFalse(client._connect_internal()) + + def test_attempt_reconnect_aborts_after_close(self): + client = _connected_client() + client.reconnect_delays = [0] + client.close() + + self.assertFalse(client._attempt_reconnect()) + + def test_refused_connection_fails_fast(self): + """A refused socket must not cost the caller the full 10s connect timeout.""" + client = _client() + client._websocket_transport = RefusedTransport() + + started = time.monotonic() + self.assertFalse(client.connect("wss://example.test/rt", "key", "model")) + self.assertLess(time.monotonic() - started, 3.0) + + def test_teardown_clears_stuck_connecting_flag(self): + """An abandoned attempt must not leave `connecting` latched True. + + _connect_internal only clears the flag when its generation is still + current, and teardown bumps the generation — so without an explicit + clear the backend would report "still connecting" forever. + """ + client = _client() + client._websocket_transport = DeadTransport() + client._connect_internal() # fails, leaving attempt state behind + client.connecting = True + + client.reset() + self.assertFalse(client.connecting) + + +class FakeConfig: + def __init__(self, backend="realtime-ws"): + self._backend = backend + + def get_setting(self, key, default=None): + if key == "transcription_backend": + return self._backend + return default + + def get_temp_directory(self): + return "/tmp" + + +def _backend(config=None): + manager = WhisperManager(config_manager=config or FakeConfig()) + return RealtimeWsBackend(manager) + + +class EnsureClientTests(unittest.TestCase): + """A torn-down client is rebuilt on the next recording, not left dead.""" + + def test_streaming_callback_rebuilds_destroyed_client(self): + backend = _backend() + sentinel = object() + + def fake_initialize(): + backend._realtime_client = _connected_client() + backend._realtime_streaming_callback = sentinel + return True + + with mock.patch.object(backend, "initialize", side_effect=fake_initialize) as init: + self.assertIs(backend.get_streaming_callback(), sentinel) + + self.assertEqual(init.call_count, 1) + self.assertIsNone(backend.last_connect_failure) + + def test_rebuild_is_not_retried_within_cooldown(self): + backend = _backend() + + with mock.patch.object(backend, "initialize", return_value=False) as init: + self.assertIsNone(backend.get_streaming_callback()) + self.assertEqual(backend.last_connect_failure, "failed") + + self.assertIsNone(backend.get_streaming_callback()) + self.assertEqual(init.call_count, 1) + self.assertEqual(backend.last_connect_failure, "cooldown") + + def test_rebuild_retried_after_cooldown_expires(self): + backend = _backend() + + with mock.patch.object(backend, "initialize", return_value=False) as init: + self.assertIsNone(backend.get_streaming_callback()) + backend._last_rebuild_attempt = time.monotonic() - backend.REBUILD_COOLDOWN_SECS - 1 + self.assertIsNone(backend.get_streaming_callback()) + + self.assertEqual(init.call_count, 2) + + def test_connected_client_is_used_as_is(self): + backend = _backend() + backend._realtime_client = _connected_client() + backend._realtime_streaming_callback = object() + + with mock.patch.object(backend, "initialize") as init: + self.assertIs( + backend.get_streaming_callback(), backend._realtime_streaming_callback + ) + init.assert_not_called() + + def test_non_realtime_backend_returns_none_without_rebuilding(self): + backend = _backend(config=FakeConfig(backend="pywhispercpp")) + + with mock.patch.object(backend, "initialize") as init: + self.assertIsNone(backend.get_streaming_callback()) + init.assert_not_called() + + +class OnDemandReconnectTests(unittest.TestCase): + """The on-demand path and the background loop must agree about recovery.""" + + def _backend_with_idle_closed_client(self): + backend = _backend() + client = _connected_client() + backend._realtime_client = client + backend._realtime_streaming_callback = object() + backend._realtime_connect_params = { + "websocket_url": "wss://example.test/rt", + "api_key": "key", + "model_id": "model", + "instructions": None, + } + _idle_close(client) + return backend, client + + def test_on_demand_reconnect_recovers_idle_close(self): + backend, client = self._backend_with_idle_closed_client() + + self.assertIs( + backend.get_streaming_callback(), backend._realtime_streaming_callback + ) + self.assertTrue(client.connected) + self.assertIsNone(backend.last_connect_failure) + + def test_background_reconnect_recovers_same_state(self): + """Same starting state, the other path: both must end connected.""" + _, client = self._backend_with_idle_closed_client() + client.reconnect_delays = [0] + client.receiver_running = True + + self.assertTrue(client._attempt_reconnect()) + self.assertTrue(client.connected) + + def test_reconnect_failure_is_reported_as_failed(self): + backend, client = self._backend_with_idle_closed_client() + client._websocket_transport = DeadTransport() + + self.assertIsNone(backend.get_streaming_callback()) + self.assertEqual(backend.last_connect_failure, "failed") + + def test_in_flight_handshake_is_not_torn_down(self): + backend, client = self._backend_with_idle_closed_client() + client.connecting = True + + with mock.patch.object(client, "connect") as connect: + self.assertIsNone(backend.get_streaming_callback()) + + connect.assert_not_called() + self.assertEqual(backend.last_connect_failure, "connecting") + + +if __name__ == "__main__": + unittest.main()