瀏覽代碼

Replace client-fetch try/except ceremony with guard clauses

"No client right now" is a normal state during startup, shutdown and
reconnects, so it should read as a guard clause, not an exception.
_get_chatto_client() + is None now fetches at every call site whose
recovery is log-and-return (or a retryable SendResult); the nested
double-tries in the typing loop, reactions, upload and DM/room creation
flatten accordingly.

_require_client() stays for exactly one caller: the realtime event
loop, whose except chain turns the raise into a backed-off reconnect.
Both docstrings now state when to use which.
Paul Klumpp 1 周之前
父節點
當前提交
172087260a
共有 2 個文件被更改,包括 80 次插入74 次删除
  1. 76 70
      adapter.py
  2. 4 4
      test_adapter.py

+ 76 - 70
adapter.py

@@ -314,7 +314,20 @@ class ChattoAdapter(BasePlatformAdapter):
     # ------------------------------------------------------------------ #
 
     async def _get_chatto_client(self: ChattoAdapter) -> ChattoClient | None:
-        """Get or create a ChattoClient instance."""
+        """Return the shared ChattoClient, creating and logging in on first use.
+
+        The default way to get a client. The fast path is a plain attribute
+        read; first creation runs under a lock so concurrent callers log in
+        exactly once. Returns ``None`` when no client exists yet or creation
+        failed (bad credentials, unreachable server) — a normal state during
+        startup, shutdown and reconnects, not an exceptional one. Callers
+        decide what "no client" means for them, as a guard clause:
+
+            client = await self._get_chatto_client()
+            if client is None:
+                logger.warning("Chatto: dropping X - no client available")
+                return
+        """
         if self._chatto_client is not None:
             return self._chatto_client
 
@@ -349,9 +362,12 @@ class ChattoAdapter(BasePlatformAdapter):
     async def _require_client(self) -> ChattoClient:
         """Return a ChattoClient or raise RuntimeError if unavailable.
 
-        Use this helper when the caller expects a client to exist and
-        wants a single canonical failure path. Methods that prefer a
-        soft-fail can catch RuntimeError and return gracefully.
+        The exception-flavoured variant of :meth:`_get_chatto_client`, for
+        callers whose surrounding machinery already routes exceptions.
+        Currently that is only the realtime event loop, whose except chain
+        turns the raise into a logged warning plus a backed-off reconnect —
+        no special "no client" branch needed there. Everywhere else, prefer
+        the ``is None`` guard shown in :meth:`_get_chatto_client`.
         """
         client = await self._get_chatto_client()
         if client is None:
@@ -394,9 +410,8 @@ class ChattoAdapter(BasePlatformAdapter):
         if not await self._ensure_token():
             return False
 
-        try:
-            client = await self._require_client()
-        except RuntimeError:
+        client = await self._get_chatto_client()
+        if client is None:
             self._set_fatal_error(
                 "connect_failed", "Chatto client not available", retryable=True
             )
@@ -452,8 +467,13 @@ class ChattoAdapter(BasePlatformAdapter):
         Logged at warning level on failure: a silently dropped presence call is
         indistinguishable from a bot that is simply not running.
         """
+        client = await self._get_chatto_client()
+        if client is None:
+            logger.warning(
+                "Chatto: presence refresh failed, bot may appear offline: no client"
+            )
+            return False
         try:
-            client = await self._require_client()
             await client.update_presence(status=PresenceStatus.ONLINE)
             return True
         except Exception as exc:
@@ -522,14 +542,13 @@ class ChattoAdapter(BasePlatformAdapter):
     async def _seed_room(self, room_id: str) -> None:
         """Seed high-water mark from the newest events so a restart doesn't replay history."""
         try:
-            try:
-                client = await self._require_client()
-                timeline_page = await client.get_room_events(room_id)
-            except RuntimeError:
+            client = await self._get_chatto_client()
+            if client is None:
                 logger.debug(
                     "Chatto: _seed_room aborted - no client available for %s", room_id
                 )
                 return
+            timeline_page = await client.get_room_events(room_id)
 
             for ev in timeline_page.events:
                 if ev.id:
@@ -586,11 +605,13 @@ class ChattoAdapter(BasePlatformAdapter):
         known = self._known_handles.get(handle)
         if known is not None:
             return known
+        client = await self._get_chatto_client()
+        if client is None:
+            # Unresolved means "not confirmed", so the message goes through.
+            return False
         try:
-            client = await self._require_client()
             member = await client.get_user(login=handle)
         except Exception as exc:
-            # Unresolved means "not confirmed", so the message goes through.
             logger.debug("Chatto: could not resolve handle @%s: %s", handle, exc)
             return False
         exists = member is not None and member.user is not None
