mirror of
https://github.com/pyMC-dev/pyMC_Repeater.git
synced 2026-07-27 20:12:41 +02:00
fix(repeater): report all boot-time config errors without a stack trace
Startup config mistakes were handled inconsistently: an identity collision exited cleanly, but a missing or invalid config file (FileNotFoundError / RuntimeError from load_config) and a missing identity key (RuntimeError) dumped a full traceback -- and load_config ran outside main()'s try, so those errors escaped the fatal handler entirely. Add repeater/exceptions.py with ConfigurationError(RuntimeError) and re-parent IdentityConfigurationError onto it (it stays a RuntimeError, so existing except-sites are unaffected). Raise ConfigurationError for the missing/invalid config file, the config-load failure, and the missing identity key. Move load_config and daemon construction inside main()'s try and catch ConfigurationError there, logging just the message and exiting 1; unexpected failures still log with a traceback.
This commit is contained in:
+3
-2
@@ -6,6 +6,7 @@ from typing import Any, Dict, Optional, overload
|
||||
|
||||
import yaml
|
||||
|
||||
from repeater.exceptions import ConfigurationError
|
||||
from repeater.policy_engine import default_policy_engine_config
|
||||
|
||||
logger = logging.getLogger("Config")
|
||||
@@ -202,7 +203,7 @@ def load_config(config_path: Optional[str] = None) -> Dict[str, Any]:
|
||||
|
||||
# Check if config file exists
|
||||
if not Path(config_path).exists():
|
||||
raise FileNotFoundError(
|
||||
raise ConfigurationError(
|
||||
f"Configuration file not found: {config_path}\n"
|
||||
f"Please create a config file. Example: \n"
|
||||
f" sudo cp {Path(config_path).parent}/config.yaml.example {config_path}\n"
|
||||
@@ -215,7 +216,7 @@ def load_config(config_path: Optional[str] = None) -> Dict[str, Any]:
|
||||
config = yaml.safe_load(f) or {}
|
||||
logger.info(f"Loaded config from {config_path}")
|
||||
except Exception as e:
|
||||
raise RuntimeError(f"Failed to load configuration from {config_path}: {e}") from e
|
||||
raise ConfigurationError(f"Failed to load configuration from {config_path}: {e}") from e
|
||||
|
||||
storage_dir = resolve_storage_dir(config, config_path=config_path)
|
||||
if "storage" not in config or not isinstance(config.get("storage"), dict):
|
||||
|
||||
@@ -0,0 +1,17 @@
|
||||
"""Shared exception types for the repeater.
|
||||
|
||||
Kept dependency-free (no other ``repeater`` imports) so any module can raise
|
||||
these without risking an import cycle.
|
||||
"""
|
||||
|
||||
|
||||
class ConfigurationError(RuntimeError):
|
||||
"""A user-actionable configuration problem.
|
||||
|
||||
Raised for boot-time config mistakes (missing/invalid config file, missing
|
||||
required keys, colliding local identities). ``main()`` reports the message
|
||||
and exits non-zero *without* a stack trace, since the fix is in the config,
|
||||
not the code. Subclasses ``RuntimeError`` so existing ``except
|
||||
RuntimeError`` sites keep catching the errors that used to be raised as
|
||||
plain ``RuntimeError``.
|
||||
"""
|
||||
@@ -2,6 +2,8 @@ import logging
|
||||
from dataclasses import dataclass
|
||||
from typing import Any, Dict, Iterable, Optional, Tuple
|
||||
|
||||
from repeater.exceptions import ConfigurationError
|
||||
|
||||
logger = logging.getLogger("IdentityManager")
|
||||
|
||||
|
||||
@@ -22,7 +24,7 @@ def _namespace_for(identity_type: str) -> str:
|
||||
return "companion" if identity_type == "companion" else "server"
|
||||
|
||||
|
||||
class IdentityConfigurationError(RuntimeError):
|
||||
class IdentityConfigurationError(ConfigurationError):
|
||||
"""A configured local identity cannot be represented safely."""
|
||||
|
||||
|
||||
|
||||
+21
-19
@@ -25,6 +25,7 @@ from repeater.config_manager import ConfigManager
|
||||
from repeater.data_acquisition.glass_handler import GlassHandler
|
||||
from repeater.data_acquisition.gps_service import GPSService
|
||||
from repeater.engine import RepeaterHandler
|
||||
from repeater.exceptions import ConfigurationError
|
||||
from repeater.handler_helpers import (
|
||||
AdvertHelper,
|
||||
DiscoveryHelper,
|
||||
@@ -404,7 +405,7 @@ class RepeaterDaemon:
|
||||
identity_key = self.config.get("repeater", {}).get("identity_key")
|
||||
if not identity_key:
|
||||
logger.error("No identity key found in configuration. Cannot init repeater.")
|
||||
raise RuntimeError("Identity key is required for repeater operation")
|
||||
raise ConfigurationError("Identity key is required for repeater operation")
|
||||
|
||||
local_identity = LocalIdentity(seed=identity_key)
|
||||
self.local_identity = local_identity
|
||||
@@ -1755,28 +1756,29 @@ def main():
|
||||
|
||||
args = parser.parse_args()
|
||||
|
||||
# Load configuration
|
||||
config = load_config(args.config)
|
||||
config_path = args.config if args.config else "/etc/openhop_repeater/config.yaml"
|
||||
|
||||
if args.log_level:
|
||||
if "logging" not in config:
|
||||
config["logging"] = {}
|
||||
config["logging"]["level"] = args.log_level
|
||||
|
||||
# Don't initialize radio here - it will be done inside the async event loop
|
||||
daemon = RepeaterDaemon(config, radio=None)
|
||||
daemon.config_path = config_path
|
||||
|
||||
# Run
|
||||
# Load configuration, build the daemon, and run it. Config mistakes (a
|
||||
# missing or invalid config file, a missing required key, colliding local
|
||||
# identities) surface as ConfigurationError and exit cleanly with just the
|
||||
# message; only unexpected failures get the full traceback.
|
||||
try:
|
||||
config = load_config(args.config)
|
||||
config_path = args.config if args.config else "/etc/openhop_repeater/config.yaml"
|
||||
|
||||
if args.log_level:
|
||||
if "logging" not in config:
|
||||
config["logging"] = {}
|
||||
config["logging"]["level"] = args.log_level
|
||||
|
||||
# Don't initialize radio here - it will be done inside the async event loop
|
||||
daemon = RepeaterDaemon(config, radio=None)
|
||||
daemon.config_path = config_path
|
||||
|
||||
asyncio.run(daemon.run())
|
||||
except KeyboardInterrupt:
|
||||
logger.info("Repeater stopped")
|
||||
except IdentityConfigurationError as e:
|
||||
# A misconfigured local identity is an actionable config problem, not a
|
||||
# crash: report just the message so the fix is obvious, no stack trace.
|
||||
logger.error("Identity configuration error: %s", e)
|
||||
except ConfigurationError as e:
|
||||
# An actionable config problem, not a crash: report just the message.
|
||||
logger.error("Configuration error: %s", e)
|
||||
sys.exit(1)
|
||||
except Exception as e:
|
||||
logger.error(f"Fatal error: {e}", exc_info=True)
|
||||
|
||||
@@ -5,6 +5,7 @@ from unittest.mock import AsyncMock, MagicMock, call, patch
|
||||
import pytest
|
||||
|
||||
from repeater.companion.constants import STATS_TYPE_CORE, STATS_TYPE_PACKETS, STATS_TYPE_RADIO
|
||||
from repeater.exceptions import ConfigurationError
|
||||
from repeater.identity_manager import IdentityConfigurationError
|
||||
from repeater.main import RepeaterDaemon
|
||||
from repeater.main import main as repeater_main
|
||||
@@ -538,7 +539,8 @@ def test_main_entrypoint_success_and_fatal_paths(monkeypatch):
|
||||
|
||||
|
||||
def test_main_identity_config_error_exits_cleanly_without_traceback(caplog):
|
||||
"""A configured-identity collision exits 1 with a clean message and no
|
||||
"""A configured-identity collision (an IdentityConfigurationError, which is
|
||||
a ConfigurationError subclass) exits 1 with a clean message and no
|
||||
stack-trace dump (regression: the fatal handler used to log exc_info=True
|
||||
for every exception)."""
|
||||
|
||||
@@ -563,7 +565,62 @@ def test_main_identity_config_error_exits_cleanly_without_traceback(caplog):
|
||||
repeater_main()
|
||||
|
||||
exit_mock.assert_called_once_with(1)
|
||||
config_errors = [r for r in caplog.records if "Identity configuration error" in r.getMessage()]
|
||||
config_errors = [r for r in caplog.records if "Configuration error" in r.getMessage()]
|
||||
assert len(config_errors) == 1
|
||||
assert config_errors[0].exc_info is None # no traceback attached
|
||||
assert not any("Fatal error" in r.getMessage() for r in caplog.records)
|
||||
|
||||
|
||||
def test_main_config_load_error_exits_cleanly_without_traceback(caplog):
|
||||
"""A ConfigurationError from load_config (missing/invalid config file) is
|
||||
now inside the try, so it also exits 1 cleanly rather than dumping a trace
|
||||
from before the event loop starts."""
|
||||
|
||||
class _Args:
|
||||
config = "/tmp/missing.yaml"
|
||||
log_level = None
|
||||
|
||||
with (
|
||||
patch("argparse.ArgumentParser.parse_args", return_value=_Args()),
|
||||
patch(
|
||||
"repeater.main.load_config",
|
||||
side_effect=ConfigurationError("Configuration file not found: /tmp/missing.yaml"),
|
||||
),
|
||||
patch("sys.exit", side_effect=SystemExit(1)) as exit_mock,
|
||||
caplog.at_level(logging.ERROR),
|
||||
):
|
||||
with pytest.raises(SystemExit):
|
||||
repeater_main()
|
||||
|
||||
exit_mock.assert_called_once_with(1)
|
||||
config_errors = [r for r in caplog.records if "Configuration error" in r.getMessage()]
|
||||
assert len(config_errors) == 1
|
||||
assert config_errors[0].exc_info is None
|
||||
assert not any("Fatal error" in r.getMessage() for r in caplog.records)
|
||||
|
||||
|
||||
def test_main_unexpected_error_keeps_traceback(caplog):
|
||||
"""A genuine, non-config failure still logs 'Fatal error' with a traceback."""
|
||||
|
||||
class _Args:
|
||||
config = "/tmp/test.yaml"
|
||||
log_level = None
|
||||
|
||||
fake_daemon = SimpleNamespace(run=MagicMock(return_value=object()))
|
||||
|
||||
with (
|
||||
patch("argparse.ArgumentParser.parse_args", return_value=_Args()),
|
||||
patch("repeater.main.load_config", return_value=_base_config()),
|
||||
patch("repeater.main.RepeaterDaemon", return_value=fake_daemon),
|
||||
patch("asyncio.run", side_effect=RuntimeError("boom")),
|
||||
patch("sys.exit", side_effect=SystemExit(1)) as exit_mock,
|
||||
caplog.at_level(logging.ERROR),
|
||||
):
|
||||
with pytest.raises(SystemExit):
|
||||
repeater_main()
|
||||
|
||||
exit_mock.assert_called_once_with(1)
|
||||
fatal = [r for r in caplog.records if "Fatal error" in r.getMessage()]
|
||||
assert len(fatal) == 1
|
||||
assert fatal[0].exc_info is not None # traceback preserved for real crashes
|
||||
assert not any("Configuration error" in r.getMessage() for r in caplog.records)
|
||||
|
||||
Reference in New Issue
Block a user