Merge pull request #343 from agessaman/fix/telegram-stop-without-token

fix(bridges): stop cleanly when the bridge disabled itself at startup

CHANGELOG.md

@@ -39,6 +39,7 @@ semantic versioning.
 
 ### Fixed
 
+- The Telegram bridge no longer logs "Failed to stop service" at shutdown when it is enabled without an `api_token`. It disabled itself before setting up the state `stop()` uses, so stopping it raised `AttributeError`. The Discord bridge had the same gap when neither aiohttp nor requests is installed.
 - `trace` no longer gives up on a reply that is on its way. Each attempt listened only for its own tag, so when the first trace's reply arrived after its timeout, the retry had already replaced it and the reply was dropped; on a mesh where repeaters wait a second or more before forwarding, a three-hop trace always failed. A reply to any attempt now counts until the last attempt times out, and one that arrives during the retry delay ends the trace without sending again. The timeout defaults are now 2.0 s base and 1.5 s per hop (were 1.0 s and 0.5 s, about 2.5 s for three hops); a config that sets them keeps its values.
 - Repeat counts for the bot's own messages (the web viewer's command log, the DARC MoWaS retry check) count the bot's own packets. A sent message was matched to the first packet the radio heard within 30 seconds, whatever it was, so on a busy mesh it usually took someone else's packet: its repeats were counted as the bot's, and the bot's real repeats were not. A channel message is now matched only by a packet that decrypts on the same channel to the radio's name and the same text, a DM by a direct message addressed to the recipient from the bot, and a trace by its tag. The repeat counts are written to the database off the event loop, and each repeat no longer adds an entry to Python's import path.
 - A successful `trace` now adds its links to the mesh graph, as `[Trace_Command] update_graph_one_byte` and `update_graph_two_byte` (both on by default) describe. The bot's own prefix was read from a radio attribute meshcore_py doesn't provide, so it was always unknown and every trace's graph update was skipped; it now comes from the radio's reported public key, at the trace's hash width. `update_graph_two_byte` was never read: 2-byte traces followed `update_graph_one_byte`, and now follow their own setting.

modules/service_plugins/discord_bridge_service.py

@@ -111,6 +111,14 @@ class DiscordBridgeService(BaseServicePlugin):
         # Use bot's logger directly (inherited from BaseServicePlugin)
         # self.logger is already set by super().__init__(bot)
 
+        # State stop() relies on, set before the early return below so the
+        # service still stops cleanly when it disables itself.
+        self.channel_webhooks: dict[str, list[str]] = {}
+        self.message_queues: dict[str, list[Any]] = {}
+        self.send_times: dict[str, deque] = {}
+        self.http_session: Optional[aiohttp.ClientSession] = None
+        self._queue_processor_task: Optional[asyncio.Task] = None
+
         # Check if HTTP library is available
         if not AIOHTTP_AVAILABLE and not REQUESTS_AVAILABLE:
             self.logger.error("Neither aiohttp nor requests library is available. Discord bridge requires one of these.")
@@ -119,7 +127,6 @@ class DiscordBridgeService(BaseServicePlugin):
 
         # Load channel mappings from config (bridge.* pattern)
         # Map MeshCore channel name → list of Discord webhook URLs
-        self.channel_webhooks: dict[str, list[str]] = {}
         self._load_channel_mappings()
 
         # NEVER bridge DMs (hardcoded for privacy)
@@ -154,23 +161,17 @@ class DiscordBridgeService(BaseServicePlugin):
 
         # Message queue per webhook to handle rate limits and retries
         # Using list instead of deque for easier removal of arbitrary items
-        self.message_queues: dict[str, list[Any]] = {}
         self.max_retries = 5  # Maximum retry attempts per message
         self.retry_delay_base = 1.0  # Base delay in seconds for exponential backoff
         self.max_queue_age = 300  # Max age in seconds before dropping message (5 minutes)
 
         # Proactive rate limiting: track send times per webhook
         # Discord allows 30 messages per 60 seconds, so we'll throttle to ~25/min for safety
-        self.send_times: dict[str, deque] = {}  # Track timestamps of sent messages (deque for efficient popleft)
         self.rate_limit_window = 60.0  # 60 second window
         self.rate_limit_max = 25  # Conservative limit (25/min instead of 30/min for safety)
 