@@ -651,9 +672,8 @@ class ChattoAdapter(BasePlatformAdapter):
         if verb.lower() not in self._DM_COMMANDS:
             return False
 
-        try:
-            client = await self._require_client()
-        except RuntimeError:
+        client = await self._get_chatto_client()
+        if client is None:
             await self.send(chat_id=room_id, content="Chatto client is not connected.")
             return True
 
@@ -915,9 +935,8 @@ class ChattoAdapter(BasePlatformAdapter):
             )
             return
 
-        try:
-            client = await self._require_client()
-        except RuntimeError:
+        client = await self._get_chatto_client()
+        if client is None:
             logger.warning("Chatto: dropping message - no client available")
             return
         logger.debug("Chatto WS: 'message_posted' payload:%s", payload)
@@ -1008,9 +1027,8 @@ class ChattoAdapter(BasePlatformAdapter):
             logger.debug("Chatto: edit from read-only room %s ignored", payload.room_id)
             return
 
-        try:
-            client = await self._require_client()
-        except RuntimeError:
+        client = await self._get_chatto_client()
+        if client is None:
             logger.warning("Chatto: dropping edit - no client available")
             return
         logger.debug("Chatto WS: 'message_edited' payload:%s", payload)
@@ -1466,9 +1484,8 @@ class ChattoAdapter(BasePlatformAdapter):
 
     async def _refresh_rooms(self) -> None:
         """Refresh room list via ConnectRPC, join and seed any newly discovered rooms."""
-        try:
-            client = await self._require_client()
-        except RuntimeError:
+        client = await self._get_chatto_client()
+        if client is None:
             logger.warning("Chatto WS: _refresh_rooms aborted - no client available")
             return
 
@@ -1636,9 +1653,8 @@ class ChattoAdapter(BasePlatformAdapter):
         last_error: str | None = None
         retryable = False
 
-        try:
-            client = await self._require_client()
-        except RuntimeError:
+        client = await self._get_chatto_client()
+        if client is None:
             return SendResult(
                 success=False, error="Chatto client not available", retryable=True
             )
@@ -1769,9 +1785,8 @@ class ChattoAdapter(BasePlatformAdapter):
                 ),
             )
 
-        try:
-            client = await self._require_client()
-        except RuntimeError:
+        client = await self._get_chatto_client()
+        if client is None:
             return SendResult(
                 success=False, error="Chatto client not available", retryable=True
             )
@@ -1806,9 +1821,8 @@ class ChattoAdapter(BasePlatformAdapter):
         """
         if not chat_id or not message_id:
             return False
-        try:
-            client = await self._require_client()
-        except RuntimeError:
+        client = await self._get_chatto_client()
+        if client is None:
             logger.warning("Chatto: DeleteMessage — client unavailable")
             return False
         try:
@@ -1845,9 +1859,8 @@ class ChattoAdapter(BasePlatformAdapter):
             logger.debug("Chatto: handoff thread skipped — %s is a DM", parent_chat_id)
             return None
 
-        try:
-            client = await self._require_client()
-        except RuntimeError:
+        client = await self._get_chatto_client()
+        if client is None:
             logger.warning("Chatto: handoff thread — client unavailable")
             return None
 
@@ -1898,9 +1911,8 @@ class ChattoAdapter(BasePlatformAdapter):
             try:
                 while True:
                     try:
-                        try:
-                            client = await self._require_client()
-                        except RuntimeError:
+                        client = await self._get_chatto_client()
+                        if client is None:
                             return
                         await client.update_typing_indicator(room_id=str(chat_id))
                     except asyncio.CancelledError:
@@ -1964,12 +1976,11 @@ class ChattoAdapter(BasePlatformAdapter):
     async def add_reaction(self, room_id: str, message_id: str, emoji: str) -> bool:
         """Add a reaction to a message via MessageService/AddReaction."""
         shortcode = self._emoji_to_shortcode(emoji)
+        client = await self._get_chatto_client()
+        if client is None:
+            logger.warning("Chatto: AddReaction — client unavailable")
+            return False
         try:
-            try:
-                client = await self._require_client()
-            except RuntimeError:
-                logger.warning("Chatto: AddReaction — client unavailable")
-                return False
             result = await client.add_reaction(
                 room_id=room_id,
                 message_event_id=message_id,
@@ -1986,12 +1997,11 @@ class ChattoAdapter(BasePlatformAdapter):
     async def remove_reaction(self, chat_id: str, message_id: str, emoji: str) -> bool:
         """Remove a reaction from a message via MessageService/RemoveReaction."""
         shortcode = self._emoji_to_shortcode(emoji)
+        client = await self._get_chatto_client()
+        if client is None:
+            logger.warning("Chatto: RemoveReaction — client unavailable")
+            return False
         try:
-            try:
-                client = await self._require_client()
-            except RuntimeError:
-                logger.warning("Chatto: RemoveReaction — client unavailable")
-                return False
             result = await client.remove_reaction(
                 room_id=str(chat_id),
                 message_event_id=str(message_id),
@@ -2016,11 +2026,10 @@ class ChattoAdapter(BasePlatformAdapter):
         """
         if not user_id:
             return None
