From c8c7f2d732127d209911e22906baacad0be2b4d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20P=C3=A9rez?= Date: Wed, 17 Jun 2026 11:38:53 -0400 Subject: [PATCH] fix: log clear MariaDB startup failures to file and stderr Validate DATABASE at config load and on connect; map common MySQL errors to actionable messages so the server does not fail silently. --- .../infrastructure/bootstrap/peer_server.py | 1 + .../infrastructure/config_validator.py | 17 ++++++ .../persistence/database_config.py | 9 +++ .../infrastructure/persistence/mysql_pool.py | 59 +++++++++++++++---- .../infrastructure/test_mysql_pool_ensure.py | 17 +++++- 5 files changed, 91 insertions(+), 12 deletions(-) diff --git a/src/adn_server/infrastructure/bootstrap/peer_server.py b/src/adn_server/infrastructure/bootstrap/peer_server.py index 80fd282..d379b88 100644 --- a/src/adn_server/infrastructure/bootstrap/peer_server.py +++ b/src/adn_server/infrastructure/bootstrap/peer_server.py @@ -241,6 +241,7 @@ def run_peer_server( db_settings["db_name"], db_settings["db_port"], ): + logger.critical("(GLOBAL) Startup aborted — MariaDB required for dynamic TG persistence") raise SystemExit(1) mysql_pool = create_mysql_pool( db_settings["db_server"], diff --git a/src/adn_server/infrastructure/config_validator.py b/src/adn_server/infrastructure/config_validator.py index b7ef39d..f2280f3 100644 --- a/src/adn_server/infrastructure/config_validator.py +++ b/src/adn_server/infrastructure/config_validator.py @@ -262,6 +262,22 @@ def _validate_proxy(proxy_cfg: dict[str, Any] | None, systems: dict[str, Any], e ) +def _validate_database(db_cfg: Any, errors: list[str]) -> None: + if db_cfg is None: + errors.append("DATABASE: required block missing in adn-server.yaml") + return + if not isinstance(db_cfg, dict): + errors.append(f"DATABASE: expected mapping, got {type(db_cfg).__name__}.") + return + if not str(db_cfg.get("DB_NAME", "")).strip(): + errors.append("DATABASE.DB_NAME: required.") + if not str(db_cfg.get("DB_USERNAME", "")).strip(): + errors.append("DATABASE.DB_USERNAME: required.") + port = db_cfg.get("DB_PORT", 3306) + if isinstance(port, bool) or not isinstance(port, int) or port < 1: + errors.append("DATABASE.DB_PORT: expected integer >= 1.") + + def _validate_system(name: str, sys_cfg: dict[str, Any], errors: list[str]) -> None: prefix = f"SYSTEMS.{name}" _section_string_keys(prefix, sys_cfg, SYSTEM_STRING_KEYS, errors) @@ -319,6 +335,7 @@ def validate_config(config: dict[str, Any], *, config_path: str | None = None) - proxy_cfg = config.get("PROXY") _validate_proxy(proxy_cfg if isinstance(proxy_cfg, dict) else None, systems if isinstance(systems, dict) else {}, errors) + _validate_database(config.get("DATABASE"), errors) if errors: header = f"Configuration error in {config_path}:" if config_path else "Configuration error:" diff --git a/src/adn_server/infrastructure/persistence/database_config.py b/src/adn_server/infrastructure/persistence/database_config.py index 964b652..821fb7d 100644 --- a/src/adn_server/infrastructure/persistence/database_config.py +++ b/src/adn_server/infrastructure/persistence/database_config.py @@ -40,3 +40,12 @@ def database_settings(config: dict[str, Any]) -> dict[str, Any]: "db_name": str(block.get("DB_NAME", "")), "db_port": int(block.get("DB_PORT", 3306)), } + + +def validate_database_settings(settings: dict[str, Any]) -> str | None: + """Return a user-facing error when required DATABASE fields are missing.""" + if not str(settings.get("db_name", "")).strip(): + return "DATABASE.DB_NAME is missing or empty in adn-server.yaml" + if not str(settings.get("db_username", "")).strip(): + return "DATABASE.DB_USERNAME is missing or empty in adn-server.yaml" + return None diff --git a/src/adn_server/infrastructure/persistence/mysql_pool.py b/src/adn_server/infrastructure/persistence/mysql_pool.py index 4bf6018..e75e2a9 100644 --- a/src/adn_server/infrastructure/persistence/mysql_pool.py +++ b/src/adn_server/infrastructure/persistence/mysql_pool.py @@ -23,14 +23,25 @@ from __future__ import annotations import logging +import sys from typing import Any from twisted.enterprise import adbapi +from adn_server.infrastructure.persistence.database_config import validate_database_settings + logger = logging.getLogger(__name__) _PEER_DYNAMIC_TGS_MIGRATION = "004_peer_dynamic_tgs" +_MYSQL_HINTS: dict[int, str] = { + 1045: "check DATABASE.DB_USERNAME and DB_PASSWORD in adn-server.yaml", + 1049: "database does not exist — create it or fix DATABASE.DB_NAME", + 1044: "user lacks permission on the database — fix MariaDB grants or DATABASE settings", + 2003: "cannot reach MariaDB — check DB_SERVER/DB_PORT and that MariaDB is running", + 2002: "connection refused — is MariaDB listening on DB_SERVER:DB_PORT?", +} + _CREATE_SCHEMA_MIGRATIONS = """CREATE TABLE IF NOT EXISTS schema_migrations ( id VARCHAR(64) PRIMARY KEY, applied_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP @@ -51,6 +62,27 @@ _CREATE_PEER_DYNAMIC_TGS = """CREATE TABLE IF NOT EXISTS peer_dynamic_tgs ( ) DEFAULT CHARSET=utf8mb4""" +def describe_mysql_error(err: Exception) -> str: + """Turn a MySQL/MariaDB exception into an actionable startup message.""" + args = getattr(err, "args", ()) + if len(args) >= 2 and isinstance(args[0], int): + code, msg = int(args[0]), str(args[1]) + hint = _MYSQL_HINTS.get(code, "check the DATABASE block in adn-server.yaml") + return f"MySQL error {code}: {msg} ({hint})" + return str(err) + + +def abort_database_startup(reason: str) -> bool: + """Log and print a fatal DATABASE message; return False for ensure_database_sync.""" + headline = f"(DATABASE) ADN DMR Peer Server not started — {reason}" + logger.critical(headline) + logger.critical( + "(DATABASE) Dynamic TG persistence requires MariaDB; fix DATABASE in adn-server.yaml and restart" + ) + print(headline, file=sys.stderr) + return False + + def create_mysql_pool( host: str, user: str, @@ -100,14 +132,20 @@ def ensure_database_sync( port: int, ) -> bool: """Blocking startup: connect, ensure ``peer_dynamic_tgs`` exists (idempotent).""" + settings = { + "db_server": host, + "db_username": user, + "db_password": password, + "db_name": db_name, + "db_port": port, + } + config_err = validate_database_settings(settings) + if config_err: + return abort_database_startup(config_err) try: import MySQLdb except ImportError as err: - logger.critical( - "(DATABASE) mysqlclient required for dynamic TG persistence: %s", - err, - ) - return False + return abort_database_startup(f"mysqlclient package required: {err}") try: conn = MySQLdb.connect( host=host, @@ -122,11 +160,10 @@ def ensure_database_sync( conn.commit() cur.close() conn.close() - logger.info("(DATABASE) peer_dynamic_tgs table: OK") + logger.info( + "(DATABASE) MariaDB OK (%s@%s:%s/%s, peer_dynamic_tgs ready)", + user, host, port, db_name, + ) return True except Exception as err: - logger.critical( - "(DATABASE) startup ensure failed: %s (check DATABASE in adn-server.yaml)", - err, - ) - return False + return abort_database_startup(describe_mysql_error(err)) diff --git a/tests/infrastructure/test_mysql_pool_ensure.py b/tests/infrastructure/test_mysql_pool_ensure.py index 1d64b1e..106b26a 100644 --- a/tests/infrastructure/test_mysql_pool_ensure.py +++ b/tests/infrastructure/test_mysql_pool_ensure.py @@ -24,7 +24,22 @@ from __future__ import annotations from unittest.mock import MagicMock -from adn_server.infrastructure.persistence.mysql_pool import _ensure_peer_dynamic_tgs_on_cursor +from adn_server.infrastructure.persistence.mysql_pool import ( + _ensure_peer_dynamic_tgs_on_cursor, + describe_mysql_error, +) +from adn_server.infrastructure.persistence.database_config import validate_database_settings + + +def test_validate_database_settings_requires_db_name() -> None: + assert validate_database_settings({"db_username": "hbmon", "db_name": ""}) is not None + + +def test_describe_mysql_error_unknown_database() -> None: + err = Exception(1049, "Unknown database 'missing'") + msg = describe_mysql_error(err) + assert "1049" in msg + assert "DATABASE.DB_NAME" in msg def test_ensure_peer_dynamic_tgs_applies_migration_once() -> None: