diff --git a/Docs/Tasks.md b/Docs/Tasks.md index fc42890..2063889 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,45 +1,3 @@ -### Issue #21: MEDIUM - Missing Database Indexes for Performance - -**Where found**: -- fail2ban's `bans.timeofban` column likely has no index -- BanGUI queries that filter by timeofban + jail do full table scan -- Large datasets cause slow dashboard loads - -**Why this is needed**: -Without indexes on commonly filtered columns, query performance degrades as dataset grows. - -**Goal**: -Add appropriate indexes to improve query performance. - -**What to do**: -1. Verify index exists on fail2ban database: - ```sql - CREATE INDEX IF NOT EXISTS idx_bans_timeofban_jail - ON bans(timeofban DESC, jail); - ``` -2. Check BanGUI database for missing indexes -3. Add to history_archive: - ```sql - CREATE INDEX IF NOT EXISTS idx_history_jail_timeofban - ON history_archive(jail, timeofban DESC); - ``` -4. Monitor query performance before/after -5. Add index creation to migration system - -**Possible traps and issues**: -- Creating indexes on large tables locks table (need CONCURRENT on PostgreSQL) -- Wrong index type slows queries instead of helping -- Too many indexes slow writes - -**Docs changes needed**: -- Document query optimization patterns -- Performance tuning guide - -**Doc references**: -- DETAILED_FINDINGS.md - Issue #10 "Missing DB Index" - ---- - ### Issue #22: MEDIUM - Socket Cleanup Errors Silently Suppressed **Where found**: diff --git a/backend/app/utils/external_logging.py b/backend/app/utils/external_logging.py index 45bd1c7..f3e4639 100644 --- a/backend/app/utils/external_logging.py +++ b/backend/app/utils/external_logging.py @@ -7,7 +7,6 @@ fashion using async buffering and batching. from __future__ import annotations import asyncio -import contextlib import json from abc import ABC, abstractmethod from collections import deque @@ -244,8 +243,10 @@ class PapertrailLogHandler(ExternalLogHandler): await writer.drain() finally: writer.close() - with contextlib.suppress(Exception): + try: await writer.wait_closed() + except Exception as e: + log.warning("external_logging_writer_close_error", error=str(e)) @staticmethod def _severity_from_level(level: str) -> int: diff --git a/backend/app/utils/fail2ban_client.py b/backend/app/utils/fail2ban_client.py index 6cf3cbc..bae0ebd 100644 --- a/backend/app/utils/fail2ban_client.py +++ b/backend/app/utils/fail2ban_client.py @@ -17,7 +17,6 @@ Usage:: from __future__ import annotations import asyncio -import contextlib import errno import sys import time @@ -189,8 +188,15 @@ def _send_command_sync( ) from exc finally: if client is not None: - with contextlib.suppress(Exception): + try: client.close() + except Exception as e: + log.warning( + "fail2ban_socket_close_error", + socket_path=socket_path, + error=str(e), + exc_info=True, + ) raise Fail2BanConnectionError( str(last_oserror), socket_path diff --git a/backend/tests/test_services/test_fail2ban_client.py b/backend/tests/test_services/test_fail2ban_client.py index ae1b62b..bcb74fb 100644 --- a/backend/tests/test_services/test_fail2ban_client.py +++ b/backend/tests/test_services/test_fail2ban_client.py @@ -214,6 +214,38 @@ class TestCoerceCommandToken: assert _coerce_command_token(None) == "None" +class TestSendCommandSyncCloseError: + """Tests for socket close error handling in :func:`_send_command_sync`.""" + + def test_close_error_is_logged_but_not_reRaised(self) -> None: + """close() raising must log a warning, not mask the original error.""" + fake_client = MagicMock() + fake_client.send.return_value = [0, "ok"] + fake_client.close.side_effect = RuntimeError("close failed") + fake_cls = MagicMock(return_value=fake_client) + + with ( + patch( + "app.utils.fail2ban_client._load_vendored_fail2ban_client", + return_value=fake_cls, + ), + patch("app.utils.fail2ban_client.log") as mock_log, + ): + result = _send_command_sync( + socket_path="/fake/fail2ban.sock", + command=["status"], + timeout=1.0, + ) + + assert result == [0, "ok"] + warning_calls = [ + c for c in mock_log.warning.call_args_list + if c[0][0] == "fail2ban_socket_close_error" + ] + assert len(warning_calls) == 1 + assert warning_calls[0][1]["error"] == "close failed" + + # --------------------------------------------------------------------------- # Extended tests for Fail2BanClient.send # ---------------------------------------------------------------------------