+        client = await self._get_chatto_client()
+        if client is None:
+            return None
         try:
-            try:
-                client = await self._require_client()
-            except RuntimeError:
-                return None
             room = await client.start_dm(participant_ids=[str(user_id)])
             self._room_names[room.id] = room.name
             self._room_kinds[room.id] = room.kind
@@ -2047,11 +2056,10 @@ class ChattoAdapter(BasePlatformAdapter):
 
         Returns the room ID on success, or None on failure.
         """
+        client = await self._get_chatto_client()
+        if client is None:
+            return None
         try:
-            try:
-                client = await self._require_client()
-            except RuntimeError:
-                return None
             room = await client.create_room(
                 name=name,
                 group_id=group_id or "",
@@ -2171,12 +2179,11 @@ class ChattoAdapter(BasePlatformAdapter):
         mime_type = mimetypes.guess_type(file_path)[0] or "application/octet-stream"
         sha256_hash = hashlib.sha256(file_data).hexdigest()
 
+        client = await self._get_chatto_client()
+        if client is None:
+            logger.error("Chatto: upload aborted - no client available")
+            return None
         try:
-            try:
-                client = await self._require_client()
-            except RuntimeError:
-                logger.error("Chatto: upload aborted - no client available")
-                return None
             # Step 1: Create upload session
             upload = await client.create_upload(
                 room_id=room_id,
@@ -2240,13 +2247,12 @@ class ChattoAdapter(BasePlatformAdapter):
         if reply_to:
             thread_id = reply_to
 
+        client = await self._get_chatto_client()
+        if client is None:
+            return SendResult(
+                success=False, error="Chatto client not available", retryable=True
+            )
         try:
-            try:
-                client = await self._require_client()
-            except RuntimeError:
-                return SendResult(
-                    success=False, error="Chatto client not available", retryable=True
-                )
             msg = await client.post_message(
                 room_id=str(chat_id),
                 body=self.format_message(caption) if caption else "",

+ 4 - 4
test_adapter.py

@@ -1775,7 +1775,7 @@ class TestRespondRooms:
         adapter.chatto_config.respond_rooms.value = respond_rooms
         adapter.me = _make_user("bot-user-id", "hermes_bot")
         adapter.handle_message = AsyncMock()
-        adapter._require_client = AsyncMock(side_effect=RuntimeError("no client"))
+        adapter._get_chatto_client = AsyncMock(return_value=None)
         return adapter
 
     async def test_unlisted_room_is_dropped_before_any_api_call(self):
@@ -1803,7 +1803,7 @@ class TestRespondRooms:
     async def test_listed_room_reaches_the_pipeline(self):
         adapter = self._dropping_adapter(["room-1"])
         adapter.chatto_config.allow_all_users.value = True
-        adapter._require_client = AsyncMock(return_value=adapter._chatto_client)
+        adapter._get_chatto_client = AsyncMock(return_value=adapter._chatto_client)
         adapter._room_kinds["room-1"] = RoomKind.CHANNEL
         adapter._user_cache["user-1"] = _make_user("user-1", "alice")
         payload = _make_posted_payload(room_id="room-1")
@@ -1819,7 +1819,7 @@ class TestRespondRooms:
         """DMs stay respond rooms so /join remains reachable."""
         adapter = self._dropping_adapter(["listed-1"])
         adapter.chatto_config.allow_all_users.value = True
-        adapter._require_client = AsyncMock(return_value=adapter._chatto_client)
+        adapter._get_chatto_client = AsyncMock(return_value=adapter._chatto_client)
         adapter._room_kinds["dm-1"] = RoomKind.DM
         adapter._user_cache["user-1"] = _make_user("user-1", "alice")
         payload = _make_posted_payload(room_id="dm-1")
@@ -1835,7 +1835,7 @@ class TestRespondRooms:
         """Unset list keeps the pre-existing behaviour: every room responds."""
         adapter = self._dropping_adapter([])
         adapter.chatto_config.allow_all_users.value = True
-        adapter._require_client = AsyncMock(return_value=adapter._chatto_client)
+        adapter._get_chatto_client = AsyncMock(return_value=adapter._chatto_client)
         adapter._room_kinds["any-room"] = RoomKind.CHANNEL
         adapter._user_cache["user-1"] = _make_user("user-1", "alice")
         payload = _make_posted_payload(room_id="any-room")