From a1480a5c1b29dd5d9bfbdc2e48899fdbd77f3c49 Mon Sep 17 00:00:00 2001 From: Ivan Date: Sun, 30 Nov 2025 15:55:30 -0600 Subject: [PATCH] Improve logging and error handling across modules - Added logging functionality to app.py and rns.py for better error tracking. - Improved exception handling in RNSManager methods to log specific failures. - Refactored code in various modules to ensure consistent logging practices. - Updated UI components to handle exceptions with user feedback. - Cleaned up formatting in several files for better readability. --- ren_browser/app.py | 4 +++- ren_browser/renderer/micron.py | 14 ++++++------ ren_browser/rns.py | 39 +++++++++++++++++++--------------- ren_browser/storage/storage.py | 3 ++- ren_browser/tabs/tabs.py | 3 ++- ren_browser/ui/settings.py | 23 +++++++++++++------- ren_browser/ui/ui.py | 3 ++- tests/unit/test_storage.py | 28 ++++++++++++------------ tests/unit/test_ui.py | 20 ++++++++++++----- 9 files changed, 83 insertions(+), 54 deletions(-) diff --git a/ren_browser/app.py b/ren_browser/app.py index 078a276..58c9b20 100644 --- a/ren_browser/app.py +++ b/ren_browser/app.py @@ -5,6 +5,7 @@ Ren Browser, a browser for the Reticulum Network built with Flet. """ import argparse +import logging import os from pathlib import Path @@ -19,6 +20,7 @@ from ren_browser.ui.ui import build_ui RENDERER = "plaintext" RNS_CONFIG_DIR = None RNS_INSTANCE = None +logger = logging.getLogger(__name__) async def main(page: Page): @@ -63,7 +65,7 @@ async def main(page: Page): ren_browser.logs.setup_rns_logging() except Exception: - pass + logger.exception("Unable to configure RNS logging") success = rns.initialize_reticulum(config_override) if not success: diff --git a/ren_browser/renderer/micron.py b/ren_browser/renderer/micron.py index 9b1f62f..eccf2c3 100644 --- a/ren_browser/renderer/micron.py +++ b/ren_browser/renderer/micron.py @@ -45,7 +45,7 @@ def parse_micron_line(line: str) -> list: "underline": underline, "color": color, "bgcolor": bgcolor, - } + }, ) current_text = "" @@ -95,7 +95,7 @@ def parse_micron_line(line: str) -> list: "underline": underline, "color": color, "bgcolor": bgcolor, - } + }, ) return spans @@ -196,8 +196,9 @@ def _render_micron_internal(content: str, on_link_click=None) -> ft.Control: before = line[last_end : link_match.start()] if before: before_spans = parse_micron_line(before) - for span in before_spans: - row_controls.append(create_text_span(span)) + row_controls.extend( + create_text_span(span) for span in before_spans + ) label = link_match.group(1) url = link_match.group(2) @@ -225,8 +226,9 @@ def _render_micron_internal(content: str, on_link_click=None) -> ft.Control: after = line[last_end:] if after: after_spans = parse_micron_line(after) - for span in after_spans: - row_controls.append(create_text_span(span)) + row_controls.extend( + create_text_span(span) for span in after_spans + ) if row_controls: controls.append( diff --git a/ren_browser/rns.py b/ren_browser/rns.py index 43b0d05..89948ed 100644 --- a/ren_browser/rns.py +++ b/ren_browser/rns.py @@ -2,21 +2,23 @@ from __future__ import annotations +import logging import os import tempfile from pathlib import Path -from typing import Optional import RNS +logger = logging.getLogger(__name__) + class RNSManager: """Manage Reticulum lifecycle and configuration.""" def __init__(self): self.reticulum = None - self.config_path: Optional[str] = None - self.last_error: Optional[str] = None + self.config_path: str | None = None + self.last_error: str | None = None def _is_android(self) -> bool: vendor = getattr(RNS, "vendor", None) @@ -47,9 +49,8 @@ class RNSManager: return Path(tempfile.gettempdir()) def _default_config_root(self) -> Path: - override = ( - os.environ.get("REN_BROWSER_RNS_DIR") - or os.environ.get("REN_RETICULUM_CONFIG_DIR") + override = os.environ.get("REN_BROWSER_RNS_DIR") or os.environ.get( + "REN_RETICULUM_CONFIG_DIR", ) if override: return Path(override).expanduser() @@ -57,8 +58,10 @@ class RNSManager: return self._android_storage_root() / "ren_browser" / "reticulum" return Path.home() / ".reticulum" - def _resolve_config_dir(self, preferred: Optional[str | Path]) -> Path: - target = Path(preferred).expanduser() if preferred else self._default_config_root() + def _resolve_config_dir(self, preferred: str | Path | None) -> Path: + target = ( + Path(preferred).expanduser() if preferred else self._default_config_root() + ) allow_fallback = preferred is None try: @@ -128,7 +131,7 @@ class RNSManager: config_file.write_text(base_content, encoding="utf-8") os.chmod(config_file, 0o600) except Exception: - pass + logger.exception("Failed to seed default config at %s", config_file) def _ensure_default_tcp_interfaces(self) -> None: if not self.config_path: @@ -152,7 +155,10 @@ class RNSManager: cfg.write("\n") cfg.write("\n" + snippet + "\n") except Exception: - pass + logger.exception( + "Failed to append default TCP interfaces to %s", + config_file, + ) def _get_or_create_config_dir(self) -> Path: if self.config_path: @@ -162,7 +168,7 @@ class RNSManager: self.config_path = str(resolved) return resolved - def initialize(self, config_dir: Optional[str] = None) -> bool: + def initialize(self, config_dir: str | None = None) -> bool: """Initialize the Reticulum instance.""" self.last_error = None try: @@ -225,7 +231,7 @@ class RNSManager: self.last_error = str(exc) return False - def get_config_path(self) -> Optional[str]: + def get_config_path(self) -> str | None: """Return the directory holding the active Reticulum config.""" if self.config_path: return self.config_path @@ -240,7 +246,7 @@ class RNSManager: """Return the current Reticulum instance, if any.""" return self.reticulum - def get_last_error(self) -> Optional[str]: + def get_last_error(self) -> str | None: """Return the last recorded error string.""" return self.last_error @@ -248,7 +254,7 @@ class RNSManager: rns_manager = RNSManager() -def initialize_reticulum(config_dir: Optional[str] = None) -> bool: +def initialize_reticulum(config_dir: str | None = None) -> bool: """Initialize Reticulum using the shared manager.""" return rns_manager.initialize(config_dir) @@ -263,7 +269,7 @@ def get_reticulum_instance(): return rns_manager.get_reticulum_instance() -def get_config_path() -> Optional[str]: +def get_config_path() -> str | None: """Expose the active configuration directory.""" return rns_manager.get_config_path() @@ -278,7 +284,6 @@ def write_config_file(content: str) -> bool: return rns_manager.write_config_file(content) -def get_last_error() -> Optional[str]: +def get_last_error() -> str | None: """Return the last recorded Reticulum error.""" return rns_manager.get_last_error() - diff --git a/ren_browser/storage/storage.py b/ren_browser/storage/storage.py index 11932db..45be111 100644 --- a/ren_browser/storage/storage.py +++ b/ren_browser/storage/storage.py @@ -259,7 +259,8 @@ class StorageManager: if self.page and hasattr(self.page, "client_storage"): self.page.client_storage.set( - "ren_browser_settings", json.dumps(settings) + "ren_browser_settings", + json.dumps(settings), ) return True diff --git a/ren_browser/tabs/tabs.py b/ren_browser/tabs/tabs.py index 091da5f..fc8066b 100644 --- a/ren_browser/tabs/tabs.py +++ b/ren_browser/tabs/tabs.py @@ -61,7 +61,8 @@ class TabsManager: default_content = ( render_micron( - "Welcome to Ren Browser", on_link_click=handle_link_click_home + "Welcome to Ren Browser", + on_link_click=handle_link_click_home, ) if app_module.RENDERER == "micron" else render_plaintext("Welcome to Ren Browser") diff --git a/ren_browser/ui/settings.py b/ren_browser/ui/settings.py index 667939b..ed06918 100644 --- a/ren_browser/ui/settings.py +++ b/ren_browser/ui/settings.py @@ -4,6 +4,7 @@ from __future__ import annotations from datetime import datetime from pathlib import Path +import logging import flet as ft import RNS @@ -13,6 +14,7 @@ from ren_browser.storage.storage import get_storage_manager BUTTON_BG = "#0B3D91" BUTTON_BG_HOVER = "#082C6C" +logger = logging.getLogger(__name__) def _blue_button_style() -> ft.ButtonStyle: @@ -52,14 +54,14 @@ def _get_interface_statuses(): for interface in interfaces: if interface is None: continue - if ( - interface.__class__.__name__ == "LocalClientInterface" - and getattr(interface, "is_connected_to_shared_instance", False) + if interface.__class__.__name__ == "LocalClientInterface" and getattr( + interface, "is_connected_to_shared_instance", False, ): continue statuses.append( { - "name": getattr(interface, "name", None) or interface.__class__.__name__, + "name": getattr(interface, "name", None) + or interface.__class__.__name__, "online": bool(getattr(interface, "online", False)), "type": interface.__class__.__name__, "bitrate": getattr(interface, "bitrate", None), @@ -194,7 +196,9 @@ def _build_storage_field(storage): def refresh(): info = storage.get_storage_info() - storage_field.value = "\n".join(f"{key}: {value}" for key, value in info.items()) + storage_field.value = "\n".join( + f"{key}: {value}" for key, value in info.items() + ) refresh() return storage_field, refresh @@ -245,8 +249,12 @@ def open_settings_tab(page: ft.Page, tab_manager): try: color_preview.bgcolor = page_bgcolor_field.value page.update() - except Exception: - pass + except Exception as exc: + logger.warning( + "Ignoring invalid background color '%s': %s", + page_bgcolor_field.value, + exc, + ) page_bgcolor_field.on_change = on_bgcolor_change @@ -484,4 +492,3 @@ def open_settings_tab(page: ft.Page, tab_manager): idx = len(tab_manager.manager.tabs) - 1 tab_manager.select_tab(idx) page.update() - diff --git a/ren_browser/ui/ui.py b/ren_browser/ui/ui.py index e2b6ef9..25e2349 100644 --- a/ren_browser/ui/ui.py +++ b/ren_browser/ui/ui.py @@ -85,7 +85,8 @@ def build_ui(page: Page): if req.page_path.endswith(".mu"): new_control = render_micron( - result, on_link_click=handle_link_click + result, + on_link_click=handle_link_click, ) else: new_control = render_plaintext(result) diff --git a/tests/unit/test_storage.py b/tests/unit/test_storage.py index e91df59..2d9df7e 100644 --- a/tests/unit/test_storage.py +++ b/tests/unit/test_storage.py @@ -80,14 +80,14 @@ class TestStorageManager: clear=True, ), patch("pathlib.Path.mkdir"), - ): - with patch( + patch( "ren_browser.storage.storage.StorageManager._ensure_storage_directory", - ): - storage = StorageManager() - storage._storage_dir = storage._get_storage_directory() - expected_dir = Path("/data/ren_browser") - assert storage._storage_dir == expected_dir + ), + ): + storage = StorageManager() + storage._storage_dir = storage._get_storage_directory() + expected_dir = Path("/data/ren_browser") + assert storage._storage_dir == expected_dir def test_get_storage_directory_android_with_external_storage(self): """Test storage directory detection for Android with EXTERNAL_STORAGE.""" @@ -99,14 +99,14 @@ class TestStorageManager: clear=True, ), patch("pathlib.Path.mkdir"), - ): - with patch( + patch( "ren_browser.storage.storage.StorageManager._ensure_storage_directory", - ): - storage = StorageManager() - storage._storage_dir = storage._get_storage_directory() - expected_dir = Path("/storage/emulated/0/ren_browser") - assert storage._storage_dir == expected_dir + ), + ): + storage = StorageManager() + storage._storage_dir = storage._get_storage_directory() + expected_dir = Path("/storage/emulated/0/ren_browser") + assert storage._storage_dir == expected_dir def test_get_storage_directory_android_fallback(self): """Test storage directory detection for Android with fallback.""" diff --git a/tests/unit/test_ui.py b/tests/unit/test_ui.py index 28cbd4c..26e0c21 100644 --- a/tests/unit/test_ui.py +++ b/tests/unit/test_ui.py @@ -107,7 +107,9 @@ class TestOpenSettingsTab: "ren_browser.ui.settings.get_storage_manager", return_value=mock_storage_manager, ), - patch("ren_browser.ui.settings.rns.get_config_path", return_value="/tmp/rns"), + patch( + "ren_browser.ui.settings.rns.get_config_path", return_value="/tmp/rns", + ), patch("pathlib.Path.read_text", return_value="config content"), ): open_settings_tab(mock_page, mock_tab_manager) @@ -130,7 +132,9 @@ class TestOpenSettingsTab: "ren_browser.ui.settings.get_storage_manager", return_value=mock_storage_manager, ), - patch("ren_browser.ui.settings.rns.get_config_path", return_value="/tmp/rns"), + patch( + "ren_browser.ui.settings.rns.get_config_path", return_value="/tmp/rns", + ), patch("pathlib.Path.read_text", side_effect=Exception("File not found")), ): open_settings_tab(mock_page, mock_tab_manager) @@ -155,7 +159,9 @@ class TestOpenSettingsTab: "ren_browser.ui.settings.get_storage_manager", return_value=mock_storage_manager, ), - patch("ren_browser.ui.settings.rns.get_config_path", return_value="/tmp/rns"), + patch( + "ren_browser.ui.settings.rns.get_config_path", return_value="/tmp/rns", + ), patch("pathlib.Path.read_text", return_value="config"), patch("pathlib.Path.write_text") as mock_write, ): @@ -194,7 +200,9 @@ class TestOpenSettingsTab: "ren_browser.ui.settings.get_storage_manager", return_value=mock_storage_manager, ), - patch("ren_browser.ui.settings.rns.get_config_path", return_value="/tmp/rns"), + patch( + "ren_browser.ui.settings.rns.get_config_path", return_value="/tmp/rns", + ), patch("pathlib.Path.read_text", return_value="config"), patch("pathlib.Path.write_text", side_effect=Exception("disk full")), ): @@ -229,7 +237,9 @@ class TestOpenSettingsTab: "ren_browser.ui.settings.get_storage_manager", return_value=mock_storage_manager, ), - patch("ren_browser.ui.settings.rns.get_config_path", return_value="/tmp/rns"), + patch( + "ren_browser.ui.settings.rns.get_config_path", return_value="/tmp/rns", + ), patch("pathlib.Path.read_text", return_value="config"), ): open_settings_tab(mock_page, mock_tab_manager)