-        # HTTP session for async requests
-        self.http_session: Optional[aiohttp.ClientSession] = None
-
         # Background task handles
         self._message_handler_task: Optional[asyncio.Task] = None
-        self._queue_processor_task: Optional[asyncio.Task] = None
 
         if not self.channel_webhooks:
             self.logger.warning("No Discord channel mappings configured. Discord bridge will not post any messages.")

modules/service_plugins/telegram_bridge_service.py

@@ -101,6 +101,14 @@ class TelegramBridgeService(BaseServicePlugin):
     def __init__(self, bot: Any):
         super().__init__(bot)
 
+        # State stop() relies on, set before any early return so a bridge left
+        # disabled by missing config still stops cleanly at shutdown.
+        self.channel_chat_ids: dict[str, str] = {}
+        self.message_queues: dict[str, list[QueuedMessage]] = {}
+        self.send_times: dict[str, deque] = {}
+        self.http_session: Optional[aiohttp.ClientSession] = None
+        self._queue_processor_task: Optional[asyncio.Task] = None
+
         if not AIOHTTP_AVAILABLE and not REQUESTS_AVAILABLE:
             self.logger.error(
                 "Neither aiohttp nor requests available. Telegram bridge requires one of these."
@@ -118,7 +126,6 @@ class TelegramBridgeService(BaseServicePlugin):
             self.enabled = False
             return
 
-        self.channel_chat_ids: dict[str, str] = {}
         self._load_channel_mappings()
 
         # Optional settings
@@ -145,16 +152,11 @@ class TelegramBridgeService(BaseServicePlugin):
         )
 
         # Rate limiting: ~1 message per second per chat
-        self.message_queues: dict[str, list[QueuedMessage]] = {}
-        self.send_times: dict[str, deque] = {}
         self.rate_limit_min_interval = 1.0
         self.max_retries = 5
         self.retry_delay_base = 1.0
         self.max_queue_age = 300
 
-        self.http_session: Optional[aiohttp.ClientSession] = None
-        self._queue_processor_task: Optional[asyncio.Task] = None
-
         if not self.channel_chat_ids:
             self.logger.warning(
                 "No Telegram channel mappings configured. "

tests/test_bridge_disabled_stop.py

@@ -0,0 +1,53 @@
+"""A bridge that disabled itself in __init__ still starts and stops without errors."""
+
+from configparser import ConfigParser
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from modules.service_plugins.discord_bridge_service import DiscordBridgeService
+from modules.service_plugins.telegram_bridge_service import TelegramBridgeService
+
+
+@pytest.fixture
+def bot(mock_logger):
+    bot = MagicMock()
+    bot.logger = mock_logger
+    bot.config = ConfigParser()
+    bot.config.add_section("DiscordBridge")
+    bot.config.add_section("TelegramBridge")
+    bot.channel_sent_listeners = []
+    return bot
+
+
+async def _start_and_stop(service, bot):
+    await service.start()
+    await service.stop()
+    bot.logger.error.assert_called_once()  # only the reason it disabled itself
+    assert bot.channel_sent_listeners == []
+
+
+@pytest.mark.asyncio
+class TestDisabledBridgesStop:
+    async def test_telegram_without_api_token(self, bot, monkeypatch):
+        monkeypatch.delenv("TELEGRAM_BOT_TOKEN", raising=False)
+        service = TelegramBridgeService(bot)
+        assert service.enabled is False
+        await _start_and_stop(service, bot)
+        bot.logger.info.assert_any_call("Telegram bridge service stopped")
+
+    async def test_telegram_without_http_library(self, bot):
+        with patch("modules.service_plugins.telegram_bridge_service.AIOHTTP_AVAILABLE", False), \
+                patch("modules.service_plugins.telegram_bridge_service.REQUESTS_AVAILABLE", False):
+            service = TelegramBridgeService(bot)
+        assert service.enabled is False
+        await _start_and_stop(service, bot)
+        bot.logger.info.assert_any_call("Telegram bridge service stopped")
+
+    async def test_discord_without_http_library(self, bot):
+        with patch("modules.service_plugins.discord_bridge_service.AIOHTTP_AVAILABLE", False), \
+                patch("modules.service_plugins.discord_bridge_service.REQUESTS_AVAILABLE", False):
+            service = DiscordBridgeService(bot)
+        assert service.enabled is False
+        await _start_and_stop(service, bot)
+        bot.logger.info.assert_any_call("Discord bridge service stopped")