From 9d344a8fc00c360dac662cfcafd3610b52b83e99 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Wed, 25 Feb 2026 15:50:15 +0100 Subject: [PATCH 01/85] test: add Filtering cog_load and test scaffold and mocked setup --- .../bot/exts/filtering/test_filtering_cog.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 tests/bot/exts/filtering/test_filtering_cog.py diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py new file mode 100644 index 0000000000..2faafa00cd --- /dev/null +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -0,0 +1,28 @@ +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +from bot.exts.filtering.filtering import Filtering + + +class FilteringCogLoadTests(unittest.IsolatedAsyncioTestCase): + """Test startup behavior of the Filtering cog (`cog_load`).""" + + def setUp(self) -> None: + """Set up a Filtering cog with a mocked bot and stubbed startup dependencies.""" + self.bot = MagicMock() + self.bot.wait_until_guild_available = AsyncMock() + + self.bot.api_client = MagicMock() + self.bot.api_client.get = AsyncMock() + + self.cog = Filtering(self.bot) + + # Stub internals that are not relevant to this unit test. + self.cog.collect_loaded_types = MagicMock() + self.cog.schedule_offending_messages_deletion = AsyncMock() + self.cog._fetch_or_generate_filtering_webhook = AsyncMock(return_value=MagicMock()) + + # `weekly_auto_infraction_report_task` is a discord task loop; patch its start method. + self.start_patcher = patch.object(self.cog.weekly_auto_infraction_report_task, "start") + self.mock_weekly_task_start = self.start_patcher.start() + self.addCleanup(self.start_patcher.stop) From 7a66cd43c4d01583281e6cb7ca0a133a100c7345 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Wed, 25 Feb 2026 15:53:19 +0100 Subject: [PATCH 02/85] test: cover cog_load behavior when filter list fetch fails --- tests/bot/exts/filtering/test_filtering_cog.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 2faafa00cd..58b111505a 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -26,3 +26,18 @@ def setUp(self) -> None: self.start_patcher = patch.object(self.cog.weekly_auto_infraction_report_task, "start") self.mock_weekly_task_start = self.start_patcher.start() self.addCleanup(self.start_patcher.stop) + + async def test_cog_load_when_filter_list_fetch_fails(self): + """`cog_load` should currently raise if loading filter lists from the API fails.""" + self.bot.api_client.get.side_effect = RuntimeError("Simulated site/API outage during cog_load") + + with self.assertRaises(RuntimeError): + await self.cog.cog_load() + + self.bot.wait_until_guild_available.assert_awaited_once() + self.bot.api_client.get.assert_awaited_once_with("bot/filter/filter_lists") + + # Startup should stop before later steps. + self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() + self.cog.schedule_offending_messages_deletion.assert_not_awaited() + self.mock_weekly_task_start.assert_not_called() From 26d30a82e8a0ed322fb25dd811bb2f5d609b5dcb Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Wed, 25 Feb 2026 20:42:22 +0100 Subject: [PATCH 03/85] feat: Add retry error filter and mod alert for filter load failures #6 --- bot/exts/filtering/filtering.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index 210ae3fb05..837b552719 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -64,6 +64,8 @@ HOURS_BETWEEN_NICKNAME_ALERTS = 1 OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=7) WEEKLY_REPORT_ISO_DAY = 3 # 1=Monday, 7=Sunday +FILTER_LOAD_MAX_ATTEMPTS = 3 +INITIAL_BACKOFF_SECONDS = 1 async def _extract_text_file_content(att: discord.Attachment) -> str: @@ -122,6 +124,33 @@ async def cog_load(self) -> None: await self.schedule_offending_messages_deletion() self.weekly_auto_infraction_report_task.start() + @staticmethod + def _retryable_filter_load_error(error: Exception) -> bool: + """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" + if isinstance(error, ResponseCodeError): + return error.status == 429 or error.status >= 500 + + return isinstance(error, (TimeoutError, OSError)) + + async def _alert_mods_filter_load_failure(self, error: Exception, attempts: int) -> None: + """Send an alert to mod-alerts when startup fails after all retry attempts.""" + mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) + if mod_alerts_channel is None: + log.error("Failed to send filtering startup failure alert: #mod-alerts channel is unavailable.") + return + + error_details = f"{error.__class__.__name__}: {error}" + if isinstance(error, ResponseCodeError): + error_details = f"HTTP {error.status} - {error_details}" + + try: + await mod_alerts_channel.send( + ":warning: Filtering failed to load filter lists during startup " + f"after {attempts} attempt(s). Error: `{error_details}`" + ) + except discord.HTTPException: + log.exception("Failed to send filtering startup failure alert to #mod-alerts.") + def subscribe(self, filter_list: FilterList, *events: Event) -> None: """ Subscribe a filter list to the given events. From 8c3f009e262420e335d6416cf510f051da4c91ca Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Wed, 25 Feb 2026 20:46:25 +0100 Subject: [PATCH 04/85] feat: add retry with exponential backoff for filter list loading #6 --- bot/exts/filtering/filtering.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index 837b552719..850fbae110 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -1,3 +1,4 @@ +import asyncio import datetime import io import json @@ -110,7 +111,32 @@ async def cog_load(self) -> None: await self.bot.wait_until_guild_available() log.trace("Loading filtering information from the database.") - raw_filter_lists = await self.bot.api_client.get("bot/filter/filter_lists") + for attempt in range(1, FILTER_LOAD_MAX_ATTEMPTS + 1): + try: + raw_filter_lists = await self.bot.api_client.get("bot/filter/filter_lists") + break + except Exception as error: + is_retryable = self._retryable_filter_load_error(error) + is_last_attempt = attempt == FILTER_LOAD_MAX_ATTEMPTS + + if not is_retryable: + raise + + if is_last_attempt: + log.exception("Failed to load filtering data after %d attempts.", FILTER_LOAD_MAX_ATTEMPTS) + await self._alert_mods_filter_load_failure(error, attempt) + raise + + backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) + log.warning( + "Failed to load filtering data (attempt %d/%d). Retrying in %d second(s): %s", + attempt, + FILTER_LOAD_MAX_ATTEMPTS, + backoff_seconds, + error + ) + await asyncio.sleep(backoff_seconds) + example_list = None for raw_filter_list in raw_filter_lists: loaded_list = self._load_raw_filter_list(raw_filter_list) From 92537609398f8f7490b29a308043eb375f1de6a3 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Thu, 26 Feb 2026 11:08:59 +0100 Subject: [PATCH 05/85] test: add tests for invalid extensions and cogs (#3) --- bot/bot.py | 13 ++- tests/bot/exts/test_extensions.py | 157 ++++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 tests/bot/exts/test_extensions.py diff --git a/bot/bot.py b/bot/bot.py index 35dbd1ba4e..5e01824ba9 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -1,5 +1,6 @@ import asyncio import contextlib +import contextvars from sys import exception import aiohttp @@ -13,6 +14,10 @@ log = get_logger("bot") +_current_extension: contextvars.ContextVar[str | None] = contextvars.ContextVar( + "current_extension", default=None +) + class StartupError(Exception): """Exception class for startup errors.""" @@ -26,9 +31,15 @@ class Bot(BotBase): """A subclass of `pydis_core.BotBase` that implements bot-specific functions.""" def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) + # Track extension load failures and tasks so we can report them after all attempts have completed + self.extension_load_failures: dict[str, BaseException] = {} + self._extension_load_tasks: dict[str, asyncio.Task] = {} + + + + async def load_extension(self, name: str, *args, **kwargs) -> None: """Extend D.py's load_extension function to also record sentry performance stats.""" with start_transaction(op="cog-load", name=name): diff --git a/tests/bot/exts/test_extensions.py b/tests/bot/exts/test_extensions.py new file mode 100644 index 0000000000..2546873883 --- /dev/null +++ b/tests/bot/exts/test_extensions.py @@ -0,0 +1,157 @@ +import asyncio +import contextlib +import importlib +import sys +import unittest +import unittest.mock +from pathlib import Path +from tempfile import TemporaryDirectory + +import discord + +from bot.bot import Bot + + +class ExtensionLoadingTests(unittest.IsolatedAsyncioTestCase): + async def asyncSetUp(self) -> None: + self.http_session = unittest.mock.MagicMock(name="http_session") + + # Set up a Bot instance with minimal configuration for testing extension loading. + self.bot = Bot( + command_prefix="!", + guild_id=123456789012345678, + allowed_roles=[], + http_session=self.http_session, + intents=discord.Intents.none() + ) + + # Avoid blocking in _load_extensions() + async def _instant() -> None: + return None + self.bot.wait_until_guild_available = _instant + + # Ensure clean state + self.bot.extension_load_failures = {} + self.bot._extension_load_tasks = {} + + # Temporary importable package: tmp_root/testexts/__init__.py + modules + self._temp_dir = TemporaryDirectory() + self.addCleanup(self._temp_dir.cleanup) + self.tmp_root = Path(self._temp_dir.name) + + self.pkg_name = "testexts" + self.pkg_dir = self.tmp_root / self.pkg_name + self.pkg_dir.mkdir(parents=True, exist_ok=True) + (self.pkg_dir / "__init__.py").write_text("", encoding="utf-8") + + sys.path.insert(0, str(self.tmp_root)) + self.addCleanup(self._remove_tmp_from_syspath) + self._purge_modules(self.pkg_name) + + # Ensure scheduled tasks execute during tests + self._create_task_patcher = unittest.mock.patch( + "pydis_core.utils.scheduling.create_task", + side_effect=lambda coro, *a, **k: asyncio.create_task(coro), + ) + self._create_task_patcher.start() + self.addCleanup(self._create_task_patcher.stop) + + def _remove_tmp_from_syspath(self) -> None: + """Remove the temporary directory from sys.path.""" + with contextlib.suppress(ValueError): + sys.path.remove(str(self.tmp_root)) + + def _purge_modules(self, prefix: str) -> None: + """Remove modules from sys.modules that match the given prefix.""" + for name in list(sys.modules.keys()): + if name == prefix or name.startswith(prefix + "."): + del sys.modules[name] + + def _write_ext(self, module_name: str, source: str) -> str: + """Write an extension module with the given source code and + return its full import path.""" + (self.pkg_dir / f"{module_name}.py").write_text(source, encoding="utf-8") + full = f"{self.pkg_name}.{module_name}" + self._purge_modules(full) + return full + + async def _run_loader(self) -> None: + """Run the extension loader on the package containing our test extensions.""" + module = importlib.import_module(self.pkg_name) + + await self.bot._load_extensions(module) + + # Wait for all extension load tasks to complete so that exceptions are recorded in the bot's state + tasks = getattr(self.bot, "_extension_load_tasks", {}) or {} + if tasks: + await asyncio.gather(*tasks.values(), return_exceptions=True) + + def _assert_failure_recorded_for_extension(self, ext: str) -> None: + """Assert that a failure is recorded for the given extension.""" + if ext in self.bot.extension_load_failures: + return + for key in self.bot.extension_load_failures: + if key.startswith(ext): + return + self.fail( + f"Expected a failure recorded for {ext!r}. " + f"Recorded keys: {sorted(self.bot.extension_load_failures.keys())}" + ) + + async def test_setup_failure_is_captured(self) -> None: + ext = self._write_ext( + "ext_setup_fail", + """ +async def setup(bot): + raise RuntimeError("setup failed") +""", + ) + await self._run_loader() + self._assert_failure_recorded_for_extension(ext) + + async def test_cog_load_failure_is_captured(self) -> None: + ext = self._write_ext( + "ext_cogload_fail", + """ +from discord.ext import commands + +class BadCog(commands.Cog): + async def cog_load(self): + raise RuntimeError("cog_load failed") + +async def setup(bot): + await bot.add_cog(BadCog()) +""", + ) + await self._run_loader() + self._assert_failure_recorded_for_extension(ext) + + async def test_add_cog_failure_is_captured(self) -> None: + ext = self._write_ext( + "ext_addcog_fail", + """ +from discord.ext import commands + +class DupCog(commands.Cog): + pass + +async def setup(bot): + await bot.add_cog(DupCog()) + await bot.add_cog(DupCog()) +""", + ) + await self._run_loader() + self._assert_failure_recorded_for_extension(ext) + + async def test_import_failure_is_captured(self) -> None: + ext = self._write_ext( + "ext_import_fail", + """ +raise RuntimeError("import failed before setup()") + +async def setup(bot): + pass +""", + ) + await self._run_loader() + self._assert_failure_recorded_for_extension(ext) From 96e301845d45bf57b5ba08ef2718094b2a9190fd Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Thu, 26 Feb 2026 11:09:48 +0100 Subject: [PATCH 06/85] fix: updated untracked unit test --- tests/bot/exts/filtering/test_filtering_cog.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 58b111505a..f708cadea0 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -29,7 +29,7 @@ def setUp(self) -> None: async def test_cog_load_when_filter_list_fetch_fails(self): """`cog_load` should currently raise if loading filter lists from the API fails.""" - self.bot.api_client.get.side_effect = RuntimeError("Simulated site/API outage during cog_load") + self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") with self.assertRaises(RuntimeError): await self.cog.cog_load() @@ -41,3 +41,16 @@ async def test_cog_load_when_filter_list_fetch_fails(self): self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() self.cog.schedule_offending_messages_deletion.assert_not_awaited() self.mock_weekly_task_start.assert_not_called() + + async def test_cog_load_completes_when_filter_list_fetch_succeeds(self): + """`cog_load` should continue startup when the API returns filter lists successfully.""" + self.bot.api_client.get.return_value = [] + + await self.cog.cog_load() + + self.bot.wait_until_guild_available.assert_awaited_once() + self.bot.api_client.get.assert_awaited_once_with("bot/filter/filter_lists") + self.cog._fetch_or_generate_filtering_webhook.assert_awaited_once() + self.cog.collect_loaded_types.assert_called_once_with(None) + self.cog.schedule_offending_messages_deletion.assert_awaited_once() + self.mock_weekly_task_start.assert_called_once() From 0bfb84ec95a4f307e73a71bc9e1374dd472555fd Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Thu, 26 Feb 2026 11:12:53 +0100 Subject: [PATCH 07/85] feat: add centralized exception logging for extensions and cogs (#3) --- bot/bot.py | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/bot/bot.py b/bot/bot.py index 5e01824ba9..4184c26cc4 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -1,11 +1,15 @@ import asyncio import contextlib import contextvars +import types from sys import exception import aiohttp from discord.errors import Forbidden +from discord.ext import commands from pydis_core import BotBase +from pydis_core.utils import scheduling +from pydis_core.utils._extensions import walk_extensions from pydis_core.utils.error_handling import handle_forbidden_from_block from sentry_sdk import new_scope, start_transaction @@ -38,7 +42,67 @@ def __init__(self, *args, **kwargs): self._extension_load_tasks: dict[str, asyncio.Task] = {} + async def add_cog(self, cog: commands.Cog) -> None: + """ + Add a cog to the bot with exception handling. + Override of `BotBase.add_cog` to capture and log any exceptions raised during cog loading, + including the extension name if available. + """ + extension = _current_extension.get() + + try: + await super().add_cog(cog) + log.info(f"Cog successfully loaded: {cog.qualified_name}") + + except BaseException as e: + key = extension or f"(unknown)::{cog.qualified_name}" + self.extension_load_failures[key] = e + + log.exception( + "FAILED during add_cog (extension=%s, cog=%s)", + extension, + cog.qualified_name, + ) + # Propagate error + raise + + async def _load_extensions(self, module: types.ModuleType) -> None: + + log.info("Waiting for guild %d to be available before loading extensions.", self.guild_id) + await self.wait_until_guild_available() + + self.all_extensions = walk_extensions(module) + + async def _load_one(extension: str) -> None: + token = _current_extension.set(extension) + + try: + log.info(f"Loading extension: {extension}") + await self.load_extension(extension) + log.info(f"Loaded extension: {extension}") + + except BaseException as e: + self.extension_load_failures[extension] = e + log.exception("FAILED to load extension: %s", extension) + raise + + finally: + _current_extension.reset(token) + + for extension in self.all_extensions: + task = scheduling.create_task(_load_one(extension)) + self._extension_load_tasks[extension] = task + + # Wait for all load tasks to complete so we can report any failures together + await asyncio.gather(*self._extension_load_tasks.values(), return_exceptions=True) + + if self.extension_load_failures: + log.warning( + "Extension/cog load failures (%d): %s", + len(self.extension_load_failures), + ", ".join(sorted(self.extension_load_failures.keys())), + ) async def load_extension(self, name: str, *args, **kwargs) -> None: """Extend D.py's load_extension function to also record sentry performance stats.""" From 0995803ea59c5ff207586c18efdfa765c95fde6d Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 11:14:29 +0100 Subject: [PATCH 08/85] feat: add backoff retry loading (#15) --- bot/exts/utils/reminders.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 1b386ec000..7b78b30ef8 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -1,3 +1,4 @@ +import asyncio import random import textwrap import typing as t @@ -44,6 +45,8 @@ # the 2000-character message limit. MAXIMUM_REMINDER_MENTION_OPT_INS = 80 +# setup constants when loading +MAX_RETRY_ATTEMPTS = 3 Mentionable = discord.Member | discord.Role ReminderMention = UnambiguousUser | discord.Role @@ -223,12 +226,23 @@ async def cog_unload(self) -> None: async def cog_load(self) -> None: """Get all current reminders from the API and reschedule them.""" + delay = 5 # seconds await self.bot.wait_until_guild_available() - response = await self.bot.api_client.get( - "bot/reminders", - params={"active": "true"} - ) - + for attempt in range(1, MAX_RETRY_ATTEMPTS + 1): + try: + # response either throws, or is a list of reminders (possibly empty) + response = await self.bot.api_client.get( + "bot/reminders", + params={"active": "true"} + ) + break + except Exception as e: + log.warning(f"Attempt {attempt} - Failed to fetch reminders from the API: {e}") + if attempt == MAX_RETRY_ATTEMPTS: + log.error("Max retry attempts reached. Failed to load reminders.") + raise + await asyncio.sleep(delay) + delay *= 2 # exponential backoff now = datetime.now(UTC) for reminder in response: From 2fb991a27277ca07db76153ca97f90fe41fbcd79 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Thu, 26 Feb 2026 11:32:47 +0100 Subject: [PATCH 09/85] fix: added filtering cog retry test and fixed cog_load startup tests #13 --- .../bot/exts/filtering/test_filtering_cog.py | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index f708cadea0..45dbe23e2f 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -1,6 +1,8 @@ import unittest from unittest.mock import AsyncMock, MagicMock, patch +from pydis_core.site_api import ResponseCodeError + from bot.exts.filtering.filtering import Filtering @@ -30,12 +32,16 @@ def setUp(self) -> None: async def test_cog_load_when_filter_list_fetch_fails(self): """`cog_load` should currently raise if loading filter lists from the API fails.""" self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") + mock_alerts_channel = MagicMock() + mock_alerts_channel.send = AsyncMock() + self.bot.get_channel.return_value = mock_alerts_channel - with self.assertRaises(RuntimeError): + with self.assertRaises(OSError): await self.cog.cog_load() self.bot.wait_until_guild_available.assert_awaited_once() - self.bot.api_client.get.assert_awaited_once_with("bot/filter/filter_lists") + self.assertEqual(self.bot.api_client.get.await_count, 3) + self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") # Startup should stop before later steps. self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() @@ -49,8 +55,27 @@ async def test_cog_load_completes_when_filter_list_fetch_succeeds(self): await self.cog.cog_load() self.bot.wait_until_guild_available.assert_awaited_once() - self.bot.api_client.get.assert_awaited_once_with("bot/filter/filter_lists") + self.assertEqual(self.bot.api_client.get.await_count, 1) + self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") self.cog._fetch_or_generate_filtering_webhook.assert_awaited_once() self.cog.collect_loaded_types.assert_called_once_with(None) self.cog.schedule_offending_messages_deletion.assert_awaited_once() self.mock_weekly_task_start.assert_called_once() + + def test_retryable_filter_load_error(self): + """`_retryable_filter_load_error` should classify temporary failures as retryable.""" + test_cases = ( + (ResponseCodeError(MagicMock(status=429)), True), + (ResponseCodeError(MagicMock(status=500)), True), + (ResponseCodeError(MagicMock(status=503)), True), + (ResponseCodeError(MagicMock(status=400)), False), + (ResponseCodeError(MagicMock(status=404)), False), + (TimeoutError("timeout"), True), + (OSError("os error"), True), + (AttributeError("attr"), False), + (ValueError("value"), False), + ) + + for error, expected_retryable in test_cases: + with self.subTest(error=error): + self.assertEqual(self.cog._retryable_filter_load_error(error), expected_retryable) From 2fe59dd065a51cb288907e247231f5c7c633698a Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 11:37:22 +0100 Subject: [PATCH 10/85] feat: alert mods through discord (#15) Alerts the moderators through a discord error message if the loading of the Reminders Cog has failed. --- bot/exts/utils/reminders.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 7b78b30ef8..8a37972aab 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -33,6 +33,7 @@ from bot.utils.checks import has_any_role_check, has_no_roles_check from bot.utils.lock import lock_arg from bot.utils.messages import send_denial +from bot.utils.modlog import send_log_message log = get_logger(__name__) @@ -240,11 +241,11 @@ async def cog_load(self) -> None: log.warning(f"Attempt {attempt} - Failed to fetch reminders from the API: {e}") if attempt == MAX_RETRY_ATTEMPTS: log.error("Max retry attempts reached. Failed to load reminders.") + await self._alert_mods_if_loading_failed(e) raise await asyncio.sleep(delay) delay *= 2 # exponential backoff now = datetime.now(UTC) - for reminder in response: is_valid, *_ = self.ensure_valid_reminder(reminder) if not is_valid: @@ -258,6 +259,25 @@ async def cog_load(self) -> None: else: self.schedule_reminder(reminder) + async def _alert_mods_if_loading_failed(self, error: Exception) -> None: + message = textwrap.dedent( + f""" + An error occurred while loading the Reminders Cog, and it failed to initialize properly. + + Error details: + {error} + """ + ) + + await send_log_message( + self.bot, + title="Error: Failed to initialize the Reminders Cog", + text=message, + ping_everyone=True, + icon_url=Icons.token_removed, + colour=discord.Color.red() + ) + def ensure_valid_reminder(self, reminder: dict) -> tuple[bool, discord.TextChannel]: """Ensure reminder channel can be fetched otherwise delete the reminder.""" channel = self.bot.get_channel(reminder["channel_id"]) From 6216c17eb4a2fc94f596cc706be8b4408fc1ad0c Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Thu, 26 Feb 2026 11:52:10 +0100 Subject: [PATCH 11/85] test: Added filtering cog retry-path tests for success and final-failure alerting #13 --- .../bot/exts/filtering/test_filtering_cog.py | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 45dbe23e2f..02c1ecfa41 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -29,39 +29,54 @@ def setUp(self) -> None: self.mock_weekly_task_start = self.start_patcher.start() self.addCleanup(self.start_patcher.stop) - async def test_cog_load_when_filter_list_fetch_fails(self): - """`cog_load` should currently raise if loading filter lists from the API fails.""" + async def test_cog_load_retries_then_succeeds(self): + """`cog_load` should retry temporary failures and complete startup after a successful fetch.""" + self.bot.api_client.get.side_effect = [ + OSError("temporary outage"), + TimeoutError("temporary timeout"), + [], + ] + self.cog._alert_mods_filter_load_failure = AsyncMock() + + with patch("bot.exts.filtering.filtering.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + await self.cog.cog_load() + + self.bot.wait_until_guild_available.assert_awaited_once() + self.assertEqual(self.bot.api_client.get.await_count, 3) + self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") + self.assertEqual(mock_sleep.await_count, 2) + self.cog._alert_mods_filter_load_failure.assert_not_awaited() + self.cog._fetch_or_generate_filtering_webhook.assert_awaited_once() + self.cog.collect_loaded_types.assert_called_once_with(None) + self.cog.schedule_offending_messages_deletion.assert_awaited_once() + self.mock_weekly_task_start.assert_called_once() + + async def test_retries_three_times_fails_and_alerts(self): + """`cog_load` should alert and re-raise when all retry attempts fail.""" self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") - mock_alerts_channel = MagicMock() - mock_alerts_channel.send = AsyncMock() - self.bot.get_channel.return_value = mock_alerts_channel + self.cog._alert_mods_filter_load_failure = AsyncMock() - with self.assertRaises(OSError): + with ( + patch("bot.exts.filtering.filtering.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, + self.assertRaises(OSError), + ): await self.cog.cog_load() self.bot.wait_until_guild_available.assert_awaited_once() self.assertEqual(self.bot.api_client.get.await_count, 3) self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") + self.assertEqual(mock_sleep.await_count, 2) + self.cog._alert_mods_filter_load_failure.assert_awaited_once() + + error, attempts = self.cog._alert_mods_filter_load_failure.await_args.args + self.assertIsInstance(error, OSError) + self.assertEqual(attempts, 3) # Startup should stop before later steps. self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() self.cog.schedule_offending_messages_deletion.assert_not_awaited() self.mock_weekly_task_start.assert_not_called() - async def test_cog_load_completes_when_filter_list_fetch_succeeds(self): - """`cog_load` should continue startup when the API returns filter lists successfully.""" - self.bot.api_client.get.return_value = [] - - await self.cog.cog_load() - - self.bot.wait_until_guild_available.assert_awaited_once() - self.assertEqual(self.bot.api_client.get.await_count, 1) - self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") - self.cog._fetch_or_generate_filtering_webhook.assert_awaited_once() - self.cog.collect_loaded_types.assert_called_once_with(None) - self.cog.schedule_offending_messages_deletion.assert_awaited_once() - self.mock_weekly_task_start.assert_called_once() - def test_retryable_filter_load_error(self): """`_retryable_filter_load_error` should classify temporary failures as retryable.""" test_cases = ( From 685788e269145a21eb00ab3a9622190e4f074683 Mon Sep 17 00:00:00 2001 From: Carl Isaksson Date: Thu, 26 Feb 2026 11:56:56 +0100 Subject: [PATCH 12/85] feat: add helper for checking if error is retryable (#16) --- bot/exts/info/python_news.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index c786a9d192..6478eed3d1 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -15,6 +15,9 @@ from bot.log import get_logger from bot.utils.webhooks import send_webhook +PYTHON_NEWS_LOAD_MAX_ATTEMPTS = 3 +PYTHON_NEWS_INITIAL_BACKOFF_SECONDS = 1 + PEPS_RSS_URL = "https://peps.python.org/peps.rss" RECENT_THREADS_TEMPLATE = "https://mail.python.org/archives/list/{name}@python.org/recent-threads" @@ -44,6 +47,12 @@ def __init__(self, bot: Bot): self.webhook: discord.Webhook | None = None self.seen_items: dict[str, set[str]] = {} + @staticmethod + def _retryable_site_load_error(error: Exception) -> bool: + if isinstance(error, ResponseCodeError): + return error.status == 429 or error.status >= 500 + return isinstance(error, (TimeoutError, OSError)) + async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" with sentry_sdk.start_span(description="Fetch mailing lists from site"): From 7727c307de22ec1900080e9abd02d92134335a74 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Thu, 26 Feb 2026 11:58:20 +0100 Subject: [PATCH 13/85] fix: updated filter_load_max_attempts contant to reuse connect_max_retries from contants.py #13 --- bot/exts/filtering/filtering.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index 850fbae110..0bb297495f 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -65,7 +65,7 @@ HOURS_BETWEEN_NICKNAME_ALERTS = 1 OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=7) WEEKLY_REPORT_ISO_DAY = 3 # 1=Monday, 7=Sunday -FILTER_LOAD_MAX_ATTEMPTS = 3 +FILTER_LOAD_MAX_ATTEMPTS = constants.URLs.connect_max_retries INITIAL_BACKOFF_SECONDS = 1 From aa9ec7aaf3017dd03a318a4faf9a134250464422 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Thu, 26 Feb 2026 12:02:16 +0100 Subject: [PATCH 14/85] fix: added 408: Request Timeout Error, to imply for retryable and it's unit test #13 --- bot/exts/filtering/filtering.py | 2 +- tests/bot/exts/filtering/test_filtering_cog.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index 0bb297495f..5937b4f4d8 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -154,7 +154,7 @@ async def cog_load(self) -> None: def _retryable_filter_load_error(error: Exception) -> bool: """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" if isinstance(error, ResponseCodeError): - return error.status == 429 or error.status >= 500 + return error.status in (408, 429) or error.status >= 500 return isinstance(error, (TimeoutError, OSError)) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 02c1ecfa41..5299f58b81 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -80,6 +80,7 @@ async def test_retries_three_times_fails_and_alerts(self): def test_retryable_filter_load_error(self): """`_retryable_filter_load_error` should classify temporary failures as retryable.""" test_cases = ( + (ResponseCodeError(MagicMock(status=408)), True), (ResponseCodeError(MagicMock(status=429)), True), (ResponseCodeError(MagicMock(status=500)), True), (ResponseCodeError(MagicMock(status=503)), True), From ea30b974ae14f58f92a71ff6efdf156bfe9d3413 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 12:05:14 +0100 Subject: [PATCH 15/85] test: add test skeleton and valid load test (#15) --- tests/bot/exts/utils/test_reminders.py | 31 ++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/bot/exts/utils/test_reminders.py diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py new file mode 100644 index 0000000000..f00952722d --- /dev/null +++ b/tests/bot/exts/utils/test_reminders.py @@ -0,0 +1,31 @@ +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +from bot.exts.utils.reminders import Reminders +from tests.helpers import MockBot + + +class RemindersCogLoadTests(unittest.IsolatedAsyncioTestCase): + """ Tests startup behaviour of the Reminders cog. """ + + def setUp(self): + self.bot = MockBot() + self.bot.wait_until_guild_available = AsyncMock() + self.cog = Reminders(self.bot) + + self.cog._alert_mods_if_loading_failed = AsyncMock() + self.cog.ensure_valid_reminder = MagicMock(return_value=(False, None)) + self.cog.schedule_reminder = MagicMock() + self.cog._alert_mods_if_loading_failed = AsyncMock() + + self.bot.api_client = MagicMock() + self.bot.api_client.get = AsyncMock() + + @patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) + async def test_reminders_cog_loads(self, sleep_mock): + """ Tests if the Reminders cog loads without error if the GET requests works. """ + self.bot.api_client.get.return_value = [] + try: + await self.cog.cog_load() + except Exception as e: + self.fail(f"Reminders cog failed to load with exception: {e}") From bcf9cb90e082d88aaf10c81306a897d92124f243 Mon Sep 17 00:00:00 2001 From: Carl Isaksson Date: Thu, 26 Feb 2026 15:53:11 +0100 Subject: [PATCH 16/85] feat: add retry logic to python_news (#16) Changed cog_load() function to retry connecting to api if it fails initially with an exponential delay and limited max attempts. --- bot/exts/info/python_news.py | 57 ++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 6478eed3d1..53873d5d5c 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -1,3 +1,4 @@ +import asyncio import re import typing as t from datetime import UTC, datetime, timedelta @@ -15,8 +16,8 @@ from bot.log import get_logger from bot.utils.webhooks import send_webhook -PYTHON_NEWS_LOAD_MAX_ATTEMPTS = 3 -PYTHON_NEWS_INITIAL_BACKOFF_SECONDS = 1 +MAX_ATTEMPTS = constants.URLs.connect_max_retries +INITIAL_BACKOFF_SECONDS = 1 PEPS_RSS_URL = "https://peps.python.org/peps.rss" @@ -55,19 +56,45 @@ def _retryable_site_load_error(error: Exception) -> bool: async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" - with sentry_sdk.start_span(description="Fetch mailing lists from site"): - response = await self.bot.api_client.get("bot/mailing-lists") - - for mailing_list in response: - self.seen_items[mailing_list["name"]] = set(mailing_list["seen_items"]) - - with sentry_sdk.start_span(description="Update site with new mailing lists"): - for mailing_list in ("pep", *constants.PythonNews.mail_lists): - if mailing_list not in self.seen_items: - await self.bot.api_client.post("bot/mailing-lists", json={"name": mailing_list}) - self.seen_items[mailing_list] = set() - - self.fetch_new_media.start() + for attempt in range(1, MAX_ATTEMPTS + 1): + try: + with sentry_sdk.start_span(description="Fetch mailing lists from site"): + response = await self.bot.api_client.get("bot/mailing-lists") + + # Rebuild state on each successful fetch (avoid partial state across retries) + self.seen_items = {} + for mailing_list in response: + self.seen_items[mailing_list["name"]] = set(mailing_list["seen_items"]) + + with sentry_sdk.start_span(description="Update site with new mailing lists"): + for mailing_list in ("pep", *constants.PythonNews.mail_lists): + if mailing_list not in self.seen_items: + await self.bot.api_client.post("bot/mailing-lists", json={"name": mailing_list}) + self.seen_items[mailing_list] = set() + + self.fetch_new_media.start() + return + + except Exception as error: + if not self._retryable_site_load_error(error): + raise + + if attempt == MAX_ATTEMPTS: + log.exception( + "Failed to load PythonNews mailing lists after %d attempt(s).", + MAX_ATTEMPTS, + ) + raise + + backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) + log.warning( + "Failed to load PythonNews mailing lists (attempt %d/%d). Retrying in %d second(s). Error: %s", + attempt, + MAX_ATTEMPTS, + backoff_seconds, + error, + ) + await asyncio.sleep(backoff_seconds) async def cog_unload(self) -> None: """Stop news posting tasks on cog unload.""" From 200a4d897d5f5e906f387d4ee966d3fb34b6aab5 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 15:55:31 +0100 Subject: [PATCH 17/85] test: test retry functionality (#15) --- bot/exts/utils/reminders.py | 8 +++---- tests/bot/exts/utils/test_reminders.py | 30 +++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 8a37972aab..37eaf58f3d 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -24,6 +24,7 @@ POSITIVE_REPLIES, Roles, STAFF_AND_COMMUNITY_ROLES, + URLs, ) from bot.converters import Duration, UnambiguousUser from bot.errors import LockedResourceError @@ -47,7 +48,8 @@ MAXIMUM_REMINDER_MENTION_OPT_INS = 80 # setup constants when loading -MAX_RETRY_ATTEMPTS = 3 +MAX_RETRY_ATTEMPTS = URLs.connect_max_retries +BACKOFF_INITIAL_DELAY = 5 # seconds Mentionable = discord.Member | discord.Role ReminderMention = UnambiguousUser | discord.Role @@ -227,7 +229,6 @@ async def cog_unload(self) -> None: async def cog_load(self) -> None: """Get all current reminders from the API and reschedule them.""" - delay = 5 # seconds await self.bot.wait_until_guild_available() for attempt in range(1, MAX_RETRY_ATTEMPTS + 1): try: @@ -243,8 +244,7 @@ async def cog_load(self) -> None: log.error("Max retry attempts reached. Failed to load reminders.") await self._alert_mods_if_loading_failed(e) raise - await asyncio.sleep(delay) - delay *= 2 # exponential backoff + await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) # Exponential backoff now = datetime.now(UTC) for reminder in response: is_valid, *_ = self.ensure_valid_reminder(reminder) diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py index f00952722d..8dfa43064a 100644 --- a/tests/bot/exts/utils/test_reminders.py +++ b/tests/bot/exts/utils/test_reminders.py @@ -1,9 +1,12 @@ import unittest from unittest.mock import AsyncMock, MagicMock, patch +from bot.constants import URLs from bot.exts.utils.reminders import Reminders from tests.helpers import MockBot +MAX_RETRY_ATTEMPTS = URLs.connect_max_retries + class RemindersCogLoadTests(unittest.IsolatedAsyncioTestCase): """ Tests startup behaviour of the Reminders cog. """ @@ -21,11 +24,32 @@ def setUp(self): self.bot.api_client = MagicMock() self.bot.api_client.get = AsyncMock() - @patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) - async def test_reminders_cog_loads(self, sleep_mock): + async def test_reminders_cog_loads_correctly(self): """ Tests if the Reminders cog loads without error if the GET requests works. """ self.bot.api_client.get.return_value = [] try: - await self.cog.cog_load() + with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock): + await self.cog.cog_load() except Exception as e: self.fail(f"Reminders cog failed to load with exception: {e}") + + async def test_reminders_cog_load_retries_after_initial_exception(self): + """ Tests if the Reminders cog loads after retrying on initial exception. """ + self.bot.api_client.get.side_effect = [Exception("fail 1"), Exception("fail 2"), []] + try: + with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + await self.cog.cog_load() + except Exception as e: + self.fail(f"Reminders cog failed to load after retrying with exception: {e}") + self.assertEqual(mock_sleep.await_count, 2) + self.bot.api_client.get.assert_called() + + async def test_reminders_cog_load_fails_after_max_retries(self): + """ Tests if the Reminders cog fails to load after max retries. """ + self.bot.api_client.get.side_effect = Exception("fail") + with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + await self.cog.cog_load() + # Should have retried MAX_RETRY_ATTEMPTS - 1 times before failing + self.assertEqual(mock_sleep.await_count, MAX_RETRY_ATTEMPTS - 1) + self.bot.api_client.get.assert_called() + self.cog._alert_mods_if_loading_failed.assert_called_once() From 43368677a324cb1b7cb0f53ca7fbb3986a6b9e94 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 15:58:52 +0100 Subject: [PATCH 18/85] fix: rewrite test to pass checks (#15) --- tests/bot/exts/utils/test_reminders.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py index 8dfa43064a..1117efc63b 100644 --- a/tests/bot/exts/utils/test_reminders.py +++ b/tests/bot/exts/utils/test_reminders.py @@ -46,9 +46,11 @@ async def test_reminders_cog_load_retries_after_initial_exception(self): async def test_reminders_cog_load_fails_after_max_retries(self): """ Tests if the Reminders cog fails to load after max retries. """ - self.bot.api_client.get.side_effect = Exception("fail") - with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: - await self.cog.cog_load() + self.bot.api_client.get.side_effect = RuntimeError("fail") + with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, \ + self.assertRaises(RuntimeError): + await self.cog.cog_load() + # Should have retried MAX_RETRY_ATTEMPTS - 1 times before failing self.assertEqual(mock_sleep.await_count, MAX_RETRY_ATTEMPTS - 1) self.bot.api_client.get.assert_called() From 9ce2efa3bd86bee244d861e494b9b5d3a8b65954 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 16:03:53 +0100 Subject: [PATCH 19/85] docs: add comments in the cog_load function (#15) --- bot/exts/utils/reminders.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 37eaf58f3d..558560b8ad 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -230,6 +230,7 @@ async def cog_unload(self) -> None: async def cog_load(self) -> None: """Get all current reminders from the API and reschedule them.""" await self.bot.wait_until_guild_available() + # retry fetching reminders with exponential backoff for attempt in range(1, MAX_RETRY_ATTEMPTS + 1): try: # response either throws, or is a list of reminders (possibly empty) From bac8217c13bb3dc5cb2bb846269bc4cfad7d10d6 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 18:05:57 +0100 Subject: [PATCH 20/85] fix: check if the error is retriable (#15) --- bot/exts/utils/reminders.py | 11 +++++++++++ tests/bot/exts/utils/test_reminders.py | 9 ++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 558560b8ad..4f3cc31e59 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -240,6 +240,10 @@ async def cog_load(self) -> None: ) break except Exception as e: + if not self._check_error_is_retriable(e): + log.error(f"Failed to load reminders due to non-retryable error: {e}") + await self._alert_mods_if_loading_failed(e) + raise log.warning(f"Attempt {attempt} - Failed to fetch reminders from the API: {e}") if attempt == MAX_RETRY_ATTEMPTS: log.error("Max retry attempts reached. Failed to load reminders.") @@ -279,6 +283,13 @@ async def _alert_mods_if_loading_failed(self, error: Exception) -> None: colour=discord.Color.red() ) + def _check_error_is_retriable(self, error: Exception) -> bool: + """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" + if isinstance(error, ResponseCodeError): + return error.status in (408, 429) or error.status >= 500 + + return isinstance(error, (TimeoutError, OSError)) + def ensure_valid_reminder(self, reminder: dict) -> tuple[bool, discord.TextChannel]: """Ensure reminder channel can be fetched otherwise delete the reminder.""" channel = self.bot.get_channel(reminder["channel_id"]) diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py index 1117efc63b..04c633cb8f 100644 --- a/tests/bot/exts/utils/test_reminders.py +++ b/tests/bot/exts/utils/test_reminders.py @@ -1,6 +1,8 @@ import unittest from unittest.mock import AsyncMock, MagicMock, patch +from pydis_core.site_api import ResponseCodeError + from bot.constants import URLs from bot.exts.utils.reminders import Reminders from tests.helpers import MockBot @@ -35,7 +37,7 @@ async def test_reminders_cog_loads_correctly(self): async def test_reminders_cog_load_retries_after_initial_exception(self): """ Tests if the Reminders cog loads after retrying on initial exception. """ - self.bot.api_client.get.side_effect = [Exception("fail 1"), Exception("fail 2"), []] + self.bot.api_client.get.side_effect = [OSError("fail1"), OSError("fail2"), []] try: with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: await self.cog.cog_load() @@ -46,9 +48,10 @@ async def test_reminders_cog_load_retries_after_initial_exception(self): async def test_reminders_cog_load_fails_after_max_retries(self): """ Tests if the Reminders cog fails to load after max retries. """ - self.bot.api_client.get.side_effect = RuntimeError("fail") + self.bot.api_client.get.side_effect = ResponseCodeError(response=MagicMock(status=500), + response_text="Internal Server Error") with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, \ - self.assertRaises(RuntimeError): + self.assertRaises(ResponseCodeError): await self.cog.cog_load() # Should have retried MAX_RETRY_ATTEMPTS - 1 times before failing From 4f209a6ccb4fabde60df0a4126bb87f8dc22daeb Mon Sep 17 00:00:00 2001 From: Carl Isaksson Date: Thu, 26 Feb 2026 18:06:39 +0100 Subject: [PATCH 21/85] feat: alert moderators after max attempts (#16) --- bot/exts/info/python_news.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 53873d5d5c..255d875d45 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -51,9 +51,31 @@ def __init__(self, bot: Bot): @staticmethod def _retryable_site_load_error(error: Exception) -> bool: if isinstance(error, ResponseCodeError): - return error.status == 429 or error.status >= 500 + return error.status in (408, 429) or error.status >= 500 return isinstance(error, (TimeoutError, OSError)) + async def _alert_mods_python_news_load_failure( + self, error: Exception, attempts: int + ) -> None: + """Alert moderators if PythonNews fails to load after all retries.""" + channel = self.bot.get_channel(constants.Channels.mod_alerts) + + if channel is None: + log.error("Could not find mod_alerts channel to send PythonNews failure alert.") + return + + status_info = "" + if isinstance(error, ResponseCodeError): + status_info = f" (status {error.status})" + + await channel.send( + ":warning: **PythonNews failed to load.**\n" + f"Attempts: {attempts}\n" + f"Error: `{error.__class__.__name__}{status_info}`\n\n" + "Python news posting will not be active until the bot is restarted " + "or the extension is reloaded." + ) + async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" for attempt in range(1, MAX_ATTEMPTS + 1): @@ -84,6 +106,7 @@ async def cog_load(self) -> None: "Failed to load PythonNews mailing lists after %d attempt(s).", MAX_ATTEMPTS, ) + await self._alert_mods_python_news_load_failure(error, attempt) raise backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) From 3d0ad6cc2fc6dbd1db3179c00f949f24dcca182d Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Fri, 27 Feb 2026 14:41:47 +0100 Subject: [PATCH 22/85] feat: add startup failure status message logs (#3 #5) --- bot/bot.py | 121 ++++++++++++++++----------------- bot/utils/startup_reporting.py | 61 +++++++++++++++++ 2 files changed, 118 insertions(+), 64 deletions(-) create mode 100644 bot/utils/startup_reporting.py diff --git a/bot/bot.py b/bot/bot.py index 4184c26cc4..3d235da920 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -15,6 +15,7 @@ from bot import constants, exts from bot.log import get_logger +from bot.utils.startup_reporting import StartupFailureReporter log = get_logger("bot") @@ -22,7 +23,6 @@ "current_extension", default=None ) - class StartupError(Exception): """Exception class for startup errors.""" @@ -40,69 +40,7 @@ def __init__(self, *args, **kwargs): # Track extension load failures and tasks so we can report them after all attempts have completed self.extension_load_failures: dict[str, BaseException] = {} self._extension_load_tasks: dict[str, asyncio.Task] = {} - - - async def add_cog(self, cog: commands.Cog) -> None: - """ - Add a cog to the bot with exception handling. - - Override of `BotBase.add_cog` to capture and log any exceptions raised during cog loading, - including the extension name if available. - """ - extension = _current_extension.get() - - try: - await super().add_cog(cog) - log.info(f"Cog successfully loaded: {cog.qualified_name}") - - except BaseException as e: - key = extension or f"(unknown)::{cog.qualified_name}" - self.extension_load_failures[key] = e - - log.exception( - "FAILED during add_cog (extension=%s, cog=%s)", - extension, - cog.qualified_name, - ) - # Propagate error - raise - - async def _load_extensions(self, module: types.ModuleType) -> None: - - log.info("Waiting for guild %d to be available before loading extensions.", self.guild_id) - await self.wait_until_guild_available() - - self.all_extensions = walk_extensions(module) - - async def _load_one(extension: str) -> None: - token = _current_extension.set(extension) - - try: - log.info(f"Loading extension: {extension}") - await self.load_extension(extension) - log.info(f"Loaded extension: {extension}") - - except BaseException as e: - self.extension_load_failures[extension] = e - log.exception("FAILED to load extension: %s", extension) - raise - - finally: - _current_extension.reset(token) - - for extension in self.all_extensions: - task = scheduling.create_task(_load_one(extension)) - self._extension_load_tasks[extension] = task - - # Wait for all load tasks to complete so we can report any failures together - await asyncio.gather(*self._extension_load_tasks.values(), return_exceptions=True) - - if self.extension_load_failures: - log.warning( - "Extension/cog load failures (%d): %s", - len(self.extension_load_failures), - ", ".join(sorted(self.extension_load_failures.keys())), - ) + self._startup_failure_reporter = StartupFailureReporter() async def load_extension(self, name: str, *args, **kwargs) -> None: """Extend D.py's load_extension function to also record sentry performance stats.""" @@ -152,3 +90,58 @@ async def on_error(self, event: str, *args, **kwargs) -> None: scope.set_extra("kwargs", kwargs) log.exception(f"Unhandled exception in {event}.") + + async def add_cog(self, cog: commands.Cog) -> None: + """ + Add a cog to the bot with exception handling. + + Override of `BotBase.add_cog` to capture and log any exceptions raised during cog loading, + including the extension name if available. + """ + extension = _current_extension.get() + + try: + await super().add_cog(cog) + log.info(f"Cog successfully loaded: {cog.qualified_name}") + + except BaseException as e: + key = extension or f"(unknown)::{cog.qualified_name}" + self.extension_load_failures[key] = e + + log.exception( + f"Failed during add_cog (extension={extension}, cog={cog.qualified_name})" + ) + # Propagate error + raise + + async def _load_extensions(self, module: types.ModuleType) -> None: + """Load extensions for the bot.""" + await self.wait_until_guild_available() + + self.all_extensions = walk_extensions(module) + + async def _load_one(extension: str) -> None: + token = _current_extension.set(extension) + + try: + await self.load_extension(extension) + log.info(f"Extension successfully loaded: {extension}") + + except BaseException as e: + self.extension_load_failures[extension] = e + log.exception(f"Failed to load extension: {extension}") + raise + + finally: + _current_extension.reset(token) + + for extension in self.all_extensions: + task = scheduling.create_task(_load_one(extension)) + self._extension_load_tasks[extension] = task + + # Wait for all load tasks to complete so we can report any failures together + await asyncio.gather(*self._extension_load_tasks.values(), return_exceptions=True) + + # Send a Discord message to moderators if any extensions failed to load + if self.extension_load_failures : + await self._startup_failure_reporter.notify(self, self.extension_load_failures) diff --git a/bot/utils/startup_reporting.py b/bot/utils/startup_reporting.py new file mode 100644 index 0000000000..b3ad78b67f --- /dev/null +++ b/bot/utils/startup_reporting.py @@ -0,0 +1,61 @@ +import textwrap +from collections.abc import Mapping +from dataclasses import dataclass +from typing import TYPE_CHECKING + +import discord + +from bot.constants import Channels, Icons +from bot.log import get_logger + +log = get_logger(__name__) + +if TYPE_CHECKING: + from bot.bot import Bot + +@dataclass(frozen=True) +class StartupFailureReporter: + """Formats and sends one aggregated startup failure alert to moderators.""" + + async def notify(self, bot: Bot, failures: Mapping[str, BaseException], channel_id: int = Channels.mod_log) -> None: + """Notify moderators of startup failures.""" + if not failures: + return + + if bot.get_channel(channel_id) is None: + # Can't send a message if the channel doesn't exist, so log instead + log.warning("Failed to send startup failure report: mod_log channel not found.") + return + + try: + # Local import avoids circular dependency + from bot.utils.modlog import send_log_message + + text = self.render(failures) + + await send_log_message( + bot, + icon_url=Icons.token_removed, + colour=discord.Colour.red(), + title="Startup: Some extensions failed to load", + text=text, + ping_everyone=True, + channel_id=channel_id + ) + except Exception as e: + log.exception(f"Failed to send startup failure report: {e}") + + def render(self, failures: Mapping[str, BaseException]) -> str: + """Render a human-readable message from the given failures.""" + keys = sorted(failures.keys()) + + lines = [] + lines.append("The following extension(s) failed to load:") + for k in keys: + e = failures[k] + lines.append(f"- **{k}** - `{type(e).__name__}: {e}`") + + return textwrap.dedent(f""" + Failed items: + {chr(10).join(lines)} + """).strip() From c989eae4d2165c9329379e6ce700a6015b197b8c Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Fri, 27 Feb 2026 16:52:14 +0100 Subject: [PATCH 23/85] test: add superstarify tests (#14) Add test cases for retrying cog loads and skeleton for new functions --- .../moderation/infraction/superstarify.py | 11 ++- .../infraction/test_superstarify_cog.py | 82 +++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 tests/bot/exts/moderation/infraction/test_superstarify_cog.py diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 006334755d..57b5733a39 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -10,6 +10,7 @@ from bot import constants from bot.bot import Bot +from bot.constants import URLs from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils @@ -18,6 +19,7 @@ from bot.utils import time from bot.utils.messages import format_user +MAX_RETRY_ATTEMPTS = URLs.connect_max_retries log = get_logger(__name__) NICKNAME_POLICY_URL = "https://pythondiscord.com/pages/rules/#nickname-policy" SUPERSTARIFY_DEFAULT_DURATION = "1h" @@ -238,7 +240,14 @@ async def cog_check(self, ctx: Context) -> bool: """Only allow moderators to invoke the commands in this cog.""" return await has_any_role(*constants.MODERATION_ROLES).predicate(ctx) - + async def _fetch_with_retries(self, + retries: int = MAX_RETRY_ATTEMPTS, + params: dict[str, str] | None = None) -> list[dict]: + return None + async def _alert_mods_if_loading_failed(self, error: Exception) -> None: + pass + async def _check_error_is_retriable(self, error: Exception) -> bool: + return False async def setup(bot: Bot) -> None: """Load the Superstarify cog.""" await bot.add_cog(Superstarify(bot)) diff --git a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py new file mode 100644 index 0000000000..847ce43ba4 --- /dev/null +++ b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py @@ -0,0 +1,82 @@ +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +from bot.exts.moderation.infraction.superstarify import Superstarify +from tests.helpers import MockBot + + +class TestSuperstarify(unittest.IsolatedAsyncioTestCase): + + async def asyncSetUp(self): + self.bot = MockBot() + + self.cog = Superstarify(self.bot) + + self.bot.api_client = MagicMock() + self.bot.api_client.get = AsyncMock() + + self.cog._alert_mods_if_loading_failed = AsyncMock() + self.cog._check_error_is_retriable = MagicMock(return_value=True) + + async def test_fetch_from_api_success(self): + """API succeeds on first attempt.""" + expected = [{"id": 1}] + self.bot.api_client.get.return_value = expected + + result = await self.cog._fetch_with_retries( + params={"user__id": "123"} + ) + self.assertEqual(result, expected) + + self.bot.api_client.get.assert_awaited_once_with( + "bot/infractions", + params={"user__id": "123"}, + ) + self.cog._alert_mods_if_loading_failed.assert_not_called() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_fetch_retries_then_succeeds(self, _): + self.bot.api_client.get.side_effect = [ + OSError("temporary failure"), + [{"id": 42}], + ] + + result = await self.cog._fetch_with_retries( + params={"user__id": "123"} + ) + + self.assertEqual(result, [{"id": 42}]) + self.assertEqual(self.bot.api_client.get.await_count, 2) + + self.cog._alert_mods_if_loading_failed.assert_not_called() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_fetch_fails_after_max_retries(self, _): + error = OSError("API down") + + self.bot.api_client.get.side_effect = error + + with self.assertRaises(OSError): + await self.cog._fetch_with_retries( + retries=3, + params={"user__id": "123"}, + ) + + self.assertEqual(self.bot.api_client.get.await_count, 3) + + self.cog._alert_mods_if_loading_failed.assert_awaited_once_with(error) + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_non_retriable_error_stops_immediately(self, _): + error = ValueError("bad request") + + self.bot.api_client.get.side_effect = error + self.cog._check_error_is_retriable.return_value = False + + with self.assertRaises(ValueError): + await self.cog._fetch_with_retries() + + # only one attempt + self.bot.api_client.get.assert_awaited_once() + + self.cog._alert_mods_if_loading_failed.assert_awaited_once() From 81f8d34c84e2d81de2ddcad500897e2c02f283fe Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Fri, 27 Feb 2026 17:05:54 +0100 Subject: [PATCH 24/85] feat: add logic to functions (#14) Implement skeleton functions with code for retrying fetch, alerting mods, and checking if retryable --- .../moderation/infraction/superstarify.py | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 57b5733a39..8139eaa37d 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -1,16 +1,19 @@ +import asyncio import json import random import textwrap from pathlib import Path +import discord from discord import Embed, Member from discord.ext.commands import Cog, Context, command, has_any_role from discord.utils import escape_markdown +from pydis_core.site_api import ResponseCodeError from pydis_core.utils.members import get_or_fetch_member from bot import constants from bot.bot import Bot -from bot.constants import URLs +from bot.constants import Icons, URLs from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils @@ -18,8 +21,10 @@ from bot.log import get_logger from bot.utils import time from bot.utils.messages import format_user +from bot.utils.modlog import send_log_message MAX_RETRY_ATTEMPTS = URLs.connect_max_retries +BACKOFF_INITIAL_DELAY = 5 # seconds log = get_logger(__name__) NICKNAME_POLICY_URL = "https://pythondiscord.com/pages/rules/#nickname-policy" SUPERSTARIFY_DEFAULT_DURATION = "1h" @@ -45,9 +50,7 @@ async def on_member_update(self, before: Member, after: Member) -> None: f"{after.display_name}. Checking if the user is in superstar-prison..." ) - active_superstarifies = await self.bot.api_client.get( - "bot/infractions", - params={ + active_superstarifies = await self._fetch_with_retries(params={ "active": "true", "type": "superstar", "user__id": str(before.id) @@ -86,9 +89,7 @@ async def on_member_update(self, before: Member, after: Member) -> None: @Cog.listener() async def on_member_join(self, member: Member) -> None: """Reapply active superstar infractions for returning members.""" - active_superstarifies = await self.bot.api_client.get( - "bot/infractions", - params={ + active_superstarifies = await self._fetch_with_retries(params={ "active": "true", "type": "superstar", "user__id": member.id @@ -243,11 +244,43 @@ async def cog_check(self, ctx: Context) -> bool: async def _fetch_with_retries(self, retries: int = MAX_RETRY_ATTEMPTS, params: dict[str, str] | None = None) -> list[dict]: + """Fetch infractions from the API with retries and exponential backoff.""" + for attempt in range(retries): + try: + return await self.bot.api_client.get("bot/infractions", params=params) + except Exception as e: + if attempt == retries - 1 or not self._check_error_is_retriable(e): + await self._alert_mods_if_loading_failed(e) + raise + await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) return None + async def _alert_mods_if_loading_failed(self, error: Exception) -> None: - pass + """Alert moderators that loading the superstarify cog failed after retries.""" + message = textwrap.dedent( + f""" + An error occurred while loading the Superstarify Cog, and it failed to initialize properly. + + Error details: + {error} + """ + ) + + await send_log_message( + self.bot, + title="Error: Failed to initialize the Superstarify Cog", + text=message, + ping_everyone=True, + icon_url=Icons.token_removed, + colour=discord.Color.red() + ) async def _check_error_is_retriable(self, error: Exception) -> bool: - return False + """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" + if isinstance(error, ResponseCodeError): + return error.status in (408, 429) or error.status >= 500 + + return isinstance(error, (TimeoutError, OSError)) + async def setup(bot: Bot) -> None: """Load the Superstarify cog.""" await bot.add_cog(Superstarify(bot)) From 3b8d7eb9f64aa8997d5d018a8ad12a2ce91d9c6a Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Fri, 27 Feb 2026 17:20:30 +0100 Subject: [PATCH 25/85] test: more unit tests for superstarify (#14) Add unit test for on_member_update and unit test to check _alert_mods_if_loading_failed is being called --- .../infraction/test_superstarify_cog.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py index 847ce43ba4..4e3004ce6b 100644 --- a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py +++ b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py @@ -80,3 +80,33 @@ async def test_non_retriable_error_stops_immediately(self, _): self.bot.api_client.get.assert_awaited_once() self.cog._alert_mods_if_loading_failed.assert_awaited_once() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_member_update_recovers_from_api_failure(self, _): + before = MagicMock(display_name="Old", id=123) + after = MagicMock(display_name="New", id=123) + after.edit = AsyncMock() + + self.bot.api_client.get.side_effect = [ + OSError(), + [{"id": 42}], + ] + + self.cog.get_nick = MagicMock(return_value="Taylor Swift") + + with patch( + "bot.exts.moderation.infraction._utils.notify_infraction", + new=AsyncMock(return_value=True), + ): + await self.cog.on_member_update(before, after) + + after.edit.assert_awaited_once() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_alert_triggered_after_total_failure(self, _): + self.bot.api_client.get.side_effect = OSError("down") + + with self.assertRaises(OSError): + await self.cog._fetch_with_retries(retries=3) + + self.cog._alert_mods_if_loading_failed.assert_awaited_once() From 0b89bdb641995663056ba11e3d7cddfaaaf0dcf5 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Sat, 28 Feb 2026 18:31:12 +0100 Subject: [PATCH 26/85] fix: remove duplicate moderator notification in filters extension (#3) --- bot/exts/filtering/filtering.py | 1 - .../bot/exts/filtering/test_filtering_cog.py | 29 ++++++++++--------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index 5937b4f4d8..d9cb2c504b 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -124,7 +124,6 @@ async def cog_load(self) -> None: if is_last_attempt: log.exception("Failed to load filtering data after %d attempts.", FILTER_LOAD_MAX_ATTEMPTS) - await self._alert_mods_filter_load_failure(error, attempt) raise backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 5299f58b81..f9d8536f42 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -51,26 +51,29 @@ async def test_cog_load_retries_then_succeeds(self): self.cog.schedule_offending_messages_deletion.assert_awaited_once() self.mock_weekly_task_start.assert_called_once() - async def test_retries_three_times_fails_and_alerts(self): - """`cog_load` should alert and re-raise when all retry attempts fail.""" - self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") - self.cog._alert_mods_filter_load_failure = AsyncMock() + async def test_retries_three_times_fails_and_reraises(self): + """`cog_load` should retry and re-raise when all retry attempts fail.""" + self.bot.api_client.get.side_effect = OSError( + "Simulated site/API outage during cog_load" + ) - with ( - patch("bot.exts.filtering.filtering.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, - self.assertRaises(OSError), - ): + with patch( + "bot.exts.filtering.filtering.asyncio.sleep", + new_callable=AsyncMock, + ) as mock_sleep, self.assertRaises(OSError) as ctx: await self.cog.cog_load() + self.assertIs(ctx.exception, self.bot.api_client.get.side_effect) + + # Waited for guild availability self.bot.wait_until_guild_available.assert_awaited_once() + + # 3 attempts self.assertEqual(self.bot.api_client.get.await_count, 3) self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") - self.assertEqual(mock_sleep.await_count, 2) - self.cog._alert_mods_filter_load_failure.assert_awaited_once() - error, attempts = self.cog._alert_mods_filter_load_failure.await_args.args - self.assertIsInstance(error, OSError) - self.assertEqual(attempts, 3) + # Backoff between attempts (attempts - 1) + self.assertEqual(mock_sleep.await_count, 2) # Startup should stop before later steps. self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() From 8ecd6023d0a88c437b9c7b854a04961b25396dfe Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Sat, 28 Feb 2026 18:56:20 +0100 Subject: [PATCH 27/85] fix: remove duplicate moderator notification in reminders extension (#3) --- bot/exts/utils/reminders.py | 22 ---------------------- tests/bot/exts/utils/test_reminders.py | 1 - 2 files changed, 23 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 4f3cc31e59..86ab17c76e 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -34,7 +34,6 @@ from bot.utils.checks import has_any_role_check, has_no_roles_check from bot.utils.lock import lock_arg from bot.utils.messages import send_denial -from bot.utils.modlog import send_log_message log = get_logger(__name__) @@ -242,12 +241,10 @@ async def cog_load(self) -> None: except Exception as e: if not self._check_error_is_retriable(e): log.error(f"Failed to load reminders due to non-retryable error: {e}") - await self._alert_mods_if_loading_failed(e) raise log.warning(f"Attempt {attempt} - Failed to fetch reminders from the API: {e}") if attempt == MAX_RETRY_ATTEMPTS: log.error("Max retry attempts reached. Failed to load reminders.") - await self._alert_mods_if_loading_failed(e) raise await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) # Exponential backoff now = datetime.now(UTC) @@ -264,25 +261,6 @@ async def cog_load(self) -> None: else: self.schedule_reminder(reminder) - async def _alert_mods_if_loading_failed(self, error: Exception) -> None: - message = textwrap.dedent( - f""" - An error occurred while loading the Reminders Cog, and it failed to initialize properly. - - Error details: - {error} - """ - ) - - await send_log_message( - self.bot, - title="Error: Failed to initialize the Reminders Cog", - text=message, - ping_everyone=True, - icon_url=Icons.token_removed, - colour=discord.Color.red() - ) - def _check_error_is_retriable(self, error: Exception) -> bool: """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" if isinstance(error, ResponseCodeError): diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py index 04c633cb8f..67c8904fba 100644 --- a/tests/bot/exts/utils/test_reminders.py +++ b/tests/bot/exts/utils/test_reminders.py @@ -57,4 +57,3 @@ async def test_reminders_cog_load_fails_after_max_retries(self): # Should have retried MAX_RETRY_ATTEMPTS - 1 times before failing self.assertEqual(mock_sleep.await_count, MAX_RETRY_ATTEMPTS - 1) self.bot.api_client.get.assert_called() - self.cog._alert_mods_if_loading_failed.assert_called_once() From 768b2faede41ef8fa8f2cd52c94230ebcf19b5d6 Mon Sep 17 00:00:00 2001 From: Carl Isaksson Date: Sat, 28 Feb 2026 21:22:49 +0100 Subject: [PATCH 28/85] test: mocked setup and pythonNews tests (#16) --- tests/bot/exts/info/test_python_news.py | 132 ++++++++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 tests/bot/exts/info/test_python_news.py diff --git a/tests/bot/exts/info/test_python_news.py b/tests/bot/exts/info/test_python_news.py new file mode 100644 index 0000000000..0b47c6d7b3 --- /dev/null +++ b/tests/bot/exts/info/test_python_news.py @@ -0,0 +1,132 @@ +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +from pydis_core.site_api import ResponseCodeError + +from bot.exts.info.python_news import PythonNews + + +class PythonNewsCogLoadTests(unittest.IsolatedAsyncioTestCase): + """Test startup behavior of the PythonNews cog (`cog_load`).""" + + def setUp(self) -> None: + """Set up a PythonNews cog with a mocked bot and stubbed startup dependencies.""" + self.bot = MagicMock() + self.bot.wait_until_guild_available = AsyncMock() + + self.bot.api_client = MagicMock() + self.bot.api_client.get = AsyncMock() + self.bot.api_client.post = AsyncMock() + + # Required by `fetch_new_media` later, but not used in these tests. + self.bot.http_session = MagicMock() + + self.cog = PythonNews(self.bot) + + # Stub out task-loop start, so it doesn't actually schedule anything. + self.start_patcher = patch.object(self.cog.fetch_new_media, "start") + self.mock_fetch_new_media_start = self.start_patcher.start() + self.addCleanup(self.start_patcher.stop) + + async def test_cog_load_retries_then_succeeds(self): + """`cog_load` should retry temporary failures and complete startup after a successful fetch.""" + # First two attempts fail with retryable errors, third succeeds. + self.bot.api_client.get.side_effect = [ + OSError("temporary outage"), + TimeoutError("temporary timeout"), + [ + {"name": "pep", "seen_items": ["1", "2"]}, + ], + ] + + # Ensure no missing mailing lists need creating in this test. + with patch("bot.exts.info.python_news.constants.PythonNews.mail_lists", new=()): + self.cog._alert_mods_python_news_load_failure = AsyncMock() + + with patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + await self.cog.cog_load() + + self.assertEqual(self.bot.api_client.get.await_count, 3) + self.bot.api_client.get.assert_awaited_with("bot/mailing-lists") + + # Sleep should have been awaited for the two failed attempts. + self.assertEqual(mock_sleep.await_count, 2) + + # No final alert on success. + self.cog._alert_mods_python_news_load_failure.assert_not_awaited() + + # Task should start after successful load. + self.mock_fetch_new_media_start.assert_called_once() + + # State should be populated. + self.assertIn("pep", self.cog.seen_items) + self.assertEqual(self.cog.seen_items["pep"], {"1", "2"}) + + # No posts should happen because no missing lists. + self.bot.api_client.post.assert_not_awaited() + + async def test_retries_max_times_fails_and_alerts(self): + """`cog_load` should alert and re-raise when all retry attempts fail.""" + self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") + self.cog._alert_mods_python_news_load_failure = AsyncMock() + + with ( + patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, + self.assertRaises(OSError), + ): + await self.cog.cog_load() + + # Should try exactly MAX_ATTEMPTS times. + from bot.exts.info import python_news as python_news_module + + self.assertEqual(self.bot.api_client.get.await_count, python_news_module.MAX_ATTEMPTS) + self.bot.api_client.get.assert_awaited_with("bot/mailing-lists") + + # Sleeps happen between attempts, so MAX_ATTEMPTS - 1 times. + self.assertEqual(mock_sleep.await_count, python_news_module.MAX_ATTEMPTS - 1) + + # Alert should be sent once at the end. + self.cog._alert_mods_python_news_load_failure.assert_awaited_once() + + error, attempts = self.cog._alert_mods_python_news_load_failure.await_args.args + self.assertIsInstance(error, OSError) + self.assertEqual(attempts, python_news_module.MAX_ATTEMPTS) + + # Task should never start if load fails. + self.mock_fetch_new_media_start.assert_not_called() + + def test_retryable_python_news_load_error(self): + """`_retryable_site_load_error` should classify temporary failures as retryable.""" + test_cases = ( + (ResponseCodeError(MagicMock(status=408)), True), + (ResponseCodeError(MagicMock(status=429)), True), + (ResponseCodeError(MagicMock(status=500)), True), + (ResponseCodeError(MagicMock(status=503)), True), + (ResponseCodeError(MagicMock(status=400)), False), + (ResponseCodeError(MagicMock(status=404)), False), + (TimeoutError("timeout"), True), + (OSError("os error"), True), + (AttributeError("attr"), False), + (ValueError("value"), False), + ) + + for error, expected_retryable in test_cases: + with self.subTest(error=error): + self.assertEqual(self.cog._retryable_site_load_error(error), expected_retryable) + + async def test_cog_load_does_not_retry_non_retryable_error(self): + """`cog_load` should not retry when the error is non-retryable.""" + # 404 should be considered non-retryable by your predicate. + self.bot.api_client.get.side_effect = ResponseCodeError(MagicMock(status=404)) + self.cog._alert_mods_python_news_load_failure = AsyncMock() + + with ( + patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, + self.assertRaises(ResponseCodeError), + ): + await self.cog.cog_load() + + self.assertEqual(self.bot.api_client.get.await_count, 1) + self.assertEqual(mock_sleep.await_count, 0) + self.cog._alert_mods_python_news_load_failure.assert_not_awaited() + self.mock_fetch_new_media_start.assert_not_called() From e534601b6e7286131ca39018b83ce0c81d6e0946 Mon Sep 17 00:00:00 2001 From: Carl Isaksson Date: Sun, 1 Mar 2026 14:20:52 +0100 Subject: [PATCH 29/85] refactor: remove local mod alerting and update tests (#16) --- bot/exts/info/python_news.py | 23 --------------------- tests/bot/exts/info/test_python_news.py | 27 +++++++------------------ 2 files changed, 7 insertions(+), 43 deletions(-) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 255d875d45..2dd607a6ed 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -54,28 +54,6 @@ def _retryable_site_load_error(error: Exception) -> bool: return error.status in (408, 429) or error.status >= 500 return isinstance(error, (TimeoutError, OSError)) - async def _alert_mods_python_news_load_failure( - self, error: Exception, attempts: int - ) -> None: - """Alert moderators if PythonNews fails to load after all retries.""" - channel = self.bot.get_channel(constants.Channels.mod_alerts) - - if channel is None: - log.error("Could not find mod_alerts channel to send PythonNews failure alert.") - return - - status_info = "" - if isinstance(error, ResponseCodeError): - status_info = f" (status {error.status})" - - await channel.send( - ":warning: **PythonNews failed to load.**\n" - f"Attempts: {attempts}\n" - f"Error: `{error.__class__.__name__}{status_info}`\n\n" - "Python news posting will not be active until the bot is restarted " - "or the extension is reloaded." - ) - async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" for attempt in range(1, MAX_ATTEMPTS + 1): @@ -106,7 +84,6 @@ async def cog_load(self) -> None: "Failed to load PythonNews mailing lists after %d attempt(s).", MAX_ATTEMPTS, ) - await self._alert_mods_python_news_load_failure(error, attempt) raise backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) diff --git a/tests/bot/exts/info/test_python_news.py b/tests/bot/exts/info/test_python_news.py index 0b47c6d7b3..273e8d5dc8 100644 --- a/tests/bot/exts/info/test_python_news.py +++ b/tests/bot/exts/info/test_python_news.py @@ -40,11 +40,11 @@ async def test_cog_load_retries_then_succeeds(self): ] # Ensure no missing mailing lists need creating in this test. - with patch("bot.exts.info.python_news.constants.PythonNews.mail_lists", new=()): - self.cog._alert_mods_python_news_load_failure = AsyncMock() - - with patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: - await self.cog.cog_load() + with ( + patch("bot.exts.info.python_news.constants.PythonNews.mail_lists", new=()), + patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, + ): + await self.cog.cog_load() self.assertEqual(self.bot.api_client.get.await_count, 3) self.bot.api_client.get.assert_awaited_with("bot/mailing-lists") @@ -52,9 +52,6 @@ async def test_cog_load_retries_then_succeeds(self): # Sleep should have been awaited for the two failed attempts. self.assertEqual(mock_sleep.await_count, 2) - # No final alert on success. - self.cog._alert_mods_python_news_load_failure.assert_not_awaited() - # Task should start after successful load. self.mock_fetch_new_media_start.assert_called_once() @@ -65,10 +62,9 @@ async def test_cog_load_retries_then_succeeds(self): # No posts should happen because no missing lists. self.bot.api_client.post.assert_not_awaited() - async def test_retries_max_times_fails_and_alerts(self): - """`cog_load` should alert and re-raise when all retry attempts fail.""" + async def test_retries_max_times_fails_and_reraises(self): + """`cog_load` should re-raise when all retry attempts fail.""" self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") - self.cog._alert_mods_python_news_load_failure = AsyncMock() with ( patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, @@ -85,13 +81,6 @@ async def test_retries_max_times_fails_and_alerts(self): # Sleeps happen between attempts, so MAX_ATTEMPTS - 1 times. self.assertEqual(mock_sleep.await_count, python_news_module.MAX_ATTEMPTS - 1) - # Alert should be sent once at the end. - self.cog._alert_mods_python_news_load_failure.assert_awaited_once() - - error, attempts = self.cog._alert_mods_python_news_load_failure.await_args.args - self.assertIsInstance(error, OSError) - self.assertEqual(attempts, python_news_module.MAX_ATTEMPTS) - # Task should never start if load fails. self.mock_fetch_new_media_start.assert_not_called() @@ -118,7 +107,6 @@ async def test_cog_load_does_not_retry_non_retryable_error(self): """`cog_load` should not retry when the error is non-retryable.""" # 404 should be considered non-retryable by your predicate. self.bot.api_client.get.side_effect = ResponseCodeError(MagicMock(status=404)) - self.cog._alert_mods_python_news_load_failure = AsyncMock() with ( patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, @@ -128,5 +116,4 @@ async def test_cog_load_does_not_retry_non_retryable_error(self): self.assertEqual(self.bot.api_client.get.await_count, 1) self.assertEqual(mock_sleep.await_count, 0) - self.cog._alert_mods_python_news_load_failure.assert_not_awaited() self.mock_fetch_new_media_start.assert_not_called() From 4e203c2bb0d8b55b183196ac905a9eddbe2782ce Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Sun, 1 Mar 2026 14:53:15 +0100 Subject: [PATCH 30/85] refactor: remove _alert_mods_if_loading_failed() (#14) Remove redundant code and corresponding parts of unit tests. --- .../moderation/infraction/superstarify.py | 24 +------------------ .../infraction/test_superstarify_cog.py | 7 ------ 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 8139eaa37d..180a49d304 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -4,7 +4,6 @@ import textwrap from pathlib import Path -import discord from discord import Embed, Member from discord.ext.commands import Cog, Context, command, has_any_role from discord.utils import escape_markdown @@ -13,7 +12,7 @@ from bot import constants from bot.bot import Bot -from bot.constants import Icons, URLs +from bot.constants import URLs from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils @@ -21,7 +20,6 @@ from bot.log import get_logger from bot.utils import time from bot.utils.messages import format_user -from bot.utils.modlog import send_log_message MAX_RETRY_ATTEMPTS = URLs.connect_max_retries BACKOFF_INITIAL_DELAY = 5 # seconds @@ -250,30 +248,10 @@ async def _fetch_with_retries(self, return await self.bot.api_client.get("bot/infractions", params=params) except Exception as e: if attempt == retries - 1 or not self._check_error_is_retriable(e): - await self._alert_mods_if_loading_failed(e) raise await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) return None - async def _alert_mods_if_loading_failed(self, error: Exception) -> None: - """Alert moderators that loading the superstarify cog failed after retries.""" - message = textwrap.dedent( - f""" - An error occurred while loading the Superstarify Cog, and it failed to initialize properly. - - Error details: - {error} - """ - ) - - await send_log_message( - self.bot, - title="Error: Failed to initialize the Superstarify Cog", - text=message, - ping_everyone=True, - icon_url=Icons.token_removed, - colour=discord.Color.red() - ) async def _check_error_is_retriable(self, error: Exception) -> bool: """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" if isinstance(error, ResponseCodeError): diff --git a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py index 4e3004ce6b..54473c7064 100644 --- a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py +++ b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py @@ -15,7 +15,6 @@ async def asyncSetUp(self): self.bot.api_client = MagicMock() self.bot.api_client.get = AsyncMock() - self.cog._alert_mods_if_loading_failed = AsyncMock() self.cog._check_error_is_retriable = MagicMock(return_value=True) async def test_fetch_from_api_success(self): @@ -32,7 +31,6 @@ async def test_fetch_from_api_success(self): "bot/infractions", params={"user__id": "123"}, ) - self.cog._alert_mods_if_loading_failed.assert_not_called() @patch("asyncio.sleep", new_callable=AsyncMock) async def test_fetch_retries_then_succeeds(self, _): @@ -48,7 +46,6 @@ async def test_fetch_retries_then_succeeds(self, _): self.assertEqual(result, [{"id": 42}]) self.assertEqual(self.bot.api_client.get.await_count, 2) - self.cog._alert_mods_if_loading_failed.assert_not_called() @patch("asyncio.sleep", new_callable=AsyncMock) async def test_fetch_fails_after_max_retries(self, _): @@ -64,7 +61,6 @@ async def test_fetch_fails_after_max_retries(self, _): self.assertEqual(self.bot.api_client.get.await_count, 3) - self.cog._alert_mods_if_loading_failed.assert_awaited_once_with(error) @patch("asyncio.sleep", new_callable=AsyncMock) async def test_non_retriable_error_stops_immediately(self, _): @@ -79,7 +75,6 @@ async def test_non_retriable_error_stops_immediately(self, _): # only one attempt self.bot.api_client.get.assert_awaited_once() - self.cog._alert_mods_if_loading_failed.assert_awaited_once() @patch("asyncio.sleep", new_callable=AsyncMock) async def test_member_update_recovers_from_api_failure(self, _): @@ -108,5 +103,3 @@ async def test_alert_triggered_after_total_failure(self, _): with self.assertRaises(OSError): await self.cog._fetch_with_retries(retries=3) - - self.cog._alert_mods_if_loading_failed.assert_awaited_once() From 8428c67e321bd2059de31cba9dba5f7997c7f185 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Thu, 26 Feb 2026 11:08:59 +0100 Subject: [PATCH 31/85] test: add tests for invalid extensions and cogs (#3) --- bot/bot.py | 13 ++- tests/bot/exts/test_extensions.py | 157 ++++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 tests/bot/exts/test_extensions.py diff --git a/bot/bot.py b/bot/bot.py index 35dbd1ba4e..5e01824ba9 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -1,5 +1,6 @@ import asyncio import contextlib +import contextvars from sys import exception import aiohttp @@ -13,6 +14,10 @@ log = get_logger("bot") +_current_extension: contextvars.ContextVar[str | None] = contextvars.ContextVar( + "current_extension", default=None +) + class StartupError(Exception): """Exception class for startup errors.""" @@ -26,9 +31,15 @@ class Bot(BotBase): """A subclass of `pydis_core.BotBase` that implements bot-specific functions.""" def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) + # Track extension load failures and tasks so we can report them after all attempts have completed + self.extension_load_failures: dict[str, BaseException] = {} + self._extension_load_tasks: dict[str, asyncio.Task] = {} + + + + async def load_extension(self, name: str, *args, **kwargs) -> None: """Extend D.py's load_extension function to also record sentry performance stats.""" with start_transaction(op="cog-load", name=name): diff --git a/tests/bot/exts/test_extensions.py b/tests/bot/exts/test_extensions.py new file mode 100644 index 0000000000..2546873883 --- /dev/null +++ b/tests/bot/exts/test_extensions.py @@ -0,0 +1,157 @@ +import asyncio +import contextlib +import importlib +import sys +import unittest +import unittest.mock +from pathlib import Path +from tempfile import TemporaryDirectory + +import discord + +from bot.bot import Bot + + +class ExtensionLoadingTests(unittest.IsolatedAsyncioTestCase): + async def asyncSetUp(self) -> None: + self.http_session = unittest.mock.MagicMock(name="http_session") + + # Set up a Bot instance with minimal configuration for testing extension loading. + self.bot = Bot( + command_prefix="!", + guild_id=123456789012345678, + allowed_roles=[], + http_session=self.http_session, + intents=discord.Intents.none() + ) + + # Avoid blocking in _load_extensions() + async def _instant() -> None: + return None + self.bot.wait_until_guild_available = _instant + + # Ensure clean state + self.bot.extension_load_failures = {} + self.bot._extension_load_tasks = {} + + # Temporary importable package: tmp_root/testexts/__init__.py + modules + self._temp_dir = TemporaryDirectory() + self.addCleanup(self._temp_dir.cleanup) + self.tmp_root = Path(self._temp_dir.name) + + self.pkg_name = "testexts" + self.pkg_dir = self.tmp_root / self.pkg_name + self.pkg_dir.mkdir(parents=True, exist_ok=True) + (self.pkg_dir / "__init__.py").write_text("", encoding="utf-8") + + sys.path.insert(0, str(self.tmp_root)) + self.addCleanup(self._remove_tmp_from_syspath) + self._purge_modules(self.pkg_name) + + # Ensure scheduled tasks execute during tests + self._create_task_patcher = unittest.mock.patch( + "pydis_core.utils.scheduling.create_task", + side_effect=lambda coro, *a, **k: asyncio.create_task(coro), + ) + self._create_task_patcher.start() + self.addCleanup(self._create_task_patcher.stop) + + def _remove_tmp_from_syspath(self) -> None: + """Remove the temporary directory from sys.path.""" + with contextlib.suppress(ValueError): + sys.path.remove(str(self.tmp_root)) + + def _purge_modules(self, prefix: str) -> None: + """Remove modules from sys.modules that match the given prefix.""" + for name in list(sys.modules.keys()): + if name == prefix or name.startswith(prefix + "."): + del sys.modules[name] + + def _write_ext(self, module_name: str, source: str) -> str: + """Write an extension module with the given source code and + return its full import path.""" + (self.pkg_dir / f"{module_name}.py").write_text(source, encoding="utf-8") + full = f"{self.pkg_name}.{module_name}" + self._purge_modules(full) + return full + + async def _run_loader(self) -> None: + """Run the extension loader on the package containing our test extensions.""" + module = importlib.import_module(self.pkg_name) + + await self.bot._load_extensions(module) + + # Wait for all extension load tasks to complete so that exceptions are recorded in the bot's state + tasks = getattr(self.bot, "_extension_load_tasks", {}) or {} + if tasks: + await asyncio.gather(*tasks.values(), return_exceptions=True) + + def _assert_failure_recorded_for_extension(self, ext: str) -> None: + """Assert that a failure is recorded for the given extension.""" + if ext in self.bot.extension_load_failures: + return + for key in self.bot.extension_load_failures: + if key.startswith(ext): + return + self.fail( + f"Expected a failure recorded for {ext!r}. " + f"Recorded keys: {sorted(self.bot.extension_load_failures.keys())}" + ) + + async def test_setup_failure_is_captured(self) -> None: + ext = self._write_ext( + "ext_setup_fail", + """ +async def setup(bot): + raise RuntimeError("setup failed") +""", + ) + await self._run_loader() + self._assert_failure_recorded_for_extension(ext) + + async def test_cog_load_failure_is_captured(self) -> None: + ext = self._write_ext( + "ext_cogload_fail", + """ +from discord.ext import commands + +class BadCog(commands.Cog): + async def cog_load(self): + raise RuntimeError("cog_load failed") + +async def setup(bot): + await bot.add_cog(BadCog()) +""", + ) + await self._run_loader() + self._assert_failure_recorded_for_extension(ext) + + async def test_add_cog_failure_is_captured(self) -> None: + ext = self._write_ext( + "ext_addcog_fail", + """ +from discord.ext import commands + +class DupCog(commands.Cog): + pass + +async def setup(bot): + await bot.add_cog(DupCog()) + await bot.add_cog(DupCog()) +""", + ) + await self._run_loader() + self._assert_failure_recorded_for_extension(ext) + + async def test_import_failure_is_captured(self) -> None: + ext = self._write_ext( + "ext_import_fail", + """ +raise RuntimeError("import failed before setup()") + +async def setup(bot): + pass +""", + ) + await self._run_loader() + self._assert_failure_recorded_for_extension(ext) From b5782ab069c711308bcee5c78d2bfdd3dd5d6f36 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Thu, 26 Feb 2026 11:12:53 +0100 Subject: [PATCH 32/85] feat: add centralized exception logging for extensions and cogs (#3) --- bot/bot.py | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/bot/bot.py b/bot/bot.py index 5e01824ba9..4184c26cc4 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -1,11 +1,15 @@ import asyncio import contextlib import contextvars +import types from sys import exception import aiohttp from discord.errors import Forbidden +from discord.ext import commands from pydis_core import BotBase +from pydis_core.utils import scheduling +from pydis_core.utils._extensions import walk_extensions from pydis_core.utils.error_handling import handle_forbidden_from_block from sentry_sdk import new_scope, start_transaction @@ -38,7 +42,67 @@ def __init__(self, *args, **kwargs): self._extension_load_tasks: dict[str, asyncio.Task] = {} + async def add_cog(self, cog: commands.Cog) -> None: + """ + Add a cog to the bot with exception handling. + Override of `BotBase.add_cog` to capture and log any exceptions raised during cog loading, + including the extension name if available. + """ + extension = _current_extension.get() + + try: + await super().add_cog(cog) + log.info(f"Cog successfully loaded: {cog.qualified_name}") + + except BaseException as e: + key = extension or f"(unknown)::{cog.qualified_name}" + self.extension_load_failures[key] = e + + log.exception( + "FAILED during add_cog (extension=%s, cog=%s)", + extension, + cog.qualified_name, + ) + # Propagate error + raise + + async def _load_extensions(self, module: types.ModuleType) -> None: + + log.info("Waiting for guild %d to be available before loading extensions.", self.guild_id) + await self.wait_until_guild_available() + + self.all_extensions = walk_extensions(module) + + async def _load_one(extension: str) -> None: + token = _current_extension.set(extension) + + try: + log.info(f"Loading extension: {extension}") + await self.load_extension(extension) + log.info(f"Loaded extension: {extension}") + + except BaseException as e: + self.extension_load_failures[extension] = e + log.exception("FAILED to load extension: %s", extension) + raise + + finally: + _current_extension.reset(token) + + for extension in self.all_extensions: + task = scheduling.create_task(_load_one(extension)) + self._extension_load_tasks[extension] = task + + # Wait for all load tasks to complete so we can report any failures together + await asyncio.gather(*self._extension_load_tasks.values(), return_exceptions=True) + + if self.extension_load_failures: + log.warning( + "Extension/cog load failures (%d): %s", + len(self.extension_load_failures), + ", ".join(sorted(self.extension_load_failures.keys())), + ) async def load_extension(self, name: str, *args, **kwargs) -> None: """Extend D.py's load_extension function to also record sentry performance stats.""" From 50cdacc5915cf0c0ae21d66c2756fdeb6bc56dc8 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Fri, 27 Feb 2026 14:41:47 +0100 Subject: [PATCH 33/85] feat: add startup failure status message logs (#3 #5) --- bot/bot.py | 121 ++++++++++++++++----------------- bot/utils/startup_reporting.py | 61 +++++++++++++++++ 2 files changed, 118 insertions(+), 64 deletions(-) create mode 100644 bot/utils/startup_reporting.py diff --git a/bot/bot.py b/bot/bot.py index 4184c26cc4..3d235da920 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -15,6 +15,7 @@ from bot import constants, exts from bot.log import get_logger +from bot.utils.startup_reporting import StartupFailureReporter log = get_logger("bot") @@ -22,7 +23,6 @@ "current_extension", default=None ) - class StartupError(Exception): """Exception class for startup errors.""" @@ -40,69 +40,7 @@ def __init__(self, *args, **kwargs): # Track extension load failures and tasks so we can report them after all attempts have completed self.extension_load_failures: dict[str, BaseException] = {} self._extension_load_tasks: dict[str, asyncio.Task] = {} - - - async def add_cog(self, cog: commands.Cog) -> None: - """ - Add a cog to the bot with exception handling. - - Override of `BotBase.add_cog` to capture and log any exceptions raised during cog loading, - including the extension name if available. - """ - extension = _current_extension.get() - - try: - await super().add_cog(cog) - log.info(f"Cog successfully loaded: {cog.qualified_name}") - - except BaseException as e: - key = extension or f"(unknown)::{cog.qualified_name}" - self.extension_load_failures[key] = e - - log.exception( - "FAILED during add_cog (extension=%s, cog=%s)", - extension, - cog.qualified_name, - ) - # Propagate error - raise - - async def _load_extensions(self, module: types.ModuleType) -> None: - - log.info("Waiting for guild %d to be available before loading extensions.", self.guild_id) - await self.wait_until_guild_available() - - self.all_extensions = walk_extensions(module) - - async def _load_one(extension: str) -> None: - token = _current_extension.set(extension) - - try: - log.info(f"Loading extension: {extension}") - await self.load_extension(extension) - log.info(f"Loaded extension: {extension}") - - except BaseException as e: - self.extension_load_failures[extension] = e - log.exception("FAILED to load extension: %s", extension) - raise - - finally: - _current_extension.reset(token) - - for extension in self.all_extensions: - task = scheduling.create_task(_load_one(extension)) - self._extension_load_tasks[extension] = task - - # Wait for all load tasks to complete so we can report any failures together - await asyncio.gather(*self._extension_load_tasks.values(), return_exceptions=True) - - if self.extension_load_failures: - log.warning( - "Extension/cog load failures (%d): %s", - len(self.extension_load_failures), - ", ".join(sorted(self.extension_load_failures.keys())), - ) + self._startup_failure_reporter = StartupFailureReporter() async def load_extension(self, name: str, *args, **kwargs) -> None: """Extend D.py's load_extension function to also record sentry performance stats.""" @@ -152,3 +90,58 @@ async def on_error(self, event: str, *args, **kwargs) -> None: scope.set_extra("kwargs", kwargs) log.exception(f"Unhandled exception in {event}.") + + async def add_cog(self, cog: commands.Cog) -> None: + """ + Add a cog to the bot with exception handling. + + Override of `BotBase.add_cog` to capture and log any exceptions raised during cog loading, + including the extension name if available. + """ + extension = _current_extension.get() + + try: + await super().add_cog(cog) + log.info(f"Cog successfully loaded: {cog.qualified_name}") + + except BaseException as e: + key = extension or f"(unknown)::{cog.qualified_name}" + self.extension_load_failures[key] = e + + log.exception( + f"Failed during add_cog (extension={extension}, cog={cog.qualified_name})" + ) + # Propagate error + raise + + async def _load_extensions(self, module: types.ModuleType) -> None: + """Load extensions for the bot.""" + await self.wait_until_guild_available() + + self.all_extensions = walk_extensions(module) + + async def _load_one(extension: str) -> None: + token = _current_extension.set(extension) + + try: + await self.load_extension(extension) + log.info(f"Extension successfully loaded: {extension}") + + except BaseException as e: + self.extension_load_failures[extension] = e + log.exception(f"Failed to load extension: {extension}") + raise + + finally: + _current_extension.reset(token) + + for extension in self.all_extensions: + task = scheduling.create_task(_load_one(extension)) + self._extension_load_tasks[extension] = task + + # Wait for all load tasks to complete so we can report any failures together + await asyncio.gather(*self._extension_load_tasks.values(), return_exceptions=True) + + # Send a Discord message to moderators if any extensions failed to load + if self.extension_load_failures : + await self._startup_failure_reporter.notify(self, self.extension_load_failures) diff --git a/bot/utils/startup_reporting.py b/bot/utils/startup_reporting.py new file mode 100644 index 0000000000..b3ad78b67f --- /dev/null +++ b/bot/utils/startup_reporting.py @@ -0,0 +1,61 @@ +import textwrap +from collections.abc import Mapping +from dataclasses import dataclass +from typing import TYPE_CHECKING + +import discord + +from bot.constants import Channels, Icons +from bot.log import get_logger + +log = get_logger(__name__) + +if TYPE_CHECKING: + from bot.bot import Bot + +@dataclass(frozen=True) +class StartupFailureReporter: + """Formats and sends one aggregated startup failure alert to moderators.""" + + async def notify(self, bot: Bot, failures: Mapping[str, BaseException], channel_id: int = Channels.mod_log) -> None: + """Notify moderators of startup failures.""" + if not failures: + return + + if bot.get_channel(channel_id) is None: + # Can't send a message if the channel doesn't exist, so log instead + log.warning("Failed to send startup failure report: mod_log channel not found.") + return + + try: + # Local import avoids circular dependency + from bot.utils.modlog import send_log_message + + text = self.render(failures) + + await send_log_message( + bot, + icon_url=Icons.token_removed, + colour=discord.Colour.red(), + title="Startup: Some extensions failed to load", + text=text, + ping_everyone=True, + channel_id=channel_id + ) + except Exception as e: + log.exception(f"Failed to send startup failure report: {e}") + + def render(self, failures: Mapping[str, BaseException]) -> str: + """Render a human-readable message from the given failures.""" + keys = sorted(failures.keys()) + + lines = [] + lines.append("The following extension(s) failed to load:") + for k in keys: + e = failures[k] + lines.append(f"- **{k}** - `{type(e).__name__}: {e}`") + + return textwrap.dedent(f""" + Failed items: + {chr(10).join(lines)} + """).strip() From f22028bfadedd20c09f408c048f92aa03805b366 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Wed, 25 Feb 2026 15:50:15 +0100 Subject: [PATCH 34/85] test: add Filtering cog_load and test scaffold and mocked setup --- .../bot/exts/filtering/test_filtering_cog.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 tests/bot/exts/filtering/test_filtering_cog.py diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py new file mode 100644 index 0000000000..2faafa00cd --- /dev/null +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -0,0 +1,28 @@ +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +from bot.exts.filtering.filtering import Filtering + + +class FilteringCogLoadTests(unittest.IsolatedAsyncioTestCase): + """Test startup behavior of the Filtering cog (`cog_load`).""" + + def setUp(self) -> None: + """Set up a Filtering cog with a mocked bot and stubbed startup dependencies.""" + self.bot = MagicMock() + self.bot.wait_until_guild_available = AsyncMock() + + self.bot.api_client = MagicMock() + self.bot.api_client.get = AsyncMock() + + self.cog = Filtering(self.bot) + + # Stub internals that are not relevant to this unit test. + self.cog.collect_loaded_types = MagicMock() + self.cog.schedule_offending_messages_deletion = AsyncMock() + self.cog._fetch_or_generate_filtering_webhook = AsyncMock(return_value=MagicMock()) + + # `weekly_auto_infraction_report_task` is a discord task loop; patch its start method. + self.start_patcher = patch.object(self.cog.weekly_auto_infraction_report_task, "start") + self.mock_weekly_task_start = self.start_patcher.start() + self.addCleanup(self.start_patcher.stop) From f4d398b462ba77b26ee3ea148c31eff8d45ae379 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Wed, 25 Feb 2026 15:53:19 +0100 Subject: [PATCH 35/85] test: cover cog_load behavior when filter list fetch fails --- tests/bot/exts/filtering/test_filtering_cog.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 2faafa00cd..58b111505a 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -26,3 +26,18 @@ def setUp(self) -> None: self.start_patcher = patch.object(self.cog.weekly_auto_infraction_report_task, "start") self.mock_weekly_task_start = self.start_patcher.start() self.addCleanup(self.start_patcher.stop) + + async def test_cog_load_when_filter_list_fetch_fails(self): + """`cog_load` should currently raise if loading filter lists from the API fails.""" + self.bot.api_client.get.side_effect = RuntimeError("Simulated site/API outage during cog_load") + + with self.assertRaises(RuntimeError): + await self.cog.cog_load() + + self.bot.wait_until_guild_available.assert_awaited_once() + self.bot.api_client.get.assert_awaited_once_with("bot/filter/filter_lists") + + # Startup should stop before later steps. + self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() + self.cog.schedule_offending_messages_deletion.assert_not_awaited() + self.mock_weekly_task_start.assert_not_called() From 43f1c5b882d8159c5bd03aa93b179c958fa09e1e Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Wed, 25 Feb 2026 20:42:22 +0100 Subject: [PATCH 36/85] feat: Add retry error filter and mod alert for filter load failures #6 --- bot/exts/filtering/filtering.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index 210ae3fb05..837b552719 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -64,6 +64,8 @@ HOURS_BETWEEN_NICKNAME_ALERTS = 1 OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=7) WEEKLY_REPORT_ISO_DAY = 3 # 1=Monday, 7=Sunday +FILTER_LOAD_MAX_ATTEMPTS = 3 +INITIAL_BACKOFF_SECONDS = 1 async def _extract_text_file_content(att: discord.Attachment) -> str: @@ -122,6 +124,33 @@ async def cog_load(self) -> None: await self.schedule_offending_messages_deletion() self.weekly_auto_infraction_report_task.start() + @staticmethod + def _retryable_filter_load_error(error: Exception) -> bool: + """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" + if isinstance(error, ResponseCodeError): + return error.status == 429 or error.status >= 500 + + return isinstance(error, (TimeoutError, OSError)) + + async def _alert_mods_filter_load_failure(self, error: Exception, attempts: int) -> None: + """Send an alert to mod-alerts when startup fails after all retry attempts.""" + mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) + if mod_alerts_channel is None: + log.error("Failed to send filtering startup failure alert: #mod-alerts channel is unavailable.") + return + + error_details = f"{error.__class__.__name__}: {error}" + if isinstance(error, ResponseCodeError): + error_details = f"HTTP {error.status} - {error_details}" + + try: + await mod_alerts_channel.send( + ":warning: Filtering failed to load filter lists during startup " + f"after {attempts} attempt(s). Error: `{error_details}`" + ) + except discord.HTTPException: + log.exception("Failed to send filtering startup failure alert to #mod-alerts.") + def subscribe(self, filter_list: FilterList, *events: Event) -> None: """ Subscribe a filter list to the given events. From 8b575f6ecc4a3458468e3c6aaff8db7ced76315f Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Wed, 25 Feb 2026 20:46:25 +0100 Subject: [PATCH 37/85] feat: add retry with exponential backoff for filter list loading #6 --- bot/exts/filtering/filtering.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index 837b552719..850fbae110 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -1,3 +1,4 @@ +import asyncio import datetime import io import json @@ -110,7 +111,32 @@ async def cog_load(self) -> None: await self.bot.wait_until_guild_available() log.trace("Loading filtering information from the database.") - raw_filter_lists = await self.bot.api_client.get("bot/filter/filter_lists") + for attempt in range(1, FILTER_LOAD_MAX_ATTEMPTS + 1): + try: + raw_filter_lists = await self.bot.api_client.get("bot/filter/filter_lists") + break + except Exception as error: + is_retryable = self._retryable_filter_load_error(error) + is_last_attempt = attempt == FILTER_LOAD_MAX_ATTEMPTS + + if not is_retryable: + raise + + if is_last_attempt: + log.exception("Failed to load filtering data after %d attempts.", FILTER_LOAD_MAX_ATTEMPTS) + await self._alert_mods_filter_load_failure(error, attempt) + raise + + backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) + log.warning( + "Failed to load filtering data (attempt %d/%d). Retrying in %d second(s): %s", + attempt, + FILTER_LOAD_MAX_ATTEMPTS, + backoff_seconds, + error + ) + await asyncio.sleep(backoff_seconds) + example_list = None for raw_filter_list in raw_filter_lists: loaded_list = self._load_raw_filter_list(raw_filter_list) From 2cbb226a769b1f1a38158571cb3c40497124f4f1 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Thu, 26 Feb 2026 11:09:48 +0100 Subject: [PATCH 38/85] fix: updated untracked unit test --- tests/bot/exts/filtering/test_filtering_cog.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 58b111505a..f708cadea0 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -29,7 +29,7 @@ def setUp(self) -> None: async def test_cog_load_when_filter_list_fetch_fails(self): """`cog_load` should currently raise if loading filter lists from the API fails.""" - self.bot.api_client.get.side_effect = RuntimeError("Simulated site/API outage during cog_load") + self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") with self.assertRaises(RuntimeError): await self.cog.cog_load() @@ -41,3 +41,16 @@ async def test_cog_load_when_filter_list_fetch_fails(self): self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() self.cog.schedule_offending_messages_deletion.assert_not_awaited() self.mock_weekly_task_start.assert_not_called() + + async def test_cog_load_completes_when_filter_list_fetch_succeeds(self): + """`cog_load` should continue startup when the API returns filter lists successfully.""" + self.bot.api_client.get.return_value = [] + + await self.cog.cog_load() + + self.bot.wait_until_guild_available.assert_awaited_once() + self.bot.api_client.get.assert_awaited_once_with("bot/filter/filter_lists") + self.cog._fetch_or_generate_filtering_webhook.assert_awaited_once() + self.cog.collect_loaded_types.assert_called_once_with(None) + self.cog.schedule_offending_messages_deletion.assert_awaited_once() + self.mock_weekly_task_start.assert_called_once() From f10d6fa134e306436a9a222a287e376d223af55e Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Thu, 26 Feb 2026 11:32:47 +0100 Subject: [PATCH 39/85] fix: added filtering cog retry test and fixed cog_load startup tests #13 --- .../bot/exts/filtering/test_filtering_cog.py | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index f708cadea0..45dbe23e2f 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -1,6 +1,8 @@ import unittest from unittest.mock import AsyncMock, MagicMock, patch +from pydis_core.site_api import ResponseCodeError + from bot.exts.filtering.filtering import Filtering @@ -30,12 +32,16 @@ def setUp(self) -> None: async def test_cog_load_when_filter_list_fetch_fails(self): """`cog_load` should currently raise if loading filter lists from the API fails.""" self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") + mock_alerts_channel = MagicMock() + mock_alerts_channel.send = AsyncMock() + self.bot.get_channel.return_value = mock_alerts_channel - with self.assertRaises(RuntimeError): + with self.assertRaises(OSError): await self.cog.cog_load() self.bot.wait_until_guild_available.assert_awaited_once() - self.bot.api_client.get.assert_awaited_once_with("bot/filter/filter_lists") + self.assertEqual(self.bot.api_client.get.await_count, 3) + self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") # Startup should stop before later steps. self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() @@ -49,8 +55,27 @@ async def test_cog_load_completes_when_filter_list_fetch_succeeds(self): await self.cog.cog_load() self.bot.wait_until_guild_available.assert_awaited_once() - self.bot.api_client.get.assert_awaited_once_with("bot/filter/filter_lists") + self.assertEqual(self.bot.api_client.get.await_count, 1) + self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") self.cog._fetch_or_generate_filtering_webhook.assert_awaited_once() self.cog.collect_loaded_types.assert_called_once_with(None) self.cog.schedule_offending_messages_deletion.assert_awaited_once() self.mock_weekly_task_start.assert_called_once() + + def test_retryable_filter_load_error(self): + """`_retryable_filter_load_error` should classify temporary failures as retryable.""" + test_cases = ( + (ResponseCodeError(MagicMock(status=429)), True), + (ResponseCodeError(MagicMock(status=500)), True), + (ResponseCodeError(MagicMock(status=503)), True), + (ResponseCodeError(MagicMock(status=400)), False), + (ResponseCodeError(MagicMock(status=404)), False), + (TimeoutError("timeout"), True), + (OSError("os error"), True), + (AttributeError("attr"), False), + (ValueError("value"), False), + ) + + for error, expected_retryable in test_cases: + with self.subTest(error=error): + self.assertEqual(self.cog._retryable_filter_load_error(error), expected_retryable) From 92fc44216f43f68905ea9b6db6ba5520b556a397 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Thu, 26 Feb 2026 11:52:10 +0100 Subject: [PATCH 40/85] test: Added filtering cog retry-path tests for success and final-failure alerting #13 --- .../bot/exts/filtering/test_filtering_cog.py | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 45dbe23e2f..02c1ecfa41 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -29,39 +29,54 @@ def setUp(self) -> None: self.mock_weekly_task_start = self.start_patcher.start() self.addCleanup(self.start_patcher.stop) - async def test_cog_load_when_filter_list_fetch_fails(self): - """`cog_load` should currently raise if loading filter lists from the API fails.""" + async def test_cog_load_retries_then_succeeds(self): + """`cog_load` should retry temporary failures and complete startup after a successful fetch.""" + self.bot.api_client.get.side_effect = [ + OSError("temporary outage"), + TimeoutError("temporary timeout"), + [], + ] + self.cog._alert_mods_filter_load_failure = AsyncMock() + + with patch("bot.exts.filtering.filtering.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + await self.cog.cog_load() + + self.bot.wait_until_guild_available.assert_awaited_once() + self.assertEqual(self.bot.api_client.get.await_count, 3) + self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") + self.assertEqual(mock_sleep.await_count, 2) + self.cog._alert_mods_filter_load_failure.assert_not_awaited() + self.cog._fetch_or_generate_filtering_webhook.assert_awaited_once() + self.cog.collect_loaded_types.assert_called_once_with(None) + self.cog.schedule_offending_messages_deletion.assert_awaited_once() + self.mock_weekly_task_start.assert_called_once() + + async def test_retries_three_times_fails_and_alerts(self): + """`cog_load` should alert and re-raise when all retry attempts fail.""" self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") - mock_alerts_channel = MagicMock() - mock_alerts_channel.send = AsyncMock() - self.bot.get_channel.return_value = mock_alerts_channel + self.cog._alert_mods_filter_load_failure = AsyncMock() - with self.assertRaises(OSError): + with ( + patch("bot.exts.filtering.filtering.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, + self.assertRaises(OSError), + ): await self.cog.cog_load() self.bot.wait_until_guild_available.assert_awaited_once() self.assertEqual(self.bot.api_client.get.await_count, 3) self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") + self.assertEqual(mock_sleep.await_count, 2) + self.cog._alert_mods_filter_load_failure.assert_awaited_once() + + error, attempts = self.cog._alert_mods_filter_load_failure.await_args.args + self.assertIsInstance(error, OSError) + self.assertEqual(attempts, 3) # Startup should stop before later steps. self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() self.cog.schedule_offending_messages_deletion.assert_not_awaited() self.mock_weekly_task_start.assert_not_called() - async def test_cog_load_completes_when_filter_list_fetch_succeeds(self): - """`cog_load` should continue startup when the API returns filter lists successfully.""" - self.bot.api_client.get.return_value = [] - - await self.cog.cog_load() - - self.bot.wait_until_guild_available.assert_awaited_once() - self.assertEqual(self.bot.api_client.get.await_count, 1) - self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") - self.cog._fetch_or_generate_filtering_webhook.assert_awaited_once() - self.cog.collect_loaded_types.assert_called_once_with(None) - self.cog.schedule_offending_messages_deletion.assert_awaited_once() - self.mock_weekly_task_start.assert_called_once() - def test_retryable_filter_load_error(self): """`_retryable_filter_load_error` should classify temporary failures as retryable.""" test_cases = ( From 71bf092ce09a25fc49216ebce4f52eb6739e816d Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Thu, 26 Feb 2026 11:58:20 +0100 Subject: [PATCH 41/85] fix: updated filter_load_max_attempts contant to reuse connect_max_retries from contants.py #13 --- bot/exts/filtering/filtering.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index 850fbae110..0bb297495f 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -65,7 +65,7 @@ HOURS_BETWEEN_NICKNAME_ALERTS = 1 OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=7) WEEKLY_REPORT_ISO_DAY = 3 # 1=Monday, 7=Sunday -FILTER_LOAD_MAX_ATTEMPTS = 3 +FILTER_LOAD_MAX_ATTEMPTS = constants.URLs.connect_max_retries INITIAL_BACKOFF_SECONDS = 1 From 0f81c172af46f705058d37c5c79a99138c9df641 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Thu, 26 Feb 2026 12:02:16 +0100 Subject: [PATCH 42/85] fix: added 408: Request Timeout Error, to imply for retryable and it's unit test #13 --- bot/exts/filtering/filtering.py | 2 +- tests/bot/exts/filtering/test_filtering_cog.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index 0bb297495f..5937b4f4d8 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -154,7 +154,7 @@ async def cog_load(self) -> None: def _retryable_filter_load_error(error: Exception) -> bool: """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" if isinstance(error, ResponseCodeError): - return error.status == 429 or error.status >= 500 + return error.status in (408, 429) or error.status >= 500 return isinstance(error, (TimeoutError, OSError)) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 02c1ecfa41..5299f58b81 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -80,6 +80,7 @@ async def test_retries_three_times_fails_and_alerts(self): def test_retryable_filter_load_error(self): """`_retryable_filter_load_error` should classify temporary failures as retryable.""" test_cases = ( + (ResponseCodeError(MagicMock(status=408)), True), (ResponseCodeError(MagicMock(status=429)), True), (ResponseCodeError(MagicMock(status=500)), True), (ResponseCodeError(MagicMock(status=503)), True), From feff4b21d9cb81d867c541bfeb9f030fdb87b7bd Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Sat, 28 Feb 2026 18:31:12 +0100 Subject: [PATCH 43/85] fix: remove duplicate moderator notification in filters extension (#3) --- bot/exts/filtering/filtering.py | 1 - .../bot/exts/filtering/test_filtering_cog.py | 29 ++++++++++--------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index 5937b4f4d8..d9cb2c504b 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -124,7 +124,6 @@ async def cog_load(self) -> None: if is_last_attempt: log.exception("Failed to load filtering data after %d attempts.", FILTER_LOAD_MAX_ATTEMPTS) - await self._alert_mods_filter_load_failure(error, attempt) raise backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 5299f58b81..f9d8536f42 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -51,26 +51,29 @@ async def test_cog_load_retries_then_succeeds(self): self.cog.schedule_offending_messages_deletion.assert_awaited_once() self.mock_weekly_task_start.assert_called_once() - async def test_retries_three_times_fails_and_alerts(self): - """`cog_load` should alert and re-raise when all retry attempts fail.""" - self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") - self.cog._alert_mods_filter_load_failure = AsyncMock() + async def test_retries_three_times_fails_and_reraises(self): + """`cog_load` should retry and re-raise when all retry attempts fail.""" + self.bot.api_client.get.side_effect = OSError( + "Simulated site/API outage during cog_load" + ) - with ( - patch("bot.exts.filtering.filtering.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, - self.assertRaises(OSError), - ): + with patch( + "bot.exts.filtering.filtering.asyncio.sleep", + new_callable=AsyncMock, + ) as mock_sleep, self.assertRaises(OSError) as ctx: await self.cog.cog_load() + self.assertIs(ctx.exception, self.bot.api_client.get.side_effect) + + # Waited for guild availability self.bot.wait_until_guild_available.assert_awaited_once() + + # 3 attempts self.assertEqual(self.bot.api_client.get.await_count, 3) self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") - self.assertEqual(mock_sleep.await_count, 2) - self.cog._alert_mods_filter_load_failure.assert_awaited_once() - error, attempts = self.cog._alert_mods_filter_load_failure.await_args.args - self.assertIsInstance(error, OSError) - self.assertEqual(attempts, 3) + # Backoff between attempts (attempts - 1) + self.assertEqual(mock_sleep.await_count, 2) # Startup should stop before later steps. self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() From e38f365392ff8a44fd5c23eda38c03caec01e15c Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 11:14:29 +0100 Subject: [PATCH 44/85] feat: add backoff retry loading (#15) --- bot/exts/utils/reminders.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 1b386ec000..7b78b30ef8 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -1,3 +1,4 @@ +import asyncio import random import textwrap import typing as t @@ -44,6 +45,8 @@ # the 2000-character message limit. MAXIMUM_REMINDER_MENTION_OPT_INS = 80 +# setup constants when loading +MAX_RETRY_ATTEMPTS = 3 Mentionable = discord.Member | discord.Role ReminderMention = UnambiguousUser | discord.Role @@ -223,12 +226,23 @@ async def cog_unload(self) -> None: async def cog_load(self) -> None: """Get all current reminders from the API and reschedule them.""" + delay = 5 # seconds await self.bot.wait_until_guild_available() - response = await self.bot.api_client.get( - "bot/reminders", - params={"active": "true"} - ) - + for attempt in range(1, MAX_RETRY_ATTEMPTS + 1): + try: + # response either throws, or is a list of reminders (possibly empty) + response = await self.bot.api_client.get( + "bot/reminders", + params={"active": "true"} + ) + break + except Exception as e: + log.warning(f"Attempt {attempt} - Failed to fetch reminders from the API: {e}") + if attempt == MAX_RETRY_ATTEMPTS: + log.error("Max retry attempts reached. Failed to load reminders.") + raise + await asyncio.sleep(delay) + delay *= 2 # exponential backoff now = datetime.now(UTC) for reminder in response: From e098a665e14fde1e42162e87ef1716885a853a23 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 11:37:22 +0100 Subject: [PATCH 45/85] feat: alert mods through discord (#15) Alerts the moderators through a discord error message if the loading of the Reminders Cog has failed. --- bot/exts/utils/reminders.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 7b78b30ef8..8a37972aab 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -33,6 +33,7 @@ from bot.utils.checks import has_any_role_check, has_no_roles_check from bot.utils.lock import lock_arg from bot.utils.messages import send_denial +from bot.utils.modlog import send_log_message log = get_logger(__name__) @@ -240,11 +241,11 @@ async def cog_load(self) -> None: log.warning(f"Attempt {attempt} - Failed to fetch reminders from the API: {e}") if attempt == MAX_RETRY_ATTEMPTS: log.error("Max retry attempts reached. Failed to load reminders.") + await self._alert_mods_if_loading_failed(e) raise await asyncio.sleep(delay) delay *= 2 # exponential backoff now = datetime.now(UTC) - for reminder in response: is_valid, *_ = self.ensure_valid_reminder(reminder) if not is_valid: @@ -258,6 +259,25 @@ async def cog_load(self) -> None: else: self.schedule_reminder(reminder) + async def _alert_mods_if_loading_failed(self, error: Exception) -> None: + message = textwrap.dedent( + f""" + An error occurred while loading the Reminders Cog, and it failed to initialize properly. + + Error details: + {error} + """ + ) + + await send_log_message( + self.bot, + title="Error: Failed to initialize the Reminders Cog", + text=message, + ping_everyone=True, + icon_url=Icons.token_removed, + colour=discord.Color.red() + ) + def ensure_valid_reminder(self, reminder: dict) -> tuple[bool, discord.TextChannel]: """Ensure reminder channel can be fetched otherwise delete the reminder.""" channel = self.bot.get_channel(reminder["channel_id"]) From 4754f19b34851f75f6feb0ab8d0d515c602cae38 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 12:05:14 +0100 Subject: [PATCH 46/85] test: add test skeleton and valid load test (#15) --- tests/bot/exts/utils/test_reminders.py | 31 ++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/bot/exts/utils/test_reminders.py diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py new file mode 100644 index 0000000000..f00952722d --- /dev/null +++ b/tests/bot/exts/utils/test_reminders.py @@ -0,0 +1,31 @@ +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +from bot.exts.utils.reminders import Reminders +from tests.helpers import MockBot + + +class RemindersCogLoadTests(unittest.IsolatedAsyncioTestCase): + """ Tests startup behaviour of the Reminders cog. """ + + def setUp(self): + self.bot = MockBot() + self.bot.wait_until_guild_available = AsyncMock() + self.cog = Reminders(self.bot) + + self.cog._alert_mods_if_loading_failed = AsyncMock() + self.cog.ensure_valid_reminder = MagicMock(return_value=(False, None)) + self.cog.schedule_reminder = MagicMock() + self.cog._alert_mods_if_loading_failed = AsyncMock() + + self.bot.api_client = MagicMock() + self.bot.api_client.get = AsyncMock() + + @patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) + async def test_reminders_cog_loads(self, sleep_mock): + """ Tests if the Reminders cog loads without error if the GET requests works. """ + self.bot.api_client.get.return_value = [] + try: + await self.cog.cog_load() + except Exception as e: + self.fail(f"Reminders cog failed to load with exception: {e}") From 6d9fc0cd56883b74279587863827fa69e4cf403d Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 15:55:31 +0100 Subject: [PATCH 47/85] test: test retry functionality (#15) --- bot/exts/utils/reminders.py | 8 +++---- tests/bot/exts/utils/test_reminders.py | 30 +++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 8a37972aab..37eaf58f3d 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -24,6 +24,7 @@ POSITIVE_REPLIES, Roles, STAFF_AND_COMMUNITY_ROLES, + URLs, ) from bot.converters import Duration, UnambiguousUser from bot.errors import LockedResourceError @@ -47,7 +48,8 @@ MAXIMUM_REMINDER_MENTION_OPT_INS = 80 # setup constants when loading -MAX_RETRY_ATTEMPTS = 3 +MAX_RETRY_ATTEMPTS = URLs.connect_max_retries +BACKOFF_INITIAL_DELAY = 5 # seconds Mentionable = discord.Member | discord.Role ReminderMention = UnambiguousUser | discord.Role @@ -227,7 +229,6 @@ async def cog_unload(self) -> None: async def cog_load(self) -> None: """Get all current reminders from the API and reschedule them.""" - delay = 5 # seconds await self.bot.wait_until_guild_available() for attempt in range(1, MAX_RETRY_ATTEMPTS + 1): try: @@ -243,8 +244,7 @@ async def cog_load(self) -> None: log.error("Max retry attempts reached. Failed to load reminders.") await self._alert_mods_if_loading_failed(e) raise - await asyncio.sleep(delay) - delay *= 2 # exponential backoff + await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) # Exponential backoff now = datetime.now(UTC) for reminder in response: is_valid, *_ = self.ensure_valid_reminder(reminder) diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py index f00952722d..8dfa43064a 100644 --- a/tests/bot/exts/utils/test_reminders.py +++ b/tests/bot/exts/utils/test_reminders.py @@ -1,9 +1,12 @@ import unittest from unittest.mock import AsyncMock, MagicMock, patch +from bot.constants import URLs from bot.exts.utils.reminders import Reminders from tests.helpers import MockBot +MAX_RETRY_ATTEMPTS = URLs.connect_max_retries + class RemindersCogLoadTests(unittest.IsolatedAsyncioTestCase): """ Tests startup behaviour of the Reminders cog. """ @@ -21,11 +24,32 @@ def setUp(self): self.bot.api_client = MagicMock() self.bot.api_client.get = AsyncMock() - @patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) - async def test_reminders_cog_loads(self, sleep_mock): + async def test_reminders_cog_loads_correctly(self): """ Tests if the Reminders cog loads without error if the GET requests works. """ self.bot.api_client.get.return_value = [] try: - await self.cog.cog_load() + with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock): + await self.cog.cog_load() except Exception as e: self.fail(f"Reminders cog failed to load with exception: {e}") + + async def test_reminders_cog_load_retries_after_initial_exception(self): + """ Tests if the Reminders cog loads after retrying on initial exception. """ + self.bot.api_client.get.side_effect = [Exception("fail 1"), Exception("fail 2"), []] + try: + with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + await self.cog.cog_load() + except Exception as e: + self.fail(f"Reminders cog failed to load after retrying with exception: {e}") + self.assertEqual(mock_sleep.await_count, 2) + self.bot.api_client.get.assert_called() + + async def test_reminders_cog_load_fails_after_max_retries(self): + """ Tests if the Reminders cog fails to load after max retries. """ + self.bot.api_client.get.side_effect = Exception("fail") + with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + await self.cog.cog_load() + # Should have retried MAX_RETRY_ATTEMPTS - 1 times before failing + self.assertEqual(mock_sleep.await_count, MAX_RETRY_ATTEMPTS - 1) + self.bot.api_client.get.assert_called() + self.cog._alert_mods_if_loading_failed.assert_called_once() From e1c3796697b83dd5ac5010df030a5ffc58cee00b Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 15:58:52 +0100 Subject: [PATCH 48/85] fix: rewrite test to pass checks (#15) --- tests/bot/exts/utils/test_reminders.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py index 8dfa43064a..1117efc63b 100644 --- a/tests/bot/exts/utils/test_reminders.py +++ b/tests/bot/exts/utils/test_reminders.py @@ -46,9 +46,11 @@ async def test_reminders_cog_load_retries_after_initial_exception(self): async def test_reminders_cog_load_fails_after_max_retries(self): """ Tests if the Reminders cog fails to load after max retries. """ - self.bot.api_client.get.side_effect = Exception("fail") - with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: - await self.cog.cog_load() + self.bot.api_client.get.side_effect = RuntimeError("fail") + with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, \ + self.assertRaises(RuntimeError): + await self.cog.cog_load() + # Should have retried MAX_RETRY_ATTEMPTS - 1 times before failing self.assertEqual(mock_sleep.await_count, MAX_RETRY_ATTEMPTS - 1) self.bot.api_client.get.assert_called() From 49e96398f2b76cc8ca6d42616cb621f0901b4efe Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 16:03:53 +0100 Subject: [PATCH 49/85] docs: add comments in the cog_load function (#15) --- bot/exts/utils/reminders.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 37eaf58f3d..558560b8ad 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -230,6 +230,7 @@ async def cog_unload(self) -> None: async def cog_load(self) -> None: """Get all current reminders from the API and reschedule them.""" await self.bot.wait_until_guild_available() + # retry fetching reminders with exponential backoff for attempt in range(1, MAX_RETRY_ATTEMPTS + 1): try: # response either throws, or is a list of reminders (possibly empty) From abaccdfb9cbb5c25cd0a2ef7046bf8662f22e764 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Thu, 26 Feb 2026 18:05:57 +0100 Subject: [PATCH 50/85] fix: check if the error is retriable (#15) --- bot/exts/utils/reminders.py | 11 +++++++++++ tests/bot/exts/utils/test_reminders.py | 9 ++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 558560b8ad..4f3cc31e59 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -240,6 +240,10 @@ async def cog_load(self) -> None: ) break except Exception as e: + if not self._check_error_is_retriable(e): + log.error(f"Failed to load reminders due to non-retryable error: {e}") + await self._alert_mods_if_loading_failed(e) + raise log.warning(f"Attempt {attempt} - Failed to fetch reminders from the API: {e}") if attempt == MAX_RETRY_ATTEMPTS: log.error("Max retry attempts reached. Failed to load reminders.") @@ -279,6 +283,13 @@ async def _alert_mods_if_loading_failed(self, error: Exception) -> None: colour=discord.Color.red() ) + def _check_error_is_retriable(self, error: Exception) -> bool: + """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" + if isinstance(error, ResponseCodeError): + return error.status in (408, 429) or error.status >= 500 + + return isinstance(error, (TimeoutError, OSError)) + def ensure_valid_reminder(self, reminder: dict) -> tuple[bool, discord.TextChannel]: """Ensure reminder channel can be fetched otherwise delete the reminder.""" channel = self.bot.get_channel(reminder["channel_id"]) diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py index 1117efc63b..04c633cb8f 100644 --- a/tests/bot/exts/utils/test_reminders.py +++ b/tests/bot/exts/utils/test_reminders.py @@ -1,6 +1,8 @@ import unittest from unittest.mock import AsyncMock, MagicMock, patch +from pydis_core.site_api import ResponseCodeError + from bot.constants import URLs from bot.exts.utils.reminders import Reminders from tests.helpers import MockBot @@ -35,7 +37,7 @@ async def test_reminders_cog_loads_correctly(self): async def test_reminders_cog_load_retries_after_initial_exception(self): """ Tests if the Reminders cog loads after retrying on initial exception. """ - self.bot.api_client.get.side_effect = [Exception("fail 1"), Exception("fail 2"), []] + self.bot.api_client.get.side_effect = [OSError("fail1"), OSError("fail2"), []] try: with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: await self.cog.cog_load() @@ -46,9 +48,10 @@ async def test_reminders_cog_load_retries_after_initial_exception(self): async def test_reminders_cog_load_fails_after_max_retries(self): """ Tests if the Reminders cog fails to load after max retries. """ - self.bot.api_client.get.side_effect = RuntimeError("fail") + self.bot.api_client.get.side_effect = ResponseCodeError(response=MagicMock(status=500), + response_text="Internal Server Error") with patch("bot.exts.utils.reminders.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, \ - self.assertRaises(RuntimeError): + self.assertRaises(ResponseCodeError): await self.cog.cog_load() # Should have retried MAX_RETRY_ATTEMPTS - 1 times before failing From 673d486e3fb6f66ab9f7e395a7d170c133e6e2bd Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Sat, 28 Feb 2026 18:56:20 +0100 Subject: [PATCH 51/85] fix: remove duplicate moderator notification in reminders extension (#3) --- bot/exts/utils/reminders.py | 22 ---------------------- tests/bot/exts/utils/test_reminders.py | 1 - 2 files changed, 23 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 4f3cc31e59..86ab17c76e 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -34,7 +34,6 @@ from bot.utils.checks import has_any_role_check, has_no_roles_check from bot.utils.lock import lock_arg from bot.utils.messages import send_denial -from bot.utils.modlog import send_log_message log = get_logger(__name__) @@ -242,12 +241,10 @@ async def cog_load(self) -> None: except Exception as e: if not self._check_error_is_retriable(e): log.error(f"Failed to load reminders due to non-retryable error: {e}") - await self._alert_mods_if_loading_failed(e) raise log.warning(f"Attempt {attempt} - Failed to fetch reminders from the API: {e}") if attempt == MAX_RETRY_ATTEMPTS: log.error("Max retry attempts reached. Failed to load reminders.") - await self._alert_mods_if_loading_failed(e) raise await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) # Exponential backoff now = datetime.now(UTC) @@ -264,25 +261,6 @@ async def cog_load(self) -> None: else: self.schedule_reminder(reminder) - async def _alert_mods_if_loading_failed(self, error: Exception) -> None: - message = textwrap.dedent( - f""" - An error occurred while loading the Reminders Cog, and it failed to initialize properly. - - Error details: - {error} - """ - ) - - await send_log_message( - self.bot, - title="Error: Failed to initialize the Reminders Cog", - text=message, - ping_everyone=True, - icon_url=Icons.token_removed, - colour=discord.Color.red() - ) - def _check_error_is_retriable(self, error: Exception) -> bool: """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" if isinstance(error, ResponseCodeError): diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py index 04c633cb8f..67c8904fba 100644 --- a/tests/bot/exts/utils/test_reminders.py +++ b/tests/bot/exts/utils/test_reminders.py @@ -57,4 +57,3 @@ async def test_reminders_cog_load_fails_after_max_retries(self): # Should have retried MAX_RETRY_ATTEMPTS - 1 times before failing self.assertEqual(mock_sleep.await_count, MAX_RETRY_ATTEMPTS - 1) self.bot.api_client.get.assert_called() - self.cog._alert_mods_if_loading_failed.assert_called_once() From 97cdf38ec4c2fcbe1415b67c40b24b2cbfeddf37 Mon Sep 17 00:00:00 2001 From: Carl Isaksson Date: Thu, 26 Feb 2026 11:56:56 +0100 Subject: [PATCH 52/85] feat: add helper for checking if error is retryable (#16) --- bot/exts/info/python_news.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index c786a9d192..6478eed3d1 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -15,6 +15,9 @@ from bot.log import get_logger from bot.utils.webhooks import send_webhook +PYTHON_NEWS_LOAD_MAX_ATTEMPTS = 3 +PYTHON_NEWS_INITIAL_BACKOFF_SECONDS = 1 + PEPS_RSS_URL = "https://peps.python.org/peps.rss" RECENT_THREADS_TEMPLATE = "https://mail.python.org/archives/list/{name}@python.org/recent-threads" @@ -44,6 +47,12 @@ def __init__(self, bot: Bot): self.webhook: discord.Webhook | None = None self.seen_items: dict[str, set[str]] = {} + @staticmethod + def _retryable_site_load_error(error: Exception) -> bool: + if isinstance(error, ResponseCodeError): + return error.status == 429 or error.status >= 500 + return isinstance(error, (TimeoutError, OSError)) + async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" with sentry_sdk.start_span(description="Fetch mailing lists from site"): From 972d2981fb685a76d0c3b6adc6e8e50abdcecd6b Mon Sep 17 00:00:00 2001 From: Carl Isaksson Date: Thu, 26 Feb 2026 15:53:11 +0100 Subject: [PATCH 53/85] feat: add retry logic to python_news (#16) Changed cog_load() function to retry connecting to api if it fails initially with an exponential delay and limited max attempts. --- bot/exts/info/python_news.py | 57 ++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 6478eed3d1..53873d5d5c 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -1,3 +1,4 @@ +import asyncio import re import typing as t from datetime import UTC, datetime, timedelta @@ -15,8 +16,8 @@ from bot.log import get_logger from bot.utils.webhooks import send_webhook -PYTHON_NEWS_LOAD_MAX_ATTEMPTS = 3 -PYTHON_NEWS_INITIAL_BACKOFF_SECONDS = 1 +MAX_ATTEMPTS = constants.URLs.connect_max_retries +INITIAL_BACKOFF_SECONDS = 1 PEPS_RSS_URL = "https://peps.python.org/peps.rss" @@ -55,19 +56,45 @@ def _retryable_site_load_error(error: Exception) -> bool: async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" - with sentry_sdk.start_span(description="Fetch mailing lists from site"): - response = await self.bot.api_client.get("bot/mailing-lists") - - for mailing_list in response: - self.seen_items[mailing_list["name"]] = set(mailing_list["seen_items"]) - - with sentry_sdk.start_span(description="Update site with new mailing lists"): - for mailing_list in ("pep", *constants.PythonNews.mail_lists): - if mailing_list not in self.seen_items: - await self.bot.api_client.post("bot/mailing-lists", json={"name": mailing_list}) - self.seen_items[mailing_list] = set() - - self.fetch_new_media.start() + for attempt in range(1, MAX_ATTEMPTS + 1): + try: + with sentry_sdk.start_span(description="Fetch mailing lists from site"): + response = await self.bot.api_client.get("bot/mailing-lists") + + # Rebuild state on each successful fetch (avoid partial state across retries) + self.seen_items = {} + for mailing_list in response: + self.seen_items[mailing_list["name"]] = set(mailing_list["seen_items"]) + + with sentry_sdk.start_span(description="Update site with new mailing lists"): + for mailing_list in ("pep", *constants.PythonNews.mail_lists): + if mailing_list not in self.seen_items: + await self.bot.api_client.post("bot/mailing-lists", json={"name": mailing_list}) + self.seen_items[mailing_list] = set() + + self.fetch_new_media.start() + return + + except Exception as error: + if not self._retryable_site_load_error(error): + raise + + if attempt == MAX_ATTEMPTS: + log.exception( + "Failed to load PythonNews mailing lists after %d attempt(s).", + MAX_ATTEMPTS, + ) + raise + + backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) + log.warning( + "Failed to load PythonNews mailing lists (attempt %d/%d). Retrying in %d second(s). Error: %s", + attempt, + MAX_ATTEMPTS, + backoff_seconds, + error, + ) + await asyncio.sleep(backoff_seconds) async def cog_unload(self) -> None: """Stop news posting tasks on cog unload.""" From 015927e12e8c3f209639f4bd118b40921a73eef2 Mon Sep 17 00:00:00 2001 From: Carl Isaksson Date: Thu, 26 Feb 2026 18:06:39 +0100 Subject: [PATCH 54/85] feat: alert moderators after max attempts (#16) --- bot/exts/info/python_news.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 53873d5d5c..255d875d45 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -51,9 +51,31 @@ def __init__(self, bot: Bot): @staticmethod def _retryable_site_load_error(error: Exception) -> bool: if isinstance(error, ResponseCodeError): - return error.status == 429 or error.status >= 500 + return error.status in (408, 429) or error.status >= 500 return isinstance(error, (TimeoutError, OSError)) + async def _alert_mods_python_news_load_failure( + self, error: Exception, attempts: int + ) -> None: + """Alert moderators if PythonNews fails to load after all retries.""" + channel = self.bot.get_channel(constants.Channels.mod_alerts) + + if channel is None: + log.error("Could not find mod_alerts channel to send PythonNews failure alert.") + return + + status_info = "" + if isinstance(error, ResponseCodeError): + status_info = f" (status {error.status})" + + await channel.send( + ":warning: **PythonNews failed to load.**\n" + f"Attempts: {attempts}\n" + f"Error: `{error.__class__.__name__}{status_info}`\n\n" + "Python news posting will not be active until the bot is restarted " + "or the extension is reloaded." + ) + async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" for attempt in range(1, MAX_ATTEMPTS + 1): @@ -84,6 +106,7 @@ async def cog_load(self) -> None: "Failed to load PythonNews mailing lists after %d attempt(s).", MAX_ATTEMPTS, ) + await self._alert_mods_python_news_load_failure(error, attempt) raise backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) From 2159edb7f73e06da7229f7d6761f5bfe062a8c5d Mon Sep 17 00:00:00 2001 From: Carl Isaksson Date: Sat, 28 Feb 2026 21:22:49 +0100 Subject: [PATCH 55/85] test: mocked setup and pythonNews tests (#16) --- tests/bot/exts/info/test_python_news.py | 132 ++++++++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 tests/bot/exts/info/test_python_news.py diff --git a/tests/bot/exts/info/test_python_news.py b/tests/bot/exts/info/test_python_news.py new file mode 100644 index 0000000000..0b47c6d7b3 --- /dev/null +++ b/tests/bot/exts/info/test_python_news.py @@ -0,0 +1,132 @@ +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +from pydis_core.site_api import ResponseCodeError + +from bot.exts.info.python_news import PythonNews + + +class PythonNewsCogLoadTests(unittest.IsolatedAsyncioTestCase): + """Test startup behavior of the PythonNews cog (`cog_load`).""" + + def setUp(self) -> None: + """Set up a PythonNews cog with a mocked bot and stubbed startup dependencies.""" + self.bot = MagicMock() + self.bot.wait_until_guild_available = AsyncMock() + + self.bot.api_client = MagicMock() + self.bot.api_client.get = AsyncMock() + self.bot.api_client.post = AsyncMock() + + # Required by `fetch_new_media` later, but not used in these tests. + self.bot.http_session = MagicMock() + + self.cog = PythonNews(self.bot) + + # Stub out task-loop start, so it doesn't actually schedule anything. + self.start_patcher = patch.object(self.cog.fetch_new_media, "start") + self.mock_fetch_new_media_start = self.start_patcher.start() + self.addCleanup(self.start_patcher.stop) + + async def test_cog_load_retries_then_succeeds(self): + """`cog_load` should retry temporary failures and complete startup after a successful fetch.""" + # First two attempts fail with retryable errors, third succeeds. + self.bot.api_client.get.side_effect = [ + OSError("temporary outage"), + TimeoutError("temporary timeout"), + [ + {"name": "pep", "seen_items": ["1", "2"]}, + ], + ] + + # Ensure no missing mailing lists need creating in this test. + with patch("bot.exts.info.python_news.constants.PythonNews.mail_lists", new=()): + self.cog._alert_mods_python_news_load_failure = AsyncMock() + + with patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + await self.cog.cog_load() + + self.assertEqual(self.bot.api_client.get.await_count, 3) + self.bot.api_client.get.assert_awaited_with("bot/mailing-lists") + + # Sleep should have been awaited for the two failed attempts. + self.assertEqual(mock_sleep.await_count, 2) + + # No final alert on success. + self.cog._alert_mods_python_news_load_failure.assert_not_awaited() + + # Task should start after successful load. + self.mock_fetch_new_media_start.assert_called_once() + + # State should be populated. + self.assertIn("pep", self.cog.seen_items) + self.assertEqual(self.cog.seen_items["pep"], {"1", "2"}) + + # No posts should happen because no missing lists. + self.bot.api_client.post.assert_not_awaited() + + async def test_retries_max_times_fails_and_alerts(self): + """`cog_load` should alert and re-raise when all retry attempts fail.""" + self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") + self.cog._alert_mods_python_news_load_failure = AsyncMock() + + with ( + patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, + self.assertRaises(OSError), + ): + await self.cog.cog_load() + + # Should try exactly MAX_ATTEMPTS times. + from bot.exts.info import python_news as python_news_module + + self.assertEqual(self.bot.api_client.get.await_count, python_news_module.MAX_ATTEMPTS) + self.bot.api_client.get.assert_awaited_with("bot/mailing-lists") + + # Sleeps happen between attempts, so MAX_ATTEMPTS - 1 times. + self.assertEqual(mock_sleep.await_count, python_news_module.MAX_ATTEMPTS - 1) + + # Alert should be sent once at the end. + self.cog._alert_mods_python_news_load_failure.assert_awaited_once() + + error, attempts = self.cog._alert_mods_python_news_load_failure.await_args.args + self.assertIsInstance(error, OSError) + self.assertEqual(attempts, python_news_module.MAX_ATTEMPTS) + + # Task should never start if load fails. + self.mock_fetch_new_media_start.assert_not_called() + + def test_retryable_python_news_load_error(self): + """`_retryable_site_load_error` should classify temporary failures as retryable.""" + test_cases = ( + (ResponseCodeError(MagicMock(status=408)), True), + (ResponseCodeError(MagicMock(status=429)), True), + (ResponseCodeError(MagicMock(status=500)), True), + (ResponseCodeError(MagicMock(status=503)), True), + (ResponseCodeError(MagicMock(status=400)), False), + (ResponseCodeError(MagicMock(status=404)), False), + (TimeoutError("timeout"), True), + (OSError("os error"), True), + (AttributeError("attr"), False), + (ValueError("value"), False), + ) + + for error, expected_retryable in test_cases: + with self.subTest(error=error): + self.assertEqual(self.cog._retryable_site_load_error(error), expected_retryable) + + async def test_cog_load_does_not_retry_non_retryable_error(self): + """`cog_load` should not retry when the error is non-retryable.""" + # 404 should be considered non-retryable by your predicate. + self.bot.api_client.get.side_effect = ResponseCodeError(MagicMock(status=404)) + self.cog._alert_mods_python_news_load_failure = AsyncMock() + + with ( + patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, + self.assertRaises(ResponseCodeError), + ): + await self.cog.cog_load() + + self.assertEqual(self.bot.api_client.get.await_count, 1) + self.assertEqual(mock_sleep.await_count, 0) + self.cog._alert_mods_python_news_load_failure.assert_not_awaited() + self.mock_fetch_new_media_start.assert_not_called() From cd46116d33dfee39f58e1a3eeb6ce38e9d3ca015 Mon Sep 17 00:00:00 2001 From: Carl Isaksson Date: Sun, 1 Mar 2026 14:20:52 +0100 Subject: [PATCH 56/85] refactor: remove local mod alerting and update tests (#16) --- bot/exts/info/python_news.py | 23 --------------------- tests/bot/exts/info/test_python_news.py | 27 +++++++------------------ 2 files changed, 7 insertions(+), 43 deletions(-) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 255d875d45..2dd607a6ed 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -54,28 +54,6 @@ def _retryable_site_load_error(error: Exception) -> bool: return error.status in (408, 429) or error.status >= 500 return isinstance(error, (TimeoutError, OSError)) - async def _alert_mods_python_news_load_failure( - self, error: Exception, attempts: int - ) -> None: - """Alert moderators if PythonNews fails to load after all retries.""" - channel = self.bot.get_channel(constants.Channels.mod_alerts) - - if channel is None: - log.error("Could not find mod_alerts channel to send PythonNews failure alert.") - return - - status_info = "" - if isinstance(error, ResponseCodeError): - status_info = f" (status {error.status})" - - await channel.send( - ":warning: **PythonNews failed to load.**\n" - f"Attempts: {attempts}\n" - f"Error: `{error.__class__.__name__}{status_info}`\n\n" - "Python news posting will not be active until the bot is restarted " - "or the extension is reloaded." - ) - async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" for attempt in range(1, MAX_ATTEMPTS + 1): @@ -106,7 +84,6 @@ async def cog_load(self) -> None: "Failed to load PythonNews mailing lists after %d attempt(s).", MAX_ATTEMPTS, ) - await self._alert_mods_python_news_load_failure(error, attempt) raise backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) diff --git a/tests/bot/exts/info/test_python_news.py b/tests/bot/exts/info/test_python_news.py index 0b47c6d7b3..273e8d5dc8 100644 --- a/tests/bot/exts/info/test_python_news.py +++ b/tests/bot/exts/info/test_python_news.py @@ -40,11 +40,11 @@ async def test_cog_load_retries_then_succeeds(self): ] # Ensure no missing mailing lists need creating in this test. - with patch("bot.exts.info.python_news.constants.PythonNews.mail_lists", new=()): - self.cog._alert_mods_python_news_load_failure = AsyncMock() - - with patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: - await self.cog.cog_load() + with ( + patch("bot.exts.info.python_news.constants.PythonNews.mail_lists", new=()), + patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, + ): + await self.cog.cog_load() self.assertEqual(self.bot.api_client.get.await_count, 3) self.bot.api_client.get.assert_awaited_with("bot/mailing-lists") @@ -52,9 +52,6 @@ async def test_cog_load_retries_then_succeeds(self): # Sleep should have been awaited for the two failed attempts. self.assertEqual(mock_sleep.await_count, 2) - # No final alert on success. - self.cog._alert_mods_python_news_load_failure.assert_not_awaited() - # Task should start after successful load. self.mock_fetch_new_media_start.assert_called_once() @@ -65,10 +62,9 @@ async def test_cog_load_retries_then_succeeds(self): # No posts should happen because no missing lists. self.bot.api_client.post.assert_not_awaited() - async def test_retries_max_times_fails_and_alerts(self): - """`cog_load` should alert and re-raise when all retry attempts fail.""" + async def test_retries_max_times_fails_and_reraises(self): + """`cog_load` should re-raise when all retry attempts fail.""" self.bot.api_client.get.side_effect = OSError("Simulated site/API outage during cog_load") - self.cog._alert_mods_python_news_load_failure = AsyncMock() with ( patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, @@ -85,13 +81,6 @@ async def test_retries_max_times_fails_and_alerts(self): # Sleeps happen between attempts, so MAX_ATTEMPTS - 1 times. self.assertEqual(mock_sleep.await_count, python_news_module.MAX_ATTEMPTS - 1) - # Alert should be sent once at the end. - self.cog._alert_mods_python_news_load_failure.assert_awaited_once() - - error, attempts = self.cog._alert_mods_python_news_load_failure.await_args.args - self.assertIsInstance(error, OSError) - self.assertEqual(attempts, python_news_module.MAX_ATTEMPTS) - # Task should never start if load fails. self.mock_fetch_new_media_start.assert_not_called() @@ -118,7 +107,6 @@ async def test_cog_load_does_not_retry_non_retryable_error(self): """`cog_load` should not retry when the error is non-retryable.""" # 404 should be considered non-retryable by your predicate. self.bot.api_client.get.side_effect = ResponseCodeError(MagicMock(status=404)) - self.cog._alert_mods_python_news_load_failure = AsyncMock() with ( patch("bot.exts.info.python_news.asyncio.sleep", new_callable=AsyncMock) as mock_sleep, @@ -128,5 +116,4 @@ async def test_cog_load_does_not_retry_non_retryable_error(self): self.assertEqual(self.bot.api_client.get.await_count, 1) self.assertEqual(mock_sleep.await_count, 0) - self.cog._alert_mods_python_news_load_failure.assert_not_awaited() self.mock_fetch_new_media_start.assert_not_called() From 1afa433908d657c73f0b9202421f1d55c70338d8 Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Fri, 27 Feb 2026 16:52:14 +0100 Subject: [PATCH 57/85] test: add superstarify tests (#14) Add test cases for retrying cog loads and skeleton for new functions --- .../moderation/infraction/superstarify.py | 11 ++- .../infraction/test_superstarify_cog.py | 82 +++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 tests/bot/exts/moderation/infraction/test_superstarify_cog.py diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 006334755d..57b5733a39 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -10,6 +10,7 @@ from bot import constants from bot.bot import Bot +from bot.constants import URLs from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils @@ -18,6 +19,7 @@ from bot.utils import time from bot.utils.messages import format_user +MAX_RETRY_ATTEMPTS = URLs.connect_max_retries log = get_logger(__name__) NICKNAME_POLICY_URL = "https://pythondiscord.com/pages/rules/#nickname-policy" SUPERSTARIFY_DEFAULT_DURATION = "1h" @@ -238,7 +240,14 @@ async def cog_check(self, ctx: Context) -> bool: """Only allow moderators to invoke the commands in this cog.""" return await has_any_role(*constants.MODERATION_ROLES).predicate(ctx) - + async def _fetch_with_retries(self, + retries: int = MAX_RETRY_ATTEMPTS, + params: dict[str, str] | None = None) -> list[dict]: + return None + async def _alert_mods_if_loading_failed(self, error: Exception) -> None: + pass + async def _check_error_is_retriable(self, error: Exception) -> bool: + return False async def setup(bot: Bot) -> None: """Load the Superstarify cog.""" await bot.add_cog(Superstarify(bot)) diff --git a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py new file mode 100644 index 0000000000..847ce43ba4 --- /dev/null +++ b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py @@ -0,0 +1,82 @@ +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +from bot.exts.moderation.infraction.superstarify import Superstarify +from tests.helpers import MockBot + + +class TestSuperstarify(unittest.IsolatedAsyncioTestCase): + + async def asyncSetUp(self): + self.bot = MockBot() + + self.cog = Superstarify(self.bot) + + self.bot.api_client = MagicMock() + self.bot.api_client.get = AsyncMock() + + self.cog._alert_mods_if_loading_failed = AsyncMock() + self.cog._check_error_is_retriable = MagicMock(return_value=True) + + async def test_fetch_from_api_success(self): + """API succeeds on first attempt.""" + expected = [{"id": 1}] + self.bot.api_client.get.return_value = expected + + result = await self.cog._fetch_with_retries( + params={"user__id": "123"} + ) + self.assertEqual(result, expected) + + self.bot.api_client.get.assert_awaited_once_with( + "bot/infractions", + params={"user__id": "123"}, + ) + self.cog._alert_mods_if_loading_failed.assert_not_called() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_fetch_retries_then_succeeds(self, _): + self.bot.api_client.get.side_effect = [ + OSError("temporary failure"), + [{"id": 42}], + ] + + result = await self.cog._fetch_with_retries( + params={"user__id": "123"} + ) + + self.assertEqual(result, [{"id": 42}]) + self.assertEqual(self.bot.api_client.get.await_count, 2) + + self.cog._alert_mods_if_loading_failed.assert_not_called() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_fetch_fails_after_max_retries(self, _): + error = OSError("API down") + + self.bot.api_client.get.side_effect = error + + with self.assertRaises(OSError): + await self.cog._fetch_with_retries( + retries=3, + params={"user__id": "123"}, + ) + + self.assertEqual(self.bot.api_client.get.await_count, 3) + + self.cog._alert_mods_if_loading_failed.assert_awaited_once_with(error) + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_non_retriable_error_stops_immediately(self, _): + error = ValueError("bad request") + + self.bot.api_client.get.side_effect = error + self.cog._check_error_is_retriable.return_value = False + + with self.assertRaises(ValueError): + await self.cog._fetch_with_retries() + + # only one attempt + self.bot.api_client.get.assert_awaited_once() + + self.cog._alert_mods_if_loading_failed.assert_awaited_once() From cc59caf5933b2a1abaafd390fe4fb7af5d4bcbb9 Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Fri, 27 Feb 2026 17:05:54 +0100 Subject: [PATCH 58/85] feat: add logic to functions (#14) Implement skeleton functions with code for retrying fetch, alerting mods, and checking if retryable --- .../moderation/infraction/superstarify.py | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 57b5733a39..8139eaa37d 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -1,16 +1,19 @@ +import asyncio import json import random import textwrap from pathlib import Path +import discord from discord import Embed, Member from discord.ext.commands import Cog, Context, command, has_any_role from discord.utils import escape_markdown +from pydis_core.site_api import ResponseCodeError from pydis_core.utils.members import get_or_fetch_member from bot import constants from bot.bot import Bot -from bot.constants import URLs +from bot.constants import Icons, URLs from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils @@ -18,8 +21,10 @@ from bot.log import get_logger from bot.utils import time from bot.utils.messages import format_user +from bot.utils.modlog import send_log_message MAX_RETRY_ATTEMPTS = URLs.connect_max_retries +BACKOFF_INITIAL_DELAY = 5 # seconds log = get_logger(__name__) NICKNAME_POLICY_URL = "https://pythondiscord.com/pages/rules/#nickname-policy" SUPERSTARIFY_DEFAULT_DURATION = "1h" @@ -45,9 +50,7 @@ async def on_member_update(self, before: Member, after: Member) -> None: f"{after.display_name}. Checking if the user is in superstar-prison..." ) - active_superstarifies = await self.bot.api_client.get( - "bot/infractions", - params={ + active_superstarifies = await self._fetch_with_retries(params={ "active": "true", "type": "superstar", "user__id": str(before.id) @@ -86,9 +89,7 @@ async def on_member_update(self, before: Member, after: Member) -> None: @Cog.listener() async def on_member_join(self, member: Member) -> None: """Reapply active superstar infractions for returning members.""" - active_superstarifies = await self.bot.api_client.get( - "bot/infractions", - params={ + active_superstarifies = await self._fetch_with_retries(params={ "active": "true", "type": "superstar", "user__id": member.id @@ -243,11 +244,43 @@ async def cog_check(self, ctx: Context) -> bool: async def _fetch_with_retries(self, retries: int = MAX_RETRY_ATTEMPTS, params: dict[str, str] | None = None) -> list[dict]: + """Fetch infractions from the API with retries and exponential backoff.""" + for attempt in range(retries): + try: + return await self.bot.api_client.get("bot/infractions", params=params) + except Exception as e: + if attempt == retries - 1 or not self._check_error_is_retriable(e): + await self._alert_mods_if_loading_failed(e) + raise + await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) return None + async def _alert_mods_if_loading_failed(self, error: Exception) -> None: - pass + """Alert moderators that loading the superstarify cog failed after retries.""" + message = textwrap.dedent( + f""" + An error occurred while loading the Superstarify Cog, and it failed to initialize properly. + + Error details: + {error} + """ + ) + + await send_log_message( + self.bot, + title="Error: Failed to initialize the Superstarify Cog", + text=message, + ping_everyone=True, + icon_url=Icons.token_removed, + colour=discord.Color.red() + ) async def _check_error_is_retriable(self, error: Exception) -> bool: - return False + """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" + if isinstance(error, ResponseCodeError): + return error.status in (408, 429) or error.status >= 500 + + return isinstance(error, (TimeoutError, OSError)) + async def setup(bot: Bot) -> None: """Load the Superstarify cog.""" await bot.add_cog(Superstarify(bot)) From 62d115a0b28ddeb064cafb9bb605050e8367eb24 Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Fri, 27 Feb 2026 17:20:30 +0100 Subject: [PATCH 59/85] test: more unit tests for superstarify (#14) Add unit test for on_member_update and unit test to check _alert_mods_if_loading_failed is being called --- .../infraction/test_superstarify_cog.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py index 847ce43ba4..4e3004ce6b 100644 --- a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py +++ b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py @@ -80,3 +80,33 @@ async def test_non_retriable_error_stops_immediately(self, _): self.bot.api_client.get.assert_awaited_once() self.cog._alert_mods_if_loading_failed.assert_awaited_once() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_member_update_recovers_from_api_failure(self, _): + before = MagicMock(display_name="Old", id=123) + after = MagicMock(display_name="New", id=123) + after.edit = AsyncMock() + + self.bot.api_client.get.side_effect = [ + OSError(), + [{"id": 42}], + ] + + self.cog.get_nick = MagicMock(return_value="Taylor Swift") + + with patch( + "bot.exts.moderation.infraction._utils.notify_infraction", + new=AsyncMock(return_value=True), + ): + await self.cog.on_member_update(before, after) + + after.edit.assert_awaited_once() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_alert_triggered_after_total_failure(self, _): + self.bot.api_client.get.side_effect = OSError("down") + + with self.assertRaises(OSError): + await self.cog._fetch_with_retries(retries=3) + + self.cog._alert_mods_if_loading_failed.assert_awaited_once() From 1e733d54674d56199573892433c36e67e4f765d8 Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Sun, 1 Mar 2026 14:53:15 +0100 Subject: [PATCH 60/85] refactor: remove _alert_mods_if_loading_failed() (#14) Remove redundant code and corresponding parts of unit tests. --- .../moderation/infraction/superstarify.py | 24 +------------------ .../infraction/test_superstarify_cog.py | 7 ------ 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 8139eaa37d..180a49d304 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -4,7 +4,6 @@ import textwrap from pathlib import Path -import discord from discord import Embed, Member from discord.ext.commands import Cog, Context, command, has_any_role from discord.utils import escape_markdown @@ -13,7 +12,7 @@ from bot import constants from bot.bot import Bot -from bot.constants import Icons, URLs +from bot.constants import URLs from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils @@ -21,7 +20,6 @@ from bot.log import get_logger from bot.utils import time from bot.utils.messages import format_user -from bot.utils.modlog import send_log_message MAX_RETRY_ATTEMPTS = URLs.connect_max_retries BACKOFF_INITIAL_DELAY = 5 # seconds @@ -250,30 +248,10 @@ async def _fetch_with_retries(self, return await self.bot.api_client.get("bot/infractions", params=params) except Exception as e: if attempt == retries - 1 or not self._check_error_is_retriable(e): - await self._alert_mods_if_loading_failed(e) raise await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) return None - async def _alert_mods_if_loading_failed(self, error: Exception) -> None: - """Alert moderators that loading the superstarify cog failed after retries.""" - message = textwrap.dedent( - f""" - An error occurred while loading the Superstarify Cog, and it failed to initialize properly. - - Error details: - {error} - """ - ) - - await send_log_message( - self.bot, - title="Error: Failed to initialize the Superstarify Cog", - text=message, - ping_everyone=True, - icon_url=Icons.token_removed, - colour=discord.Color.red() - ) async def _check_error_is_retriable(self, error: Exception) -> bool: """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" if isinstance(error, ResponseCodeError): diff --git a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py index 4e3004ce6b..54473c7064 100644 --- a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py +++ b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py @@ -15,7 +15,6 @@ async def asyncSetUp(self): self.bot.api_client = MagicMock() self.bot.api_client.get = AsyncMock() - self.cog._alert_mods_if_loading_failed = AsyncMock() self.cog._check_error_is_retriable = MagicMock(return_value=True) async def test_fetch_from_api_success(self): @@ -32,7 +31,6 @@ async def test_fetch_from_api_success(self): "bot/infractions", params={"user__id": "123"}, ) - self.cog._alert_mods_if_loading_failed.assert_not_called() @patch("asyncio.sleep", new_callable=AsyncMock) async def test_fetch_retries_then_succeeds(self, _): @@ -48,7 +46,6 @@ async def test_fetch_retries_then_succeeds(self, _): self.assertEqual(result, [{"id": 42}]) self.assertEqual(self.bot.api_client.get.await_count, 2) - self.cog._alert_mods_if_loading_failed.assert_not_called() @patch("asyncio.sleep", new_callable=AsyncMock) async def test_fetch_fails_after_max_retries(self, _): @@ -64,7 +61,6 @@ async def test_fetch_fails_after_max_retries(self, _): self.assertEqual(self.bot.api_client.get.await_count, 3) - self.cog._alert_mods_if_loading_failed.assert_awaited_once_with(error) @patch("asyncio.sleep", new_callable=AsyncMock) async def test_non_retriable_error_stops_immediately(self, _): @@ -79,7 +75,6 @@ async def test_non_retriable_error_stops_immediately(self, _): # only one attempt self.bot.api_client.get.assert_awaited_once() - self.cog._alert_mods_if_loading_failed.assert_awaited_once() @patch("asyncio.sleep", new_callable=AsyncMock) async def test_member_update_recovers_from_api_failure(self, _): @@ -108,5 +103,3 @@ async def test_alert_triggered_after_total_failure(self, _): with self.assertRaises(OSError): await self.cog._fetch_with_retries(retries=3) - - self.cog._alert_mods_if_loading_failed.assert_awaited_once() From 3e875467b2045aaa9ec20f87ae1217f28ec7d395 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Mon, 2 Mar 2026 11:18:40 +0100 Subject: [PATCH 61/85] refactor: add back-off constant (#40) --- bot/constants.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bot/constants.py b/bot/constants.py index 674ae12fcc..c51d0d1cb3 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -462,6 +462,9 @@ class _URLs(_BaseURLs): connect_max_retries: int = 3 connect_cooldown: int = 5 + # Back-off in cog_load + connect_initial_backoff: int = 1 + site_logs_view: str = "https://pythondiscord.com/staff/bot/logs" From 6805c9aac0baeaa319166ae7b9de33267ab4e4d2 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Mon, 2 Mar 2026 11:19:33 +0100 Subject: [PATCH 62/85] refactor: update constants in reminders (#40) --- bot/exts/utils/reminders.py | 9 +++------ tests/bot/exts/utils/test_reminders.py | 4 +--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 86ab17c76e..e116dcf2ae 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -46,9 +46,6 @@ # the 2000-character message limit. MAXIMUM_REMINDER_MENTION_OPT_INS = 80 -# setup constants when loading -MAX_RETRY_ATTEMPTS = URLs.connect_max_retries -BACKOFF_INITIAL_DELAY = 5 # seconds Mentionable = discord.Member | discord.Role ReminderMention = UnambiguousUser | discord.Role @@ -230,7 +227,7 @@ async def cog_load(self) -> None: """Get all current reminders from the API and reschedule them.""" await self.bot.wait_until_guild_available() # retry fetching reminders with exponential backoff - for attempt in range(1, MAX_RETRY_ATTEMPTS + 1): + for attempt in range(1, URLs.connect_max_retries + 1): try: # response either throws, or is a list of reminders (possibly empty) response = await self.bot.api_client.get( @@ -243,10 +240,10 @@ async def cog_load(self) -> None: log.error(f"Failed to load reminders due to non-retryable error: {e}") raise log.warning(f"Attempt {attempt} - Failed to fetch reminders from the API: {e}") - if attempt == MAX_RETRY_ATTEMPTS: + if attempt == URLs.connect_max_retries: log.error("Max retry attempts reached. Failed to load reminders.") raise - await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) # Exponential backoff + await asyncio.sleep(URLs.connect_initial_backoff * (2 ** (attempt - 1))) # Exponential backoff now = datetime.now(UTC) for reminder in response: is_valid, *_ = self.ensure_valid_reminder(reminder) diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py index 67c8904fba..eb1d903876 100644 --- a/tests/bot/exts/utils/test_reminders.py +++ b/tests/bot/exts/utils/test_reminders.py @@ -7,8 +7,6 @@ from bot.exts.utils.reminders import Reminders from tests.helpers import MockBot -MAX_RETRY_ATTEMPTS = URLs.connect_max_retries - class RemindersCogLoadTests(unittest.IsolatedAsyncioTestCase): """ Tests startup behaviour of the Reminders cog. """ @@ -55,5 +53,5 @@ async def test_reminders_cog_load_fails_after_max_retries(self): await self.cog.cog_load() # Should have retried MAX_RETRY_ATTEMPTS - 1 times before failing - self.assertEqual(mock_sleep.await_count, MAX_RETRY_ATTEMPTS - 1) + self.assertEqual(mock_sleep.await_count, URLs.connect_max_retries - 1) self.bot.api_client.get.assert_called() From e64ccf4488dcdfdfe3dfc331e2ca7396bc00b9ae Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Mon, 2 Mar 2026 11:22:55 +0100 Subject: [PATCH 63/85] refactor: update constants in filtering (#40) --- bot/exts/filtering/filtering.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index d9cb2c504b..fd6146e405 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -25,7 +25,7 @@ import bot.exts.filtering._ui.filter as filters_ui from bot import constants from bot.bot import Bot -from bot.constants import BaseURLs, Channels, Guild, MODERATION_ROLES, Roles +from bot.constants import BaseURLs, Channels, Guild, MODERATION_ROLES, Roles, URLs from bot.exts.backend.branding._repository import HEADERS, PARAMS from bot.exts.filtering._filter_context import Event, FilterContext from bot.exts.filtering._filter_lists import FilterList, ListType, ListTypeConverter, filter_list_types @@ -65,8 +65,6 @@ HOURS_BETWEEN_NICKNAME_ALERTS = 1 OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=7) WEEKLY_REPORT_ISO_DAY = 3 # 1=Monday, 7=Sunday -FILTER_LOAD_MAX_ATTEMPTS = constants.URLs.connect_max_retries -INITIAL_BACKOFF_SECONDS = 1 async def _extract_text_file_content(att: discord.Attachment) -> str: @@ -111,26 +109,26 @@ async def cog_load(self) -> None: await self.bot.wait_until_guild_available() log.trace("Loading filtering information from the database.") - for attempt in range(1, FILTER_LOAD_MAX_ATTEMPTS + 1): + for attempt in range(1, URLs.connect_max_retries + 1): try: raw_filter_lists = await self.bot.api_client.get("bot/filter/filter_lists") break except Exception as error: is_retryable = self._retryable_filter_load_error(error) - is_last_attempt = attempt == FILTER_LOAD_MAX_ATTEMPTS + is_last_attempt = attempt == URLs.connect_max_retries if not is_retryable: raise if is_last_attempt: - log.exception("Failed to load filtering data after %d attempts.", FILTER_LOAD_MAX_ATTEMPTS) + log.exception("Failed to load filtering data after %d attempts.", URLs.connect_max_retries) raise - backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) + backoff_seconds = URLs.connect_initial_backoff * (2 ** (attempt - 1)) log.warning( "Failed to load filtering data (attempt %d/%d). Retrying in %d second(s): %s", attempt, - FILTER_LOAD_MAX_ATTEMPTS, + URLs.connect_max_retries, backoff_seconds, error ) From 1e27ddef6ec64f21a11cb75b069446f39e673b52 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Mon, 2 Mar 2026 11:26:35 +0100 Subject: [PATCH 64/85] refactor: update constants in python news (#40) --- bot/exts/info/python_news.py | 14 ++++++-------- tests/bot/exts/info/test_python_news.py | 6 +++--- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 2dd607a6ed..d57f28af22 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -13,12 +13,10 @@ from bot import constants from bot.bot import Bot +from bot.constants import URLs from bot.log import get_logger from bot.utils.webhooks import send_webhook -MAX_ATTEMPTS = constants.URLs.connect_max_retries -INITIAL_BACKOFF_SECONDS = 1 - PEPS_RSS_URL = "https://peps.python.org/peps.rss" RECENT_THREADS_TEMPLATE = "https://mail.python.org/archives/list/{name}@python.org/recent-threads" @@ -56,7 +54,7 @@ def _retryable_site_load_error(error: Exception) -> bool: async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" - for attempt in range(1, MAX_ATTEMPTS + 1): + for attempt in range(1, URLs.connect_max_retries + 1): try: with sentry_sdk.start_span(description="Fetch mailing lists from site"): response = await self.bot.api_client.get("bot/mailing-lists") @@ -79,18 +77,18 @@ async def cog_load(self) -> None: if not self._retryable_site_load_error(error): raise - if attempt == MAX_ATTEMPTS: + if attempt == URLs.connect_max_retries: log.exception( "Failed to load PythonNews mailing lists after %d attempt(s).", - MAX_ATTEMPTS, + URLs.connect_max_retries, ) raise - backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) + backoff_seconds = URLs.connect_initial_backoff * (2 ** (attempt - 1)) log.warning( "Failed to load PythonNews mailing lists (attempt %d/%d). Retrying in %d second(s). Error: %s", attempt, - MAX_ATTEMPTS, + URLs.connect_max_retries, backoff_seconds, error, ) diff --git a/tests/bot/exts/info/test_python_news.py b/tests/bot/exts/info/test_python_news.py index 273e8d5dc8..d626183e1a 100644 --- a/tests/bot/exts/info/test_python_news.py +++ b/tests/bot/exts/info/test_python_news.py @@ -3,6 +3,7 @@ from pydis_core.site_api import ResponseCodeError +from bot.constants import URLs from bot.exts.info.python_news import PythonNews @@ -73,13 +74,12 @@ async def test_retries_max_times_fails_and_reraises(self): await self.cog.cog_load() # Should try exactly MAX_ATTEMPTS times. - from bot.exts.info import python_news as python_news_module - self.assertEqual(self.bot.api_client.get.await_count, python_news_module.MAX_ATTEMPTS) + self.assertEqual(self.bot.api_client.get.await_count, URLs.connect_max_retries) self.bot.api_client.get.assert_awaited_with("bot/mailing-lists") # Sleeps happen between attempts, so MAX_ATTEMPTS - 1 times. - self.assertEqual(mock_sleep.await_count, python_news_module.MAX_ATTEMPTS - 1) + self.assertEqual(mock_sleep.await_count, URLs.connect_max_retries - 1) # Task should never start if load fails. self.mock_fetch_new_media_start.assert_not_called() From 7bfd50d5c6c6b354ca8c626feab82c5ec8720086 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Mon, 2 Mar 2026 11:29:04 +0100 Subject: [PATCH 65/85] refactor: update constants in superstarify (#40) --- bot/exts/moderation/infraction/superstarify.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 180a49d304..d21bd3ef7a 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -21,8 +21,6 @@ from bot.utils import time from bot.utils.messages import format_user -MAX_RETRY_ATTEMPTS = URLs.connect_max_retries -BACKOFF_INITIAL_DELAY = 5 # seconds log = get_logger(__name__) NICKNAME_POLICY_URL = "https://pythondiscord.com/pages/rules/#nickname-policy" SUPERSTARIFY_DEFAULT_DURATION = "1h" @@ -240,7 +238,7 @@ async def cog_check(self, ctx: Context) -> bool: return await has_any_role(*constants.MODERATION_ROLES).predicate(ctx) async def _fetch_with_retries(self, - retries: int = MAX_RETRY_ATTEMPTS, + retries: int = URLs.connect_max_retries, params: dict[str, str] | None = None) -> list[dict]: """Fetch infractions from the API with retries and exponential backoff.""" for attempt in range(retries): @@ -249,7 +247,7 @@ async def _fetch_with_retries(self, except Exception as e: if attempt == retries - 1 or not self._check_error_is_retriable(e): raise - await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) + await asyncio.sleep(URLs.connect_initial_backoff * (2 ** (attempt - 1))) return None async def _check_error_is_retriable(self, error: Exception) -> bool: From cc0f3a1643c716ce7249b270cf201a37ccf3f0b5 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Mon, 2 Mar 2026 20:20:23 +0100 Subject: [PATCH 66/85] fix: remove uncalled method (Closes #36) --- bot/exts/filtering/filtering.py | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index d9cb2c504b..1d2c7b4ce8 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -157,25 +157,6 @@ def _retryable_filter_load_error(error: Exception) -> bool: return isinstance(error, (TimeoutError, OSError)) - async def _alert_mods_filter_load_failure(self, error: Exception, attempts: int) -> None: - """Send an alert to mod-alerts when startup fails after all retry attempts.""" - mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) - if mod_alerts_channel is None: - log.error("Failed to send filtering startup failure alert: #mod-alerts channel is unavailable.") - return - - error_details = f"{error.__class__.__name__}: {error}" - if isinstance(error, ResponseCodeError): - error_details = f"HTTP {error.status} - {error_details}" - - try: - await mod_alerts_channel.send( - ":warning: Filtering failed to load filter lists during startup " - f"after {attempts} attempt(s). Error: `{error_details}`" - ) - except discord.HTTPException: - log.exception("Failed to send filtering startup failure alert to #mod-alerts.") - def subscribe(self, filter_list: FilterList, *events: Event) -> None: """ Subscribe a filter list to the given events. From 3700aee57a08a756fbf7303310d5aeeecaed2346 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Mon, 2 Mar 2026 20:28:24 +0100 Subject: [PATCH 67/85] refactor: rename variables (Closes #38) --- bot/utils/startup_reporting.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/utils/startup_reporting.py b/bot/utils/startup_reporting.py index b3ad78b67f..2ac6da3ec3 100644 --- a/bot/utils/startup_reporting.py +++ b/bot/utils/startup_reporting.py @@ -42,18 +42,18 @@ async def notify(self, bot: Bot, failures: Mapping[str, BaseException], channel_ ping_everyone=True, channel_id=channel_id ) - except Exception as e: - log.exception(f"Failed to send startup failure report: {e}") + except Exception as exception: + log.exception(f"Failed to send startup failure report: {exception}") def render(self, failures: Mapping[str, BaseException]) -> str: """Render a human-readable message from the given failures.""" - keys = sorted(failures.keys()) + failure_keys = sorted(failures.keys()) lines = [] lines.append("The following extension(s) failed to load:") - for k in keys: - e = failures[k] - lines.append(f"- **{k}** - `{type(e).__name__}: {e}`") + for failure_key in failure_keys: + exception = failures[failure_key] + lines.append(f"- **{failure_key}** - `{type(exception).__name__}: {exception}`") return textwrap.dedent(f""" Failed items: From 4d78a8130e0980d0e8fc262c76f3d38741a32368 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Mon, 2 Mar 2026 20:32:22 +0100 Subject: [PATCH 68/85] refactor: simplyfy merging of lines (Closes #39) --- bot/utils/startup_reporting.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/bot/utils/startup_reporting.py b/bot/utils/startup_reporting.py index b3ad78b67f..7f2edd1feb 100644 --- a/bot/utils/startup_reporting.py +++ b/bot/utils/startup_reporting.py @@ -1,4 +1,3 @@ -import textwrap from collections.abc import Mapping from dataclasses import dataclass from typing import TYPE_CHECKING @@ -55,7 +54,4 @@ def render(self, failures: Mapping[str, BaseException]) -> str: e = failures[k] lines.append(f"- **{k}** - `{type(e).__name__}: {e}`") - return textwrap.dedent(f""" - Failed items: - {chr(10).join(lines)} - """).strip() + return "\n".join(lines) From 8d4d0a07ee617f05cebcf85b568589a978ca0aa1 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Mon, 2 Mar 2026 21:04:48 +0100 Subject: [PATCH 69/85] refactor: remove explicit context helper function (Closes #45) --- bot/bot.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/bot/bot.py b/bot/bot.py index 3d235da920..c271f57e0e 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -1,6 +1,5 @@ import asyncio import contextlib -import contextvars import types from sys import exception @@ -19,10 +18,6 @@ log = get_logger("bot") -_current_extension: contextvars.ContextVar[str | None] = contextvars.ContextVar( - "current_extension", default=None -) - class StartupError(Exception): """Exception class for startup errors.""" @@ -98,7 +93,7 @@ async def add_cog(self, cog: commands.Cog) -> None: Override of `BotBase.add_cog` to capture and log any exceptions raised during cog loading, including the extension name if available. """ - extension = _current_extension.get() + extension = cog.__module__ try: await super().add_cog(cog) @@ -121,8 +116,6 @@ async def _load_extensions(self, module: types.ModuleType) -> None: self.all_extensions = walk_extensions(module) async def _load_one(extension: str) -> None: - token = _current_extension.set(extension) - try: await self.load_extension(extension) log.info(f"Extension successfully loaded: {extension}") @@ -132,9 +125,6 @@ async def _load_one(extension: str) -> None: log.exception(f"Failed to load extension: {extension}") raise - finally: - _current_extension.reset(token) - for extension in self.all_extensions: task = scheduling.create_task(_load_one(extension)) self._extension_load_tasks[extension] = task From 7a637c392d4e879935adab705010657f29471ef1 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Mon, 2 Mar 2026 21:11:41 +0100 Subject: [PATCH 70/85] refactor: remove dataclass label (Closes #47) --- bot/utils/startup_reporting.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/bot/utils/startup_reporting.py b/bot/utils/startup_reporting.py index b3ad78b67f..f3a1a3de0f 100644 --- a/bot/utils/startup_reporting.py +++ b/bot/utils/startup_reporting.py @@ -1,6 +1,5 @@ import textwrap from collections.abc import Mapping -from dataclasses import dataclass from typing import TYPE_CHECKING import discord @@ -13,7 +12,6 @@ if TYPE_CHECKING: from bot.bot import Bot -@dataclass(frozen=True) class StartupFailureReporter: """Formats and sends one aggregated startup failure alert to moderators.""" From 4e40408d2d4c9a387b523e26bf30139c7e333567 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Mon, 2 Mar 2026 21:12:30 +0100 Subject: [PATCH 71/85] refactor: updated the testcases accordingly #36 --- tests/bot/exts/filtering/test_filtering_cog.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index f9d8536f42..2beb7d8807 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -36,7 +36,6 @@ async def test_cog_load_retries_then_succeeds(self): TimeoutError("temporary timeout"), [], ] - self.cog._alert_mods_filter_load_failure = AsyncMock() with patch("bot.exts.filtering.filtering.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: await self.cog.cog_load() @@ -45,7 +44,6 @@ async def test_cog_load_retries_then_succeeds(self): self.assertEqual(self.bot.api_client.get.await_count, 3) self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") self.assertEqual(mock_sleep.await_count, 2) - self.cog._alert_mods_filter_load_failure.assert_not_awaited() self.cog._fetch_or_generate_filtering_webhook.assert_awaited_once() self.cog.collect_loaded_types.assert_called_once_with(None) self.cog.schedule_offending_messages_deletion.assert_awaited_once() From 7e20fee4e95b67db95090d8e2b588043ac5a0de1 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Mon, 2 Mar 2026 11:18:40 +0100 Subject: [PATCH 72/85] refactor: add back-off constant (#40) --- bot/constants.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bot/constants.py b/bot/constants.py index 674ae12fcc..c51d0d1cb3 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -462,6 +462,9 @@ class _URLs(_BaseURLs): connect_max_retries: int = 3 connect_cooldown: int = 5 + # Back-off in cog_load + connect_initial_backoff: int = 1 + site_logs_view: str = "https://pythondiscord.com/staff/bot/logs" From 4b196cc9fb2e50a34d2bc9aa5bf20e95f29f1b3e Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Mon, 2 Mar 2026 11:19:33 +0100 Subject: [PATCH 73/85] refactor: update constants in reminders (#40) --- bot/exts/utils/reminders.py | 9 +++------ tests/bot/exts/utils/test_reminders.py | 4 +--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 86ab17c76e..e116dcf2ae 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -46,9 +46,6 @@ # the 2000-character message limit. MAXIMUM_REMINDER_MENTION_OPT_INS = 80 -# setup constants when loading -MAX_RETRY_ATTEMPTS = URLs.connect_max_retries -BACKOFF_INITIAL_DELAY = 5 # seconds Mentionable = discord.Member | discord.Role ReminderMention = UnambiguousUser | discord.Role @@ -230,7 +227,7 @@ async def cog_load(self) -> None: """Get all current reminders from the API and reschedule them.""" await self.bot.wait_until_guild_available() # retry fetching reminders with exponential backoff - for attempt in range(1, MAX_RETRY_ATTEMPTS + 1): + for attempt in range(1, URLs.connect_max_retries + 1): try: # response either throws, or is a list of reminders (possibly empty) response = await self.bot.api_client.get( @@ -243,10 +240,10 @@ async def cog_load(self) -> None: log.error(f"Failed to load reminders due to non-retryable error: {e}") raise log.warning(f"Attempt {attempt} - Failed to fetch reminders from the API: {e}") - if attempt == MAX_RETRY_ATTEMPTS: + if attempt == URLs.connect_max_retries: log.error("Max retry attempts reached. Failed to load reminders.") raise - await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) # Exponential backoff + await asyncio.sleep(URLs.connect_initial_backoff * (2 ** (attempt - 1))) # Exponential backoff now = datetime.now(UTC) for reminder in response: is_valid, *_ = self.ensure_valid_reminder(reminder) diff --git a/tests/bot/exts/utils/test_reminders.py b/tests/bot/exts/utils/test_reminders.py index 67c8904fba..eb1d903876 100644 --- a/tests/bot/exts/utils/test_reminders.py +++ b/tests/bot/exts/utils/test_reminders.py @@ -7,8 +7,6 @@ from bot.exts.utils.reminders import Reminders from tests.helpers import MockBot -MAX_RETRY_ATTEMPTS = URLs.connect_max_retries - class RemindersCogLoadTests(unittest.IsolatedAsyncioTestCase): """ Tests startup behaviour of the Reminders cog. """ @@ -55,5 +53,5 @@ async def test_reminders_cog_load_fails_after_max_retries(self): await self.cog.cog_load() # Should have retried MAX_RETRY_ATTEMPTS - 1 times before failing - self.assertEqual(mock_sleep.await_count, MAX_RETRY_ATTEMPTS - 1) + self.assertEqual(mock_sleep.await_count, URLs.connect_max_retries - 1) self.bot.api_client.get.assert_called() From 7d0f75d0c4ec1d6edc39ac711a3b966f3cadb7db Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Mon, 2 Mar 2026 11:22:55 +0100 Subject: [PATCH 74/85] refactor: update constants in filtering (#40) --- bot/exts/filtering/filtering.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index d9cb2c504b..fd6146e405 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -25,7 +25,7 @@ import bot.exts.filtering._ui.filter as filters_ui from bot import constants from bot.bot import Bot -from bot.constants import BaseURLs, Channels, Guild, MODERATION_ROLES, Roles +from bot.constants import BaseURLs, Channels, Guild, MODERATION_ROLES, Roles, URLs from bot.exts.backend.branding._repository import HEADERS, PARAMS from bot.exts.filtering._filter_context import Event, FilterContext from bot.exts.filtering._filter_lists import FilterList, ListType, ListTypeConverter, filter_list_types @@ -65,8 +65,6 @@ HOURS_BETWEEN_NICKNAME_ALERTS = 1 OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=7) WEEKLY_REPORT_ISO_DAY = 3 # 1=Monday, 7=Sunday -FILTER_LOAD_MAX_ATTEMPTS = constants.URLs.connect_max_retries -INITIAL_BACKOFF_SECONDS = 1 async def _extract_text_file_content(att: discord.Attachment) -> str: @@ -111,26 +109,26 @@ async def cog_load(self) -> None: await self.bot.wait_until_guild_available() log.trace("Loading filtering information from the database.") - for attempt in range(1, FILTER_LOAD_MAX_ATTEMPTS + 1): + for attempt in range(1, URLs.connect_max_retries + 1): try: raw_filter_lists = await self.bot.api_client.get("bot/filter/filter_lists") break except Exception as error: is_retryable = self._retryable_filter_load_error(error) - is_last_attempt = attempt == FILTER_LOAD_MAX_ATTEMPTS + is_last_attempt = attempt == URLs.connect_max_retries if not is_retryable: raise if is_last_attempt: - log.exception("Failed to load filtering data after %d attempts.", FILTER_LOAD_MAX_ATTEMPTS) + log.exception("Failed to load filtering data after %d attempts.", URLs.connect_max_retries) raise - backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) + backoff_seconds = URLs.connect_initial_backoff * (2 ** (attempt - 1)) log.warning( "Failed to load filtering data (attempt %d/%d). Retrying in %d second(s): %s", attempt, - FILTER_LOAD_MAX_ATTEMPTS, + URLs.connect_max_retries, backoff_seconds, error ) From 1704c9f16673e93e08b519ce3e8c432a732f20fc Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Mon, 2 Mar 2026 11:26:35 +0100 Subject: [PATCH 75/85] refactor: update constants in python news (#40) --- bot/exts/info/python_news.py | 14 ++++++-------- tests/bot/exts/info/test_python_news.py | 6 +++--- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 2dd607a6ed..d57f28af22 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -13,12 +13,10 @@ from bot import constants from bot.bot import Bot +from bot.constants import URLs from bot.log import get_logger from bot.utils.webhooks import send_webhook -MAX_ATTEMPTS = constants.URLs.connect_max_retries -INITIAL_BACKOFF_SECONDS = 1 - PEPS_RSS_URL = "https://peps.python.org/peps.rss" RECENT_THREADS_TEMPLATE = "https://mail.python.org/archives/list/{name}@python.org/recent-threads" @@ -56,7 +54,7 @@ def _retryable_site_load_error(error: Exception) -> bool: async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" - for attempt in range(1, MAX_ATTEMPTS + 1): + for attempt in range(1, URLs.connect_max_retries + 1): try: with sentry_sdk.start_span(description="Fetch mailing lists from site"): response = await self.bot.api_client.get("bot/mailing-lists") @@ -79,18 +77,18 @@ async def cog_load(self) -> None: if not self._retryable_site_load_error(error): raise - if attempt == MAX_ATTEMPTS: + if attempt == URLs.connect_max_retries: log.exception( "Failed to load PythonNews mailing lists after %d attempt(s).", - MAX_ATTEMPTS, + URLs.connect_max_retries, ) raise - backoff_seconds = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) + backoff_seconds = URLs.connect_initial_backoff * (2 ** (attempt - 1)) log.warning( "Failed to load PythonNews mailing lists (attempt %d/%d). Retrying in %d second(s). Error: %s", attempt, - MAX_ATTEMPTS, + URLs.connect_max_retries, backoff_seconds, error, ) diff --git a/tests/bot/exts/info/test_python_news.py b/tests/bot/exts/info/test_python_news.py index 273e8d5dc8..d626183e1a 100644 --- a/tests/bot/exts/info/test_python_news.py +++ b/tests/bot/exts/info/test_python_news.py @@ -3,6 +3,7 @@ from pydis_core.site_api import ResponseCodeError +from bot.constants import URLs from bot.exts.info.python_news import PythonNews @@ -73,13 +74,12 @@ async def test_retries_max_times_fails_and_reraises(self): await self.cog.cog_load() # Should try exactly MAX_ATTEMPTS times. - from bot.exts.info import python_news as python_news_module - self.assertEqual(self.bot.api_client.get.await_count, python_news_module.MAX_ATTEMPTS) + self.assertEqual(self.bot.api_client.get.await_count, URLs.connect_max_retries) self.bot.api_client.get.assert_awaited_with("bot/mailing-lists") # Sleeps happen between attempts, so MAX_ATTEMPTS - 1 times. - self.assertEqual(mock_sleep.await_count, python_news_module.MAX_ATTEMPTS - 1) + self.assertEqual(mock_sleep.await_count, URLs.connect_max_retries - 1) # Task should never start if load fails. self.mock_fetch_new_media_start.assert_not_called() From 8428caccda19c9adebb2fdd25eed79cb37fe40c8 Mon Sep 17 00:00:00 2001 From: kahoujo1 Date: Mon, 2 Mar 2026 11:29:04 +0100 Subject: [PATCH 76/85] refactor: update constants in superstarify (#40) --- bot/exts/moderation/infraction/superstarify.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 180a49d304..d21bd3ef7a 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -21,8 +21,6 @@ from bot.utils import time from bot.utils.messages import format_user -MAX_RETRY_ATTEMPTS = URLs.connect_max_retries -BACKOFF_INITIAL_DELAY = 5 # seconds log = get_logger(__name__) NICKNAME_POLICY_URL = "https://pythondiscord.com/pages/rules/#nickname-policy" SUPERSTARIFY_DEFAULT_DURATION = "1h" @@ -240,7 +238,7 @@ async def cog_check(self, ctx: Context) -> bool: return await has_any_role(*constants.MODERATION_ROLES).predicate(ctx) async def _fetch_with_retries(self, - retries: int = MAX_RETRY_ATTEMPTS, + retries: int = URLs.connect_max_retries, params: dict[str, str] | None = None) -> list[dict]: """Fetch infractions from the API with retries and exponential backoff.""" for attempt in range(retries): @@ -249,7 +247,7 @@ async def _fetch_with_retries(self, except Exception as e: if attempt == retries - 1 or not self._check_error_is_retriable(e): raise - await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) + await asyncio.sleep(URLs.connect_initial_backoff * (2 ** (attempt - 1))) return None async def _check_error_is_retriable(self, error: Exception) -> bool: From 0d14e70a052148713eeadbff0a589dea6ef28e3b Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Mon, 2 Mar 2026 20:20:23 +0100 Subject: [PATCH 77/85] fix: remove uncalled method (Closes #36) --- bot/exts/filtering/filtering.py | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index fd6146e405..af81be2fc1 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -155,25 +155,6 @@ def _retryable_filter_load_error(error: Exception) -> bool: return isinstance(error, (TimeoutError, OSError)) - async def _alert_mods_filter_load_failure(self, error: Exception, attempts: int) -> None: - """Send an alert to mod-alerts when startup fails after all retry attempts.""" - mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) - if mod_alerts_channel is None: - log.error("Failed to send filtering startup failure alert: #mod-alerts channel is unavailable.") - return - - error_details = f"{error.__class__.__name__}: {error}" - if isinstance(error, ResponseCodeError): - error_details = f"HTTP {error.status} - {error_details}" - - try: - await mod_alerts_channel.send( - ":warning: Filtering failed to load filter lists during startup " - f"after {attempts} attempt(s). Error: `{error_details}`" - ) - except discord.HTTPException: - log.exception("Failed to send filtering startup failure alert to #mod-alerts.") - def subscribe(self, filter_list: FilterList, *events: Event) -> None: """ Subscribe a filter list to the given events. From 02e89ed0c3c32362fb3532765a49d42efd8332d1 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Mon, 2 Mar 2026 21:12:30 +0100 Subject: [PATCH 78/85] refactor: updated the testcases accordingly #36 --- tests/bot/exts/filtering/test_filtering_cog.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index f9d8536f42..2beb7d8807 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -36,7 +36,6 @@ async def test_cog_load_retries_then_succeeds(self): TimeoutError("temporary timeout"), [], ] - self.cog._alert_mods_filter_load_failure = AsyncMock() with patch("bot.exts.filtering.filtering.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: await self.cog.cog_load() @@ -45,7 +44,6 @@ async def test_cog_load_retries_then_succeeds(self): self.assertEqual(self.bot.api_client.get.await_count, 3) self.bot.api_client.get.assert_awaited_with("bot/filter/filter_lists") self.assertEqual(mock_sleep.await_count, 2) - self.cog._alert_mods_filter_load_failure.assert_not_awaited() self.cog._fetch_or_generate_filtering_webhook.assert_awaited_once() self.cog.collect_loaded_types.assert_called_once_with(None) self.cog.schedule_offending_messages_deletion.assert_awaited_once() From 955682dbd8933c2775f0dac6a0cd6964c90f9b5f Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Mon, 2 Mar 2026 20:28:24 +0100 Subject: [PATCH 79/85] refactor: rename variables (Closes #38) --- bot/utils/startup_reporting.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/utils/startup_reporting.py b/bot/utils/startup_reporting.py index b3ad78b67f..2ac6da3ec3 100644 --- a/bot/utils/startup_reporting.py +++ b/bot/utils/startup_reporting.py @@ -42,18 +42,18 @@ async def notify(self, bot: Bot, failures: Mapping[str, BaseException], channel_ ping_everyone=True, channel_id=channel_id ) - except Exception as e: - log.exception(f"Failed to send startup failure report: {e}") + except Exception as exception: + log.exception(f"Failed to send startup failure report: {exception}") def render(self, failures: Mapping[str, BaseException]) -> str: """Render a human-readable message from the given failures.""" - keys = sorted(failures.keys()) + failure_keys = sorted(failures.keys()) lines = [] lines.append("The following extension(s) failed to load:") - for k in keys: - e = failures[k] - lines.append(f"- **{k}** - `{type(e).__name__}: {e}`") + for failure_key in failure_keys: + exception = failures[failure_key] + lines.append(f"- **{failure_key}** - `{type(exception).__name__}: {exception}`") return textwrap.dedent(f""" Failed items: From 831058899cf21fd966c0962be76edaf295eefc98 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Mon, 2 Mar 2026 20:32:22 +0100 Subject: [PATCH 80/85] refactor: simplyfy merging of lines (Closes #39) --- bot/utils/startup_reporting.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/bot/utils/startup_reporting.py b/bot/utils/startup_reporting.py index 2ac6da3ec3..0ec1c051c4 100644 --- a/bot/utils/startup_reporting.py +++ b/bot/utils/startup_reporting.py @@ -1,4 +1,3 @@ -import textwrap from collections.abc import Mapping from dataclasses import dataclass from typing import TYPE_CHECKING @@ -55,7 +54,4 @@ def render(self, failures: Mapping[str, BaseException]) -> str: exception = failures[failure_key] lines.append(f"- **{failure_key}** - `{type(exception).__name__}: {exception}`") - return textwrap.dedent(f""" - Failed items: - {chr(10).join(lines)} - """).strip() + return "\n".join(lines) From 1524affb1e84821bd8c6ff8b324820c3c419b4b1 Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Mon, 2 Mar 2026 21:04:48 +0100 Subject: [PATCH 81/85] refactor: remove explicit context helper function (Closes #45) --- bot/bot.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/bot/bot.py b/bot/bot.py index 3d235da920..c271f57e0e 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -1,6 +1,5 @@ import asyncio import contextlib -import contextvars import types from sys import exception @@ -19,10 +18,6 @@ log = get_logger("bot") -_current_extension: contextvars.ContextVar[str | None] = contextvars.ContextVar( - "current_extension", default=None -) - class StartupError(Exception): """Exception class for startup errors.""" @@ -98,7 +93,7 @@ async def add_cog(self, cog: commands.Cog) -> None: Override of `BotBase.add_cog` to capture and log any exceptions raised during cog loading, including the extension name if available. """ - extension = _current_extension.get() + extension = cog.__module__ try: await super().add_cog(cog) @@ -121,8 +116,6 @@ async def _load_extensions(self, module: types.ModuleType) -> None: self.all_extensions = walk_extensions(module) async def _load_one(extension: str) -> None: - token = _current_extension.set(extension) - try: await self.load_extension(extension) log.info(f"Extension successfully loaded: {extension}") @@ -132,9 +125,6 @@ async def _load_one(extension: str) -> None: log.exception(f"Failed to load extension: {extension}") raise - finally: - _current_extension.reset(token) - for extension in self.all_extensions: task = scheduling.create_task(_load_one(extension)) self._extension_load_tasks[extension] = task From b5fc5b21138adaf98f23cbfec3d3b472c8e5d6bb Mon Sep 17 00:00:00 2001 From: Alexander Runebou Date: Mon, 2 Mar 2026 21:11:41 +0100 Subject: [PATCH 82/85] refactor: remove dataclass label (Closes #47) --- bot/utils/startup_reporting.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/bot/utils/startup_reporting.py b/bot/utils/startup_reporting.py index 0ec1c051c4..f7713d1ca4 100644 --- a/bot/utils/startup_reporting.py +++ b/bot/utils/startup_reporting.py @@ -1,5 +1,4 @@ from collections.abc import Mapping -from dataclasses import dataclass from typing import TYPE_CHECKING import discord @@ -12,7 +11,6 @@ if TYPE_CHECKING: from bot.bot import Bot -@dataclass(frozen=True) class StartupFailureReporter: """Formats and sends one aggregated startup failure alert to moderators.""" From 4d73aadfa04db456d855511e4d49d5694ad58f9a Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Mon, 2 Mar 2026 23:19:41 +0100 Subject: [PATCH 83/85] refactor: Move the duplicated retry classifier out of the cogs into a shared utility #50 --- bot/exts/filtering/filtering.py | 11 ++----- bot/exts/info/python_news.py | 9 ++---- bot/utils/retry.py | 9 ++++++ .../bot/exts/filtering/test_filtering_cog.py | 21 -------------- tests/bot/exts/info/test_python_news.py | 19 ------------ tests/bot/utils/test_retry.py | 29 +++++++++++++++++++ 6 files changed, 42 insertions(+), 56 deletions(-) create mode 100644 bot/utils/retry.py create mode 100644 tests/bot/utils/test_retry.py diff --git a/bot/exts/filtering/filtering.py b/bot/exts/filtering/filtering.py index af81be2fc1..719e0ad796 100644 --- a/bot/exts/filtering/filtering.py +++ b/bot/exts/filtering/filtering.py @@ -56,6 +56,7 @@ from bot.utils.channel import is_mod_channel from bot.utils.lock import lock_arg from bot.utils.message_cache import MessageCache +from bot.utils.retry import is_retryable_api_error log = get_logger(__name__) @@ -114,7 +115,7 @@ async def cog_load(self) -> None: raw_filter_lists = await self.bot.api_client.get("bot/filter/filter_lists") break except Exception as error: - is_retryable = self._retryable_filter_load_error(error) + is_retryable = is_retryable_api_error(error) is_last_attempt = attempt == URLs.connect_max_retries if not is_retryable: @@ -147,14 +148,6 @@ async def cog_load(self) -> None: await self.schedule_offending_messages_deletion() self.weekly_auto_infraction_report_task.start() - @staticmethod - def _retryable_filter_load_error(error: Exception) -> bool: - """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" - if isinstance(error, ResponseCodeError): - return error.status in (408, 429) or error.status >= 500 - - return isinstance(error, (TimeoutError, OSError)) - def subscribe(self, filter_list: FilterList, *events: Event) -> None: """ Subscribe a filter list to the given events. diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index d57f28af22..437e44cd38 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -15,6 +15,7 @@ from bot.bot import Bot from bot.constants import URLs from bot.log import get_logger +from bot.utils.retry import is_retryable_api_error from bot.utils.webhooks import send_webhook PEPS_RSS_URL = "https://peps.python.org/peps.rss" @@ -46,12 +47,6 @@ def __init__(self, bot: Bot): self.webhook: discord.Webhook | None = None self.seen_items: dict[str, set[str]] = {} - @staticmethod - def _retryable_site_load_error(error: Exception) -> bool: - if isinstance(error, ResponseCodeError): - return error.status in (408, 429) or error.status >= 500 - return isinstance(error, (TimeoutError, OSError)) - async def cog_load(self) -> None: """Load all existing seen items from db and create any missing mailing lists.""" for attempt in range(1, URLs.connect_max_retries + 1): @@ -74,7 +69,7 @@ async def cog_load(self) -> None: return except Exception as error: - if not self._retryable_site_load_error(error): + if not is_retryable_api_error(error): raise if attempt == URLs.connect_max_retries: diff --git a/bot/utils/retry.py b/bot/utils/retry.py new file mode 100644 index 0000000000..342897f381 --- /dev/null +++ b/bot/utils/retry.py @@ -0,0 +1,9 @@ +from pydis_core.site_api import ResponseCodeError + + +def is_retryable_api_error(error: Exception) -> bool: + """Return whether an API error is temporary and worth retrying.""" + if isinstance(error, ResponseCodeError): + return error.status in (408, 429) or error.status >= 500 + + return isinstance(error, (TimeoutError, OSError)) diff --git a/tests/bot/exts/filtering/test_filtering_cog.py b/tests/bot/exts/filtering/test_filtering_cog.py index 2beb7d8807..736c4bf1aa 100644 --- a/tests/bot/exts/filtering/test_filtering_cog.py +++ b/tests/bot/exts/filtering/test_filtering_cog.py @@ -1,8 +1,6 @@ import unittest from unittest.mock import AsyncMock, MagicMock, patch -from pydis_core.site_api import ResponseCodeError - from bot.exts.filtering.filtering import Filtering @@ -77,22 +75,3 @@ async def test_retries_three_times_fails_and_reraises(self): self.cog._fetch_or_generate_filtering_webhook.assert_not_awaited() self.cog.schedule_offending_messages_deletion.assert_not_awaited() self.mock_weekly_task_start.assert_not_called() - - def test_retryable_filter_load_error(self): - """`_retryable_filter_load_error` should classify temporary failures as retryable.""" - test_cases = ( - (ResponseCodeError(MagicMock(status=408)), True), - (ResponseCodeError(MagicMock(status=429)), True), - (ResponseCodeError(MagicMock(status=500)), True), - (ResponseCodeError(MagicMock(status=503)), True), - (ResponseCodeError(MagicMock(status=400)), False), - (ResponseCodeError(MagicMock(status=404)), False), - (TimeoutError("timeout"), True), - (OSError("os error"), True), - (AttributeError("attr"), False), - (ValueError("value"), False), - ) - - for error, expected_retryable in test_cases: - with self.subTest(error=error): - self.assertEqual(self.cog._retryable_filter_load_error(error), expected_retryable) diff --git a/tests/bot/exts/info/test_python_news.py b/tests/bot/exts/info/test_python_news.py index d626183e1a..75c9e386aa 100644 --- a/tests/bot/exts/info/test_python_news.py +++ b/tests/bot/exts/info/test_python_news.py @@ -84,25 +84,6 @@ async def test_retries_max_times_fails_and_reraises(self): # Task should never start if load fails. self.mock_fetch_new_media_start.assert_not_called() - def test_retryable_python_news_load_error(self): - """`_retryable_site_load_error` should classify temporary failures as retryable.""" - test_cases = ( - (ResponseCodeError(MagicMock(status=408)), True), - (ResponseCodeError(MagicMock(status=429)), True), - (ResponseCodeError(MagicMock(status=500)), True), - (ResponseCodeError(MagicMock(status=503)), True), - (ResponseCodeError(MagicMock(status=400)), False), - (ResponseCodeError(MagicMock(status=404)), False), - (TimeoutError("timeout"), True), - (OSError("os error"), True), - (AttributeError("attr"), False), - (ValueError("value"), False), - ) - - for error, expected_retryable in test_cases: - with self.subTest(error=error): - self.assertEqual(self.cog._retryable_site_load_error(error), expected_retryable) - async def test_cog_load_does_not_retry_non_retryable_error(self): """`cog_load` should not retry when the error is non-retryable.""" # 404 should be considered non-retryable by your predicate. diff --git a/tests/bot/utils/test_retry.py b/tests/bot/utils/test_retry.py new file mode 100644 index 0000000000..8ce6f6db0c --- /dev/null +++ b/tests/bot/utils/test_retry.py @@ -0,0 +1,29 @@ +import unittest +from unittest.mock import MagicMock + +from pydis_core.site_api import ResponseCodeError + +from bot.utils.retry import is_retryable_api_error + + +class RetryTests(unittest.TestCase): + """Tests for retry classification helpers.""" + + def test_is_retryable_api_error(self): + """`is_retryable_api_error` should classify temporary failures as retryable.""" + test_cases = ( + (ResponseCodeError(MagicMock(status=408)), True), + (ResponseCodeError(MagicMock(status=429)), True), + (ResponseCodeError(MagicMock(status=500)), True), + (ResponseCodeError(MagicMock(status=503)), True), + (ResponseCodeError(MagicMock(status=400)), False), + (ResponseCodeError(MagicMock(status=404)), False), + (TimeoutError("timeout"), True), + (OSError("os error"), True), + (AttributeError("attr"), False), + (ValueError("value"), False), + ) + + for error, expected_retryable in test_cases: + with self.subTest(error=error): + self.assertEqual(is_retryable_api_error(error), expected_retryable) From 695206e9fd13405b0dd740ca6179764c730cb356 Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Mon, 2 Mar 2026 23:57:12 +0100 Subject: [PATCH 84/85] refactor: Updated superstarify.py to use the shared retry helper from retry.py #53 --- bot/exts/moderation/infraction/superstarify.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index d21bd3ef7a..5dacf1bb72 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -7,7 +7,6 @@ from discord import Embed, Member from discord.ext.commands import Cog, Context, command, has_any_role from discord.utils import escape_markdown -from pydis_core.site_api import ResponseCodeError from pydis_core.utils.members import get_or_fetch_member from bot import constants @@ -20,6 +19,7 @@ from bot.log import get_logger from bot.utils import time from bot.utils.messages import format_user +from bot.utils.retry import is_retryable_api_error log = get_logger(__name__) NICKNAME_POLICY_URL = "https://pythondiscord.com/pages/rules/#nickname-policy" @@ -245,18 +245,11 @@ async def _fetch_with_retries(self, try: return await self.bot.api_client.get("bot/infractions", params=params) except Exception as e: - if attempt == retries - 1 or not self._check_error_is_retriable(e): + if attempt == retries - 1 or not is_retryable_api_error(e): raise await asyncio.sleep(URLs.connect_initial_backoff * (2 ** (attempt - 1))) return None - async def _check_error_is_retriable(self, error: Exception) -> bool: - """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" - if isinstance(error, ResponseCodeError): - return error.status in (408, 429) or error.status >= 500 - - return isinstance(error, (TimeoutError, OSError)) - async def setup(bot: Bot) -> None: """Load the Superstarify cog.""" await bot.add_cog(Superstarify(bot)) From 791bb8ec3b2c260c961a00960d28b91a4e8436fe Mon Sep 17 00:00:00 2001 From: Apeel Subedi Date: Tue, 3 Mar 2026 00:26:36 +0100 Subject: [PATCH 85/85] refactor: updated _fetch_with_retries to use logic from filterign #55 --- bot/exts/moderation/infraction/superstarify.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 5dacf1bb72..01481d1f68 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -241,15 +241,19 @@ async def _fetch_with_retries(self, retries: int = URLs.connect_max_retries, params: dict[str, str] | None = None) -> list[dict]: """Fetch infractions from the API with retries and exponential backoff.""" - for attempt in range(retries): + if retries < 1: + raise ValueError("retries must be at least 1") + + for attempt in range(1, retries + 1): try: return await self.bot.api_client.get("bot/infractions", params=params) except Exception as e: - if attempt == retries - 1 or not is_retryable_api_error(e): + if attempt == retries or not is_retryable_api_error(e): raise await asyncio.sleep(URLs.connect_initial_backoff * (2 ** (attempt - 1))) return None + async def setup(bot: Bot) -> None: """Load the Superstarify cog.""" await bot.add_cog(Superstarify(bot))