From 9149c34a26d2287cd98cd8f3a51011d27023b085 Mon Sep 17 00:00:00 2001 From: aydnOktay Date: Wed, 11 Mar 2026 05:34:43 -0700 Subject: [PATCH] refactor(slack): replace print statements with structured logging Replaces all ad-hoc print() calls in the Slack gateway adapter with proper logging.getLogger(__name__) calls, matching the pattern already used by every other platform adapter (telegram, discord, whatsapp, signal, homeassistant). Changes: - Add import logging + module-level logger - Use logger.error for failures, logger.warning for non-critical fallbacks, logger.info for status, logger.debug for routine ops - Add exc_info=True for full stack traces on all error/warning paths - Use %s format strings (lazy evaluation) instead of f-strings - Wrap disconnect() in try/except for safety - Add structured context (file paths, channel IDs, URLs) to log messages - Convert document handling prints added after the original PR Cherry-picked from PR #778 by aydnOktay, rebased onto current main with conflict resolution and extended to cover document/video methods added since the PR was created. Co-authored-by: aydnOktay --- gateway/platforms/slack.py | 108 +++++++++++++++++++++++++++---------- 1 file changed, 80 insertions(+), 28 deletions(-) diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py index 3449971f..f7f0dda2 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -9,6 +9,7 @@ Uses slack-bolt (Python) with Socket Mode for: """ import asyncio +import logging import os import re from typing import Dict, List, Optional, Any @@ -41,6 +42,9 @@ from gateway.platforms.base import ( ) +logger = logging.getLogger(__name__) + + def check_slack_requirements() -> bool: """Check if Slack dependencies are available.""" return SLACK_AVAILABLE @@ -73,17 +77,19 @@ class SlackAdapter(BasePlatformAdapter): async def connect(self) -> bool: """Connect to Slack via Socket Mode.""" if not SLACK_AVAILABLE: - print("[Slack] slack-bolt not installed. Run: pip install slack-bolt") + logger.error( + "[Slack] slack-bolt not installed. Run: pip install slack-bolt", + ) return False bot_token = self.config.token app_token = os.getenv("SLACK_APP_TOKEN") if not bot_token: - print("[Slack] SLACK_BOT_TOKEN not set") + logger.error("[Slack] SLACK_BOT_TOKEN not set") return False if not app_token: - print("[Slack] SLACK_APP_TOKEN not set") + logger.error("[Slack] SLACK_APP_TOKEN not set") return False try: @@ -117,19 +123,22 @@ class SlackAdapter(BasePlatformAdapter): asyncio.create_task(self._handler.start_async()) self._running = True - print(f"[Slack] Connected as @{bot_name} (Socket Mode)") + logger.info("[Slack] Connected as @%s (Socket Mode)", bot_name) return True - except Exception as e: - print(f"[Slack] Connection failed: {e}") + except Exception as e: # pragma: no cover - defensive logging + logger.error("[Slack] Connection failed: %s", e, exc_info=True) return False async def disconnect(self) -> None: """Disconnect from Slack.""" if self._handler: - await self._handler.close_async() + try: + await self._handler.close_async() + except Exception as e: # pragma: no cover - defensive logging + logger.warning("[Slack] Error while closing Socket Mode handler: %s", e, exc_info=True) self._running = False - print("[Slack] Disconnected") + logger.info("[Slack] Disconnected") async def send( self, @@ -162,8 +171,8 @@ class SlackAdapter(BasePlatformAdapter): raw_response=result, ) - except Exception as e: - print(f"[Slack] Send error: {e}") + except Exception as e: # pragma: no cover - defensive logging + logger.error("[Slack] Send error: %s", e, exc_info=True) return SendResult(success=False, error=str(e)) async def edit_message( @@ -182,7 +191,14 @@ class SlackAdapter(BasePlatformAdapter): text=content, ) return SendResult(success=True, message_id=message_id) - except Exception as e: + except Exception as e: # pragma: no cover - defensive logging + logger.error( + "[Slack] Failed to edit message %s in channel %s: %s", + message_id, + chat_id, + e, + exc_info=True, + ) return SendResult(success=False, error=str(e)) async def send_typing(self, chat_id: str, metadata=None) -> None: @@ -214,8 +230,14 @@ class SlackAdapter(BasePlatformAdapter): ) return SendResult(success=True, raw_response=result) - except Exception as e: - print(f"[{self.name}] Failed to send local image: {e}") + except Exception as e: # pragma: no cover - defensive logging + logger.error( + "[%s] Failed to send local Slack image %s: %s", + self.name, + image_path, + e, + exc_info=True, + ) return await super().send_image_file(chat_id, image_path, caption, reply_to) async def send_image( @@ -247,7 +269,13 @@ class SlackAdapter(BasePlatformAdapter): return SendResult(success=True, raw_response=result) - except Exception as e: + except Exception as e: # pragma: no cover - defensive logging + logger.warning( + "[Slack] Failed to upload image from URL %s, falling back to text: %s", + image_url, + e, + exc_info=True, + ) # Fall back to sending the URL as text text = f"{caption}\n{image_url}" if caption else image_url return await self.send(chat_id=chat_id, content=text, reply_to=reply_to) @@ -273,7 +301,13 @@ class SlackAdapter(BasePlatformAdapter): ) return SendResult(success=True, raw_response=result) - except Exception as e: + except Exception as e: # pragma: no cover - defensive logging + logger.error( + "[Slack] Failed to send audio file %s: %s", + audio_path, + e, + exc_info=True, + ) return SendResult(success=False, error=str(e)) async def send_video( @@ -300,8 +334,14 @@ class SlackAdapter(BasePlatformAdapter): ) return SendResult(success=True, raw_response=result) - except Exception as e: - print(f"[{self.name}] Failed to send video: {e}") + except Exception as e: # pragma: no cover - defensive logging + logger.error( + "[%s] Failed to send video %s: %s", + self.name, + video_path, + e, + exc_info=True, + ) return await super().send_video(chat_id, video_path, caption, reply_to) async def send_document( @@ -331,8 +371,14 @@ class SlackAdapter(BasePlatformAdapter): ) return SendResult(success=True, raw_response=result) - except Exception as e: - print(f"[{self.name}] Failed to send document: {e}") + except Exception as e: # pragma: no cover - defensive logging + logger.error( + "[%s] Failed to send document %s: %s", + self.name, + file_path, + e, + exc_info=True, + ) return await super().send_document(chat_id, file_path, caption, file_name, reply_to) async def get_chat_info(self, chat_id: str) -> Dict[str, Any]: @@ -348,7 +394,13 @@ class SlackAdapter(BasePlatformAdapter): "name": channel.get("name", chat_id), "type": "dm" if is_dm else "group", } - except Exception: + except Exception as e: # pragma: no cover - defensive logging + logger.error( + "[Slack] Failed to fetch chat info for %s: %s", + chat_id, + e, + exc_info=True, + ) return {"name": chat_id, "type": "unknown"} # ----- Internal handlers ----- @@ -403,8 +455,8 @@ class SlackAdapter(BasePlatformAdapter): media_urls.append(cached) media_types.append(mimetype) msg_type = MessageType.PHOTO - except Exception as e: - print(f"[Slack] Failed to cache image: {e}", flush=True) + except Exception as e: # pragma: no cover - defensive logging + logger.warning("[Slack] Failed to cache image from %s: %s", url, e, exc_info=True) elif mimetype.startswith("audio/") and url: try: ext = "." + mimetype.split("/")[-1].split(";")[0] @@ -414,8 +466,8 @@ class SlackAdapter(BasePlatformAdapter): media_urls.append(cached) media_types.append(mimetype) msg_type = MessageType.VOICE - except Exception as e: - print(f"[Slack] Failed to cache audio: {e}", flush=True) + except Exception as e: # pragma: no cover - defensive logging + logger.warning("[Slack] Failed to cache audio from %s: %s", url, e, exc_info=True) elif url: # Try to handle as a document attachment try: @@ -437,7 +489,7 @@ class SlackAdapter(BasePlatformAdapter): file_size = f.get("size", 0) MAX_DOC_BYTES = 20 * 1024 * 1024 if not file_size or file_size > MAX_DOC_BYTES: - print(f"[Slack] Document too large or unknown size: {file_size}", flush=True) + logger.warning("[Slack] Document too large or unknown size: %s", file_size) continue # Download and cache @@ -449,7 +501,7 @@ class SlackAdapter(BasePlatformAdapter): media_urls.append(cached_path) media_types.append(doc_mime) msg_type = MessageType.DOCUMENT - print(f"[Slack] Cached user document: {cached_path}", flush=True) + logger.debug("[Slack] Cached user document: %s", cached_path) # Inject text content for .txt/.md files (capped at 100 KB) MAX_TEXT_INJECT_BYTES = 100 * 1024 @@ -466,8 +518,8 @@ class SlackAdapter(BasePlatformAdapter): except UnicodeDecodeError: pass # Binary content, skip injection - except Exception as e: - print(f"[Slack] Failed to cache document: {e}", flush=True) + except Exception as e: # pragma: no cover - defensive logging + logger.warning("[Slack] Failed to cache document from %s: %s", url, e, exc_info=True) # Build source source = self.build_source(