From f4587392dbccf9d9558ab621795c9391df09d9dd Mon Sep 17 00:00:00 2001 From: agessaman Date: Wed, 29 Jul 2026 19:49:50 -0700 Subject: [PATCH] fix(config): update auto_manage_contacts default behavior and documentation Changed the default setting for `auto_manage_contacts` from `false` to `device` across configuration files and updated related documentation. This ensures that the device handles auto-addition of contacts while the bot manages capacity. Adjusted comments and documentation to reflect this change for clarity and consistency. --- config.ini.example | 13 ++- config.ini.minimal-example | 12 ++- docs/command-reference.md | 2 +- docs/repeater-commands.md | 4 +- docs/service-installation.md | 9 +- modules/core.py | 6 +- modules/message_handler.py | 4 +- modules/repeater_manager.py | 12 +-- modules/scheduler.py | 8 +- modules/web_viewer/app.py | 100 ++++++++++++------- modules/web_viewer/integration.py | 143 +++++++++++++++++---------- tests/test_web_viewer.py | 52 ++++++++++ tests/test_web_viewer_integration.py | 41 ++++++++ 13 files changed, 290 insertions(+), 116 deletions(-) diff --git a/config.ini.example b/config.ini.example index 6d0a3b0..684f613 100644 --- a/config.ini.example +++ b/config.ini.example @@ -211,10 +211,10 @@ advert_interval_hours = 0 startup_advert = false # Auto-manage contact list when new contacts are discovered -# device: Device handles auto-addition using standard auto-discovery mode, bot manages contact list capacity (purge old contacts when near limits) +# device: Device handles auto-addition using standard auto-discovery mode, bot manages contact list capacity (purge old contacts when near limits) (default) # bot: Bot automatically adds new companion contacts to device, bot manages contact list capacity (purge old contacts when near limits) -# false: Manual mode - no automatic actions, use !repeater commands to manage contacts (default) -auto_manage_contacts = bot +# false: Manual mode - no automatic actions, use !repeater commands to manage contacts +auto_manage_contacts = device # When auto_manage_contacts = device, the bot schedules one-shot jobs after connect: # - Firmware: manual per-type contact adds + overwrite oldest non-favourite on full + auto-add chat (companions) only (bitmask 0x03) @@ -571,9 +571,12 @@ dm_only = true # CRITICAL: Only critical errors log_level = INFO -# Log file path (leave empty for console only) -# Bot will write logs to this file in addition to console +# Log file path (leave empty for console/journald only) +# Bot and web viewer write to this file in addition to console when set. +# Empty: console only (captured by journald under systemd — preferred for services). +# systemd service installs use: /var/log/meshcore-bot/meshcore_bot.log # Logs rotate at 5 MB with up to 3 backup files (e.g. meshcore_bot.log.1, .2, .3) +# Web viewer file logs (when set) go beside this path as web_viewer.log log_file = meshcore_bot.log # Enable colored console output diff --git a/config.ini.minimal-example b/config.ini.minimal-example index 856a838..a0bcab7 100644 --- a/config.ini.minimal-example +++ b/config.ini.minimal-example @@ -149,10 +149,10 @@ advert_interval_hours = 0 startup_advert = false # Auto-manage contact list when new contacts are discovered -# device: Device handles auto-addition using standard auto-discovery mode, bot manages contact list capacity (purge old contacts when near limits) +# device: Device handles auto-addition using standard auto-discovery mode, bot manages contact list capacity (purge old contacts when near limits) (default) # bot: Bot automatically adds new companion contacts to device, bot manages contact list capacity (purge old contacts when near limits) -# false: Manual mode - no automatic actions, use !repeater commands to manage contacts (default) -auto_manage_contacts = false +# false: Manual mode - no automatic actions, use !repeater commands to manage contacts +auto_manage_contacts = device # Database path for main bot database # Default: meshcore_bot.db @@ -262,8 +262,10 @@ allow_private_urls = false # CRITICAL: Only critical errors log_level = INFO -# Log file path (leave empty for console only) -# Bot will write logs to this file in addition to console +# Log file path (leave empty for console/journald only) +# Bot and web viewer write to this file in addition to console when set. +# Empty: console only (captured by journald under systemd — preferred for services). +# systemd service installs use: /var/log/meshcore-bot/meshcore_bot.log # Logs rotate at 5 MB with up to 3 backup files (e.g. meshcore_bot.log.1, .2, .3) log_file = meshcore_bot.log diff --git a/docs/command-reference.md b/docs/command-reference.md index 197f89c..e2f804b 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -954,7 +954,7 @@ repeater stats - NEW_CONTACT events are automatically monitored - Repeaters are automatically cataloged when discovered - Contact list capacity is monitored in real-time -- `auto_manage_contacts = device`: Firmware auto-adds **chat (companion)** peers only, with **overwrite oldest non-favourite** when the contact table is full; the bot schedules delayed jobs to set that firmware policy and to **favourite** keys in `Admin_ACL` plus the effective announcements ACL (same rules as the announcements command), then clear **favourite** on other contacts. The bot still runs capacity management on NEW_CONTACT (near-limit `manage_contact_list`) and does **not** call `add_contact` for new companions itself. **Contact limit** for logging and capacity is taken from the radio’s `max_contacts` and, if the live table is larger (under-reported max), raised to match the mesh so counts are not shown as over-capacity. **Companion auto-purge** never runs on the radio in this mode. Count-based **repeater** auto-purge only runs if the table grows **strictly above** that synced limit (normally off while the firmware manages slots). +- `auto_manage_contacts = device` (default): Firmware auto-adds **chat (companion)** peers only, with **overwrite oldest non-favourite** when the contact table is full; the bot schedules delayed jobs to set that firmware policy and to **favourite** keys in `Admin_ACL` plus the effective announcements ACL (same rules as the announcements command), then clear **favourite** on other contacts. The bot still runs capacity management on NEW_CONTACT (near-limit `manage_contact_list`) and does **not** call `add_contact` for new companions itself. **Contact limit** for logging and capacity is taken from the radio’s `max_contacts` and, if the live table is larger (under-reported max), raised to match the mesh so counts are not shown as over-capacity. **Companion auto-purge** never runs on the radio in this mode. Count-based **repeater** auto-purge only runs if the table grows **strictly above** that synced limit (normally off while the firmware manages slots). - `auto_manage_contacts = bot`: Bot adds new companions via `add_contact` (full NEW_CONTACT payload), runs **manage-before-add** when the list is near limit, and **retries once** after `manage_contact_list` if the radio returns `TABLE_FULL`. - `auto_manage_contacts = false`: Manual mode - NEW_CONTACT companions are tracked in the database only; use `!repeater` commands to manage the device list. diff --git a/docs/repeater-commands.md b/docs/repeater-commands.md index 1f6a60c..ff3e795 100644 --- a/docs/repeater-commands.md +++ b/docs/repeater-commands.md @@ -594,8 +594,8 @@ Shows comprehensive help for all repeater commands. The repeater management system respects several configuration settings in `config.ini`: -- `auto_manage_contacts` - Controls automatic contact management - - `device` - Device handles auto-addition, bot manages capacity +- `auto_manage_contacts` - Controls automatic contact management (default: `device`) + - `device` - Device handles auto-addition, bot manages capacity (default) - `bot` - Bot automatically adds companion contacts and manages capacity - `false` - Manual mode (use commands to manage contacts) diff --git a/docs/service-installation.md b/docs/service-installation.md index 01d1b58..64474f5 100644 --- a/docs/service-installation.md +++ b/docs/service-installation.md @@ -122,6 +122,8 @@ local_dir_path = /var/lib/meshcore-bot/local log_file = /var/log/meshcore-bot/meshcore_bot.log ``` +Leave `log_file` empty to use journald only (no log files). The installer and `.deb` package rewrite relative `log_file` values to `/var/log/meshcore-bot/`. + ### 6. Install the Service File ```bash @@ -146,7 +148,7 @@ sudo systemctl status meshcore-bot ### View Logs ```bash -# Real-time logs +# Real-time logs (always available — stdout/stderr go to journald) sudo journalctl -u meshcore-bot -f # Recent logs @@ -154,6 +156,11 @@ sudo journalctl -u meshcore-bot -n 100 # Logs since boot sudo journalctl -u meshcore-bot -b + +# Optional file logs when [Logging] log_file is set (service default): +# /var/log/meshcore-bot/meshcore_bot.log +# /var/log/meshcore-bot/web_viewer.log +sudo tail -f /var/log/meshcore-bot/meshcore_bot.log ``` ### Enable/Disable Auto-start diff --git a/modules/core.py b/modules/core.py index b1f4b4a..97c493b 100644 --- a/modules/core.py +++ b/modules/core.py @@ -1068,10 +1068,10 @@ advert_interval_hours = 0 startup_advert = false # Auto-manage contact list when new contacts are discovered -# device: Device handles auto-addition using standard auto-discovery mode, bot manages contact list capacity (purge old contacts when near limits) +# device: Device handles auto-addition using standard auto-discovery mode, bot manages contact list capacity (purge old contacts when near limits) (default) # bot: Bot automatically adds new companion contacts to device, bot manages contact list capacity (purge old contacts when near limits) -# false: Manual mode - no automatic actions, use !repeater commands to manage contacts (default) -auto_manage_contacts = false +# false: Manual mode - no automatic actions, use !repeater commands to manage contacts +auto_manage_contacts = device [Admin_ACL] # Admin Access Control List (ACL) for restricted commands diff --git a/modules/message_handler.py b/modules/message_handler.py index 54db0eb..4fc26d4 100644 --- a/modules/message_handler.py +++ b/modules/message_handler.py @@ -3640,7 +3640,7 @@ class MessageHandler: return else: # COMPANION: track in DB; device add behaviour depends on auto_manage_contacts - auto_manage_setting = self.bot.config.get("Bot", "auto_manage_contacts", fallback="false").lower() + auto_manage_setting = self.bot.config.get("Bot", "auto_manage_contacts", fallback="device").lower() if known_contact: self.logger.info( "👤 Known companion advert: %s — auto_manage_contacts=%s", @@ -3740,7 +3740,7 @@ class MessageHandler: # For unknown contact types, handle based on auto_manage_contacts setting if hasattr(self.bot, "repeater_manager"): - auto_manage_setting = self.bot.config.get("Bot", "auto_manage_contacts", fallback="false").lower() + auto_manage_setting = self.bot.config.get("Bot", "auto_manage_contacts", fallback="device").lower() if auto_manage_setting == "device": # Device mode: Let device handle auto-addition, bot manages capacity diff --git a/modules/repeater_manager.py b/modules/repeater_manager.py index f01bc0c..cf19da4 100644 --- a/modules/repeater_manager.py +++ b/modules/repeater_manager.py @@ -82,7 +82,7 @@ class RepeaterManager: self.contact_limit = 300 # MeshCore device limit (will be updated from device info) self.auto_purge_threshold = 280 # Start purging when 280+ contacts # Respect auto_manage_contacts: manual mode (false) = no auto-purge; device/bot = auto-purge on - auto_manage = bot.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() + auto_manage = bot.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() self._auto_manage_contacts = auto_manage self.auto_purge_enabled = (auto_manage != 'false') @@ -2895,7 +2895,7 @@ class RepeaterManager: """Set companion-radio firmware: manual per-type adds + overwrite oldest non-favourite + chat-only (0x03).""" if not self.bot.meshcore or not hasattr(self.bot.meshcore, 'commands'): return False - if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() != 'device': + if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() != 'device': self.logger.info('Skipping firmware autoadd setup — auto_manage_contacts is not device') return False try: @@ -2915,7 +2915,7 @@ class RepeaterManager: async def sync_device_mode_favourites_pass1(self) -> None: """Favourite all on-device contacts whose pubkey is in the protected set (admin + announcements ACL).""" - if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() != 'device': + if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() != 'device': return if not self.bot.meshcore or not hasattr(self.bot.meshcore, 'commands'): return @@ -2930,7 +2930,7 @@ class RepeaterManager: self.logger.debug('get_contacts before favourite pass1: %s', e) contacts = getattr(self.bot.meshcore, 'contacts', None) or {} for pub_key, c in list(contacts.items()): - if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() != 'device': + if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() != 'device': return pk = (pub_key or '').lower() if pk not in protected: @@ -2956,7 +2956,7 @@ class RepeaterManager: async def sync_device_mode_favourites_pass2(self) -> None: """Clear favourite bit for contacts not in the protected set.""" - if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() != 'device': + if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() != 'device': return if not self.bot.meshcore or not hasattr(self.bot.meshcore, 'commands'): return @@ -2969,7 +2969,7 @@ class RepeaterManager: self.logger.debug('get_contacts before favourite pass2: %s', e) contacts = getattr(self.bot.meshcore, 'contacts', None) or {} for pub_key, c in list(contacts.items()): - if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() != 'device': + if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() != 'device': return pk = (pub_key or '').lower() if pk in protected: diff --git a/modules/scheduler.py b/modules/scheduler.py index 5119395..4ab4269 100644 --- a/modules/scheduler.py +++ b/modules/scheduler.py @@ -182,7 +182,7 @@ class MessageScheduler: """One-shot jobs for auto_manage_contacts=device: firmware autoadd + favourite hygiene.""" if self._apscheduler is None: return - if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() != 'device': + if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() != 'device': return try: delay_fw = max(0, self.bot.config.getint('Bot', 'device_mode_firmware_delay_seconds', fallback=30)) @@ -241,18 +241,18 @@ class MessageScheduler: await self.bot.repeater_manager.sync_device_mode_favourites_pass2() def _device_mode_firmware_job_sync(self) -> None: - if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() != 'device': + if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() != 'device': self.logger.debug('Skipping device_mode_firmware job — not device mode') return self._run_async_on_main_loop(self._device_mode_firmware_coro(), timeout=120.0) def _device_mode_favourite_pass1_job_sync(self) -> None: - if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() != 'device': + if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() != 'device': return self._run_async_on_main_loop(self._device_mode_favourite_pass1_coro(), timeout=600.0) def _device_mode_favourite_pass2_job_sync(self) -> None: - if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() != 'device': + if self.bot.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() != 'device': return self._run_async_on_main_loop(self._device_mode_favourite_pass2_coro(), timeout=600.0) diff --git a/modules/web_viewer/app.py b/modules/web_viewer/app.py index d50816f..1bae2d9 100644 --- a/modules/web_viewer/app.py +++ b/modules/web_viewer/app.py @@ -216,9 +216,6 @@ class BotDataViewer: } def __init__(self, db_path="meshcore_bot.db", repeater_db_path=None, config_path="config.ini"): - # Setup comprehensive logging - self._setup_logging() - # Set bot root directory (project root) for path validation # This is the directory containing the modules folder self.bot_root = Path(os.path.join(os.path.dirname(__file__), '..', '..')).resolve() @@ -226,6 +223,20 @@ class BotDataViewer: if not os.path.isabs(config_path): config_path = str(self.bot_root / config_path) + # Load configuration before logging so [Logging] log_file can select + # journal/console-only vs file logging (same rules as the main bot). + self.config = self._load_config(config_path) + self.config_path = config_path # kept for config.ini write-back endpoints + + # Resolve db_path relative to the config file's directory — matches core.py's bot_root + # property which is Path(config_file).parent.resolve(). Using self.bot_root (the project + # code root, 2 dirs above app.py) as the base caused a mismatch when config.ini lived + # elsewhere (e.g. a separate deployment directory), resulting in a blank realtime monitor + # because the web viewer and bot opened different database files. + self._config_base = Path(config_path).parent.resolve() if os.path.exists(config_path) else self.bot_root + + self._setup_logging() + self.app = Flask( __name__, template_folder=os.path.join(os.path.dirname(__file__), 'templates'), @@ -280,17 +291,6 @@ class BotDataViewer: self._contacts_badge_cache_signature = None self._contacts_badge_cache_chunks: set[str] = set() - # Load configuration - self.config = self._load_config(config_path) - self.config_path = config_path # kept for config.ini write-back endpoints - - # Resolve db_path relative to the config file's directory — matches core.py's bot_root - # property which is Path(config_file).parent.resolve(). Using self.bot_root (the project - # code root, 2 dirs above app.py) as the base caused a mismatch when config.ini lived - # elsewhere (e.g. a separate deployment directory), resulting in a blank realtime monitor - # because the web viewer and bot opened different database files. - self._config_base = Path(config_path).parent.resolve() if os.path.exists(config_path) else self.bot_root - # Use [Bot] db_path when [Web_Viewer] db_path is unset bot_db = self.config.get('Bot', 'db_path', fallback='meshcore_bot.db') if (self.config.has_section('Web_Viewer') and self.config.has_option('Web_Viewer', 'db_path') @@ -352,11 +352,27 @@ class BotDataViewer: self.logger.info("BotDataViewer initialized with Flask-SocketIO 5.x best practices") def _setup_logging(self): - """Setup comprehensive logging with rotation""" + """Setup logging; file handler only when [Logging] log_file is set. + + Empty log_file (or missing [Logging] section) means console/journal only, + matching the main bot. When a log file is configured, viewer logs go next + to it as web_viewer.log (e.g. /var/log/meshcore-bot/web_viewer.log). + """ from logging.handlers import RotatingFileHandler - # Create logs directory if it doesn't exist - os.makedirs('logs', exist_ok=True) + log_file = '' + log_max_bytes = 5 * 1024 * 1024 + log_backup_count = 3 + if getattr(self, 'config', None) is not None and self.config.has_section('Logging'): + log_file = self.config.get('Logging', 'log_file', fallback='').strip() + try: + log_max_bytes = self.config.getint('Logging', 'log_max_bytes', fallback=log_max_bytes) + except (configparser.Error, ValueError, TypeError): + pass + try: + log_backup_count = self.config.getint('Logging', 'log_backup_count', fallback=log_backup_count) + except (configparser.Error, ValueError, TypeError): + pass # Get or create logger (don't use basicConfig as it may conflict with existing logging) self.logger = logging.getLogger('modern_web_viewer') @@ -365,29 +381,47 @@ class BotDataViewer: # Remove existing handlers to avoid duplicates self.logger.handlers.clear() - # Create rotating file handler (max 5MB per file, keep 3 backups) - file_handler = RotatingFileHandler( - 'logs/web_viewer_modern.log', - maxBytes=5 * 1024 * 1024, # 5 MB - backupCount=3, - encoding='utf-8' - ) - file_handler.setLevel(logging.DEBUG) - file_formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s') - file_handler.setFormatter(file_formatter) - self.logger.addHandler(file_handler) + formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s') - # Create console handler + # Console handler (captured by journald under systemd) console_handler = logging.StreamHandler() console_handler.setLevel(logging.INFO) - console_formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s') - console_handler.setFormatter(console_formatter) + console_handler.setFormatter(formatter) self.logger.addHandler(console_handler) # Prevent propagation to root logger to avoid duplicate messages self.logger.propagate = False - self.logger.info("Web viewer logging initialized with rotation (5MB max, 3 backups)") + if not log_file: + self.logger.info("No log file specified, using console/journal logging only") + return + + # Place viewer log beside the bot log (same directory as log_file) + bot_log_path = Path(resolve_path(log_file, self._config_base)) + viewer_log_path = bot_log_path.parent / 'web_viewer.log' + try: + viewer_log_path.parent.mkdir(parents=True, exist_ok=True) + file_handler = RotatingFileHandler( + str(viewer_log_path), + maxBytes=log_max_bytes, + backupCount=log_backup_count, + encoding='utf-8', + ) + file_handler.setLevel(logging.DEBUG) + file_handler.setFormatter(formatter) + self.logger.addHandler(file_handler) + self.logger.info( + "Web viewer logging initialized (file=%s, max=%s bytes, backups=%s)", + viewer_log_path, + log_max_bytes, + log_backup_count, + ) + except (OSError, PermissionError) as e: + self.logger.warning( + "Could not open web viewer log file %s: %s. Using console/journal only.", + viewer_log_path, + e, + ) def _load_config(self, config_path): """Load configuration from file""" @@ -882,7 +916,7 @@ class BotDataViewer: bot_name = '' name_managed = False if self.config: - auto_manage = self.config.get('Bot', 'auto_manage_contacts', fallback='false').lower() + auto_manage = self.config.get('Bot', 'auto_manage_contacts', fallback='device').lower() bot_name = (self.config.get('Bot', 'bot_name', fallback='') or '').strip() try: auto_update_name = self.config.getboolean('Bot', 'auto_update_device_name', fallback=True) diff --git a/modules/web_viewer/integration.py b/modules/web_viewer/integration.py index a7c2e99..8c13f40 100644 --- a/modules/web_viewer/integration.py +++ b/modules/web_viewer/integration.py @@ -871,10 +871,30 @@ class WebViewerIntegration: except Exception as e: self.logger.error(f"Error stopping web viewer: {e}") + def _viewer_log_dir(self, config_path: str) -> Path | None: + """Return directory for viewer stdout/stderr files, or None for journal-only. + + Mirrors the main bot: empty [Logging] log_file means console/journal only + (no files under the install tree). When set, use that file's parent + (e.g. /var/log/meshcore-bot). + """ + log_file = '' + try: + if self.bot.config.has_section('Logging'): + log_file = self.bot.config.get('Logging', 'log_file', fallback='').strip() + except Exception: + log_file = '' + if not log_file: + return None + config_base = Path(config_path).parent.resolve() + return Path(resolve_path(log_file, config_base)).parent + def _run_viewer(self): """Run the web viewer in a separate process""" stdout_file = None stderr_file = None + stdout_path = None + stderr_path = None try: # Get the path to the web viewer script @@ -895,28 +915,35 @@ class WebViewerIntegration: if self.debug: cmd.append("--debug") - # Ensure logs directory exists - os.makedirs('logs', exist_ok=True) + # Avoid PIPE deadlock: either inherit stdio (journald under systemd) + # or redirect to files beside the configured log_file. + log_dir = self._viewer_log_dir(config_path) + if log_dir is None: + # Inherit parent stdout/stderr so systemd/journald captures them. + # Do not create ./logs under the (often read-only) install tree. + popen_stdout = None + popen_stderr = None + else: + log_dir.mkdir(parents=True, exist_ok=True) + stdout_path = log_dir / 'web_viewer_stdout.log' + stderr_path = log_dir / 'web_viewer_stderr.log' + # Open in write mode to prevent buffer blocking. Using 'w' + # (overwrite) instead of 'a' (append) since: + # - The web viewer already has proper logging to web_viewer.log + # - stdout/stderr are mainly for immediate debugging + # - Prevents unbounded log file growth + stdout_file = open(stdout_path, 'w') + stderr_file = open(stderr_path, 'w') + self._viewer_stdout_file = stdout_file + self._viewer_stderr_file = stderr_file + popen_stdout = stdout_file + popen_stderr = stderr_file - # Open log files in write mode to prevent buffer blocking - # This fixes the issue where subprocess.PIPE buffers (~64KB) fill up - # after ~5 minutes and cause the subprocess to hang. - # Using 'w' mode (overwrite) instead of 'a' (append) since: - # - The web viewer already has proper logging to web_viewer_modern.log - # - stdout/stderr are mainly for immediate debugging - # - Prevents unbounded log file growth - stdout_file = open('logs/web_viewer_stdout.log', 'w') - stderr_file = open('logs/web_viewer_stderr.log', 'w') - - # Store file handles for proper cleanup - self._viewer_stdout_file = stdout_file - self._viewer_stderr_file = stderr_file - - # Start the viewer process with log file redirection + # Start the viewer process self.viewer_process = subprocess.Popen( cmd, - stdout=stdout_file, - stderr=stderr_file, + stdout=popen_stdout, + stderr=popen_stderr, text=True ) @@ -926,32 +953,37 @@ class WebViewerIntegration: # Check if it started successfully if self.viewer_process and self.viewer_process.poll() is not None: # Process failed immediately - read from log files for error reporting - stdout_file.flush() - stderr_file.flush() - - # Read last few lines from stderr for error reporting - try: - stderr_file.close() - with open('logs/web_viewer_stderr.log') as f: - stderr_lines = f.readlines()[-20:] # Last 20 lines - stderr = ''.join(stderr_lines) - except Exception: - stderr = "Could not read stderr log" - - # Read last few lines from stdout for error reporting - try: - stdout_file.close() - with open('logs/web_viewer_stdout.log') as f: - stdout_lines = f.readlines()[-20:] # Last 20 lines - stdout = ''.join(stdout_lines) - except Exception: - stdout = "Could not read stdout log" + stderr = "" + stdout = "" + if stderr_file is not None and stderr_path is not None: + with suppress(Exception): + stderr_file.flush() + stderr_file.close() + try: + with open(stderr_path) as f: + stderr = ''.join(f.readlines()[-20:]) + except Exception: + stderr = "Could not read stderr log" + if stdout_file is not None and stdout_path is not None: + with suppress(Exception): + stdout_file.flush() + stdout_file.close() + try: + with open(stdout_path) as f: + stdout = ''.join(f.readlines()[-20:]) + except Exception: + stdout = "Could not read stdout log" self.logger.error(f"Web viewer failed to start. Return code: {self.viewer_process.returncode}") if stderr and stderr.strip(): self.logger.error(f"Web viewer startup error: {stderr}") if stdout and stdout.strip(): self.logger.error(f"Web viewer startup output: {stdout}") + elif log_dir is None: + self.logger.error( + "Web viewer startup output is in the service journal " + "(empty [Logging] log_file; stdio inherited)" + ) self.viewer_process = None self._viewer_stdout_file = None @@ -986,25 +1018,28 @@ class WebViewerIntegration: # Process exited - read from log files for error reporting if needed if self.viewer_process and self.viewer_process.returncode != 0: - stdout_file.flush() - stderr_file.flush() - - # Read last few lines from stderr for error reporting - try: - stderr_file.close() - with open('logs/web_viewer_stderr.log') as f: - stderr_lines = f.readlines()[-20:] # Last 20 lines - stderr = ''.join(stderr_lines) - except Exception: - stderr = "Could not read stderr log" - - # Close stdout file as well - with suppress(Exception): - stdout_file.close() + stderr = "" + if stderr_file is not None and stderr_path is not None: + with suppress(Exception): + stderr_file.flush() + stderr_file.close() + try: + with open(stderr_path) as f: + stderr = ''.join(f.readlines()[-20:]) + except Exception: + stderr = "Could not read stderr log" + if stdout_file is not None: + with suppress(Exception): + stdout_file.close() self.logger.error(f"Web viewer process exited with code {self.viewer_process.returncode}") if stderr and stderr.strip(): self.logger.error(f"Web viewer stderr: {stderr}") + elif log_dir is None: + self.logger.error( + "Web viewer stderr is in the service journal " + "(empty [Logging] log_file; stdio inherited)" + ) self._viewer_stdout_file = None self._viewer_stderr_file = None diff --git a/tests/test_web_viewer.py b/tests/test_web_viewer.py index f3a2baa..0187838 100644 --- a/tests/test_web_viewer.py +++ b/tests/test_web_viewer.py @@ -3552,6 +3552,58 @@ class TestDbPathResolutionFromConfigDir: ) +class TestWebViewerLoggingRespectsLogFile: + """Web viewer file logging follows [Logging] log_file like the main bot.""" + + def _write_config(self, config_dir: Path, log_file: str | None) -> str: + cfg = configparser.ConfigParser() + cfg["Connection"] = {"connection_type": "serial", "serial_port": "/dev/ttyUSB0"} + cfg["Bot"] = {"bot_name": "TestBot", "db_path": "bot.db", "prefix_bytes": "1"} + cfg["Channels"] = {"monitor_channels": "general"} + cfg["Path_Command"] = {"max_hops": "5", "timeout": "30"} + if log_file is not None: + cfg["Logging"] = {"log_file": log_file} + config_path = str(config_dir / "config.ini") + with open(config_path, "w") as fh: + cfg.write(fh) + return config_path + + def _make_viewer(self, config_path: str) -> BotDataViewer: + with ( + patch.object(BotDataViewer, "_start_database_polling", lambda self: None), + patch.object(BotDataViewer, "_start_log_tailing", lambda self: None), + patch.object(BotDataViewer, "_start_cleanup_scheduler", lambda self: None), + ): + return BotDataViewer(config_path=config_path) + + def test_empty_log_file_is_console_only(self, tmp_path: Path) -> None: + from logging.handlers import RotatingFileHandler + + config_dir = tmp_path / "cfg" + config_dir.mkdir() + config_path = self._write_config(config_dir, log_file="") + v = self._make_viewer(config_path) + assert not any(isinstance(h, RotatingFileHandler) for h in v.logger.handlers) + assert not (config_dir / "logs").exists() + assert not (config_dir / "web_viewer.log").exists() + assert not (tmp_path / "logs").exists() + + def test_log_file_writes_beside_bot_log(self, tmp_path: Path) -> None: + from logging.handlers import RotatingFileHandler + + config_dir = tmp_path / "cfg" + log_dir = tmp_path / "varlog" + config_dir.mkdir() + log_dir.mkdir() + bot_log = log_dir / "meshcore_bot.log" + config_path = self._write_config(config_dir, log_file=str(bot_log)) + v = self._make_viewer(config_path) + assert any(isinstance(h, RotatingFileHandler) for h in v.logger.handlers) + assert (log_dir / "web_viewer.log").exists() + assert not (config_dir / "logs").exists() + assert not (tmp_path / "logs").exists() + + class TestRadioDebugConfig: """Tests for GET/POST /api/config/radio-debug endpoints.""" diff --git a/tests/test_web_viewer_integration.py b/tests/test_web_viewer_integration.py index 977f307..b234944 100644 --- a/tests/test_web_viewer_integration.py +++ b/tests/test_web_viewer_integration.py @@ -598,3 +598,44 @@ class TestIntegrationTimeoutConfig: assert wvi.viewer_stop_force_timeout_sec == 4 assert wvi.port_cleanup_lsof_timeout_sec == 8 assert wvi.port_cleanup_kill_timeout_sec == 1 + + +class TestViewerLogDir: + """Web viewer subprocess log dir follows [Logging] log_file.""" + + def test_empty_log_file_returns_none(self): + from modules.web_viewer.integration import WebViewerIntegration + + bot = _make_bot() + bot.config.add_section("Logging") + bot.config.set("Logging", "log_file", "") + with patch("modules.web_viewer.integration.BotIntegration._init_http_session"), \ + patch("modules.web_viewer.integration.BotIntegration._init_packet_stream_table"), \ + patch("modules.web_viewer.integration.BotIntegration._start_drain_thread"): + wvi = WebViewerIntegration(bot) + assert wvi._viewer_log_dir("/tmp/config.ini") is None + + def test_missing_logging_section_returns_none(self): + from modules.web_viewer.integration import WebViewerIntegration + + bot = _make_bot() + assert not bot.config.has_section("Logging") + with patch("modules.web_viewer.integration.BotIntegration._init_http_session"), \ + patch("modules.web_viewer.integration.BotIntegration._init_packet_stream_table"), \ + patch("modules.web_viewer.integration.BotIntegration._start_drain_thread"): + wvi = WebViewerIntegration(bot) + assert wvi._viewer_log_dir("/tmp/config.ini") is None + + def test_absolute_log_file_uses_parent(self, tmp_path: Path): + from modules.web_viewer.integration import WebViewerIntegration + + bot = _make_bot() + log_dir = tmp_path / "var" / "log" / "meshcore-bot" + log_dir.mkdir(parents=True) + bot.config.add_section("Logging") + bot.config.set("Logging", "log_file", str(log_dir / "meshcore_bot.log")) + with patch("modules.web_viewer.integration.BotIntegration._init_http_session"), \ + patch("modules.web_viewer.integration.BotIntegration._init_packet_stream_table"), \ + patch("modules.web_viewer.integration.BotIntegration._start_drain_thread"): + wvi = WebViewerIntegration(bot) + assert wvi._viewer_log_dir(str(tmp_path / "config.ini")) == log_dir.resolve()