From 42cd75e6b001c4e9369b1feeff19a2aa6af83a85 Mon Sep 17 00:00:00 2001
From: Lloyd <lloyd@rightup.co.uk>
Date: Thu, 25 Jun 2026 21:26:29 +0100
Subject: [PATCH] refactor: enhance logging and error handling in API endpoints
 and HTTP server

---
 .gitignore                                |  2 ++
 repeater/web/api_endpoints.py             | 15 +++++++++++++-
 repeater/web/http_server.py               |  5 ++++-
 tests/test_api_endpoints_core_coverage.py | 10 ++++++++++
 tests/test_http_server_unit.py            | 24 +++++++++++++++++++++++
 5 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 21aee28..c88b2a6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,6 +38,8 @@ debian/openhop-repeater.substvars
 env/
 ENV/
 
+.ruff_cache
+
 # Testing
 .pytest_cache/
 .coverage
diff --git a/repeater/web/api_endpoints.py b/repeater/web/api_endpoints.py
index 0ca2149..a51ab7e 100644
--- a/repeater/web/api_endpoints.py
+++ b/repeater/web/api_endpoints.py
@@ -4,6 +4,7 @@ import os
 import re
 import secrets
 import time
+from concurrent.futures import TimeoutError as FutureTimeoutError
 from datetime import datetime, timezone
 from typing import Callable, Optional
 
@@ -1587,6 +1588,7 @@ class APIEndpoints:
                 return self._error("Event loop not available")
             import asyncio
 
+            future = None
             future = asyncio.run_coroutine_threadsafe(self.send_advert_func(), self.event_loop)
             result = future.result(timeout=10)
             return (
@@ -1594,11 +1596,22 @@ class APIEndpoints:
                 if result
                 else self._error("Failed to send advert")
             )
+        except FutureTimeoutError:
+            logger.error(
+                "Error sending advert: timeout waiting for advert transmission to complete",
+                exc_info=True,
+            )
+            try:
+                if future is not None:
+                    future.cancel()
+            except Exception:
+                pass
+            return self._error("Timed out waiting for advert transmission after 10 seconds")
         except cherrypy.HTTPError:
             # Re-raise HTTP errors (like 405 Method Not Allowed) without logging
             raise
         except Exception as e:
-            logger.error(f"Error sending advert: {e}", exc_info=True)
+            logger.error("Error sending advert: %s", e, exc_info=True)
             return self._error(e)
 
     @cherrypy.expose
diff --git a/repeater/web/http_server.py b/repeater/web/http_server.py
index 4a72916..da1531f 100644
--- a/repeater/web/http_server.py
+++ b/repeater/web/http_server.py
@@ -94,7 +94,10 @@ class LogBuffer(logging.Handler):
             }
 
             if record.exc_info:
-                entry["exception"] = self.formatException(record.exc_info)
+                formatter = self.formatter or logging.Formatter()
+                entry["exception"] = self._sanitize_log_text(
+                    formatter.formatException(record.exc_info)
+                )
 
             with self._lock:
                 self.logs.append(entry)
diff --git a/tests/test_api_endpoints_core_coverage.py b/tests/test_api_endpoints_core_coverage.py
index 0895324..0a45a6d 100644
--- a/tests/test_api_endpoints_core_coverage.py
+++ b/tests/test_api_endpoints_core_coverage.py
@@ -1,6 +1,7 @@
 from pathlib import Path
 from types import SimpleNamespace
 from unittest.mock import MagicMock, mock_open, patch
+from concurrent.futures import TimeoutError as FutureTimeoutError
 
 import cherrypy
 import pytest
@@ -1279,6 +1280,15 @@ def test_send_advert_paths(cherrypy_ctx):
         bad = api.send_advert()
     assert bad["success"] is False
 
+    future_timeout = MagicMock()
+    future_timeout.result.side_effect = FutureTimeoutError()
+    with patch("asyncio.run_coroutine_threadsafe", return_value=future_timeout):
+        timeout = api.send_advert()
+
+    assert timeout["success"] is False
+    assert "Timed out waiting for advert transmission" in timeout["error"]
+    future_timeout.cancel.assert_called_once()
+
 
 def test_set_mode_and_set_duty_cycle_paths(cherrypy_ctx):
     request, _ = cherrypy_ctx
diff --git a/tests/test_http_server_unit.py b/tests/test_http_server_unit.py
index 0b69349..d2083f4 100644
--- a/tests/test_http_server_unit.py
+++ b/tests/test_http_server_unit.py
@@ -1,5 +1,6 @@
 import io
 import logging
+import sys
 from pathlib import Path
 from types import SimpleNamespace
 
@@ -47,6 +48,29 @@ def test_log_buffer_emit_redacts_sensitive_values():
     assert "raw_message" not in entry
 
 
+def test_log_buffer_emit_includes_exception_text_without_crashing():
+    buf = hs.LogBuffer(max_lines=5)
+    try:
+        raise RuntimeError("boom password=secret123")
+    except RuntimeError:
+        rec = logging.LogRecord(
+            "x",
+            logging.ERROR,
+            __file__,
+            20,
+            "failure while sending advert",
+            (),
+            sys.exc_info(),
+        )
+
+    buf.emit(rec)
+
+    assert len(buf.logs) == 1
+    assert "exception" in buf.logs[0]
+    assert "RuntimeError" in buf.logs[0]["exception"]
+    assert "secret123" not in buf.logs[0]["exception"]
+
+
 def test_doc_endpoint_routes_and_openapi_json_paths(monkeypatch):
     api = SimpleNamespace(docs=lambda: "docs-html")
     doc = hs.DocEndpoint(api)