From 8b64d434d63ace2b6200bf7203f3c6ed4a15467e Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Sun, 15 Feb 2026 18:20:05 -0800 Subject: [PATCH 1/3] Remove Installer Registration --- synodic_client/updater.py | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/synodic_client/updater.py b/synodic_client/updater.py index a24d6ea..b2efc1a 100644 --- a/synodic_client/updater.py +++ b/synodic_client/updater.py @@ -17,7 +17,7 @@ import velopack from packaging.version import Version -from synodic_client.protocol import register_protocol, remove_protocol +from synodic_client.protocol import remove_protocol logger = logging.getLogger(__name__) @@ -308,17 +308,6 @@ def _get_velopack_manager(self) -> Any: return None -def _on_after_install(version: str) -> None: # noqa: ARG001 - """Velopack hook: called after the app is installed. - - Registers the ``synodic://`` URI protocol handler. - - Args: - version: The installed version string (provided by Velopack). - """ - register_protocol(sys.executable) - - def _on_before_uninstall(version: str) -> None: # noqa: ARG001 """Velopack hook: called before the app is uninstalled. @@ -327,7 +316,10 @@ def _on_before_uninstall(version: str) -> None: # noqa: ARG001 Args: version: The current version string (provided by Velopack). """ - remove_protocol() + try: + remove_protocol() + except Exception: + logger.debug('Protocol removal failed during uninstall hook', exc_info=True) def initialize_velopack() -> None: @@ -337,11 +329,11 @@ def initialize_velopack() -> None: before any UI is shown. Velopack may need to perform cleanup or apply pending updates. - On Windows, install/uninstall hooks register the ``synodic://`` URI protocol. + On Windows, the uninstall hook removes the ``synodic://`` URI protocol. + Protocol registration happens on every app launch (see ``qt.application``). """ try: app = velopack.App() - app.on_after_install_fast_callback(_on_after_install) app.on_before_uninstall_fast_callback(_on_before_uninstall) app.run() logger.debug('Velopack initialized') From ec9da85ee76610c20b730ecb674a7495eed324d1 Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Sun, 15 Feb 2026 18:33:23 -0800 Subject: [PATCH 2/3] Better Velopack Exception Handling --- synodic_client/updater.py | 50 +++++++++++++++++++++------ tests/unit/qt/conftest.py | 2 +- tests/unit/test_updater.py | 71 +++++++++++++++++++++++++++++++++++--- 3 files changed, 107 insertions(+), 16 deletions(-) diff --git a/synodic_client/updater.py b/synodic_client/updater.py index b2efc1a..c1bb0ac 100644 --- a/synodic_client/updater.py +++ b/synodic_client/updater.py @@ -102,6 +102,7 @@ def __init__(self, current_version: Version, config: UpdateConfig | None = None) self._state = UpdateState.NO_UPDATE self._update_info: UpdateInfo | None = None self._velopack_manager: Any = None + self._velopack_not_installed: bool = False @property def state(self) -> UpdateState: @@ -110,12 +111,17 @@ def state(self) -> UpdateState: @property def is_installed(self) -> bool: - """Check if running as a Velopack-installed application.""" + """Check if running as a Velopack-installed application. + + Delegates to ``_get_velopack_manager`` which creates the + ``UpdateManager``. The SDK constructor raises ``RuntimeError`` + with *"not properly installed"* when no Velopack manifest is + found; that specific error is treated as "not installed" while + all other failures propagate. + """ try: - manager = self._get_velopack_manager() - # If we can get the manager and it has a version, we're installed - return manager is not None - except Exception: + return self._get_velopack_manager() is not None + except RuntimeError: return False def check_for_update(self) -> UpdateInfo: @@ -284,28 +290,50 @@ def apply_update_on_exit(self, restart: bool = True, restart_args: list[str] | N self._update_info.error = str(e) raise + _NOT_INSTALLED_SENTINEL = 'not properly installed' + """Substring the Velopack SDK includes in its ``RuntimeError`` when + the application was not installed via Velopack.""" + def _get_velopack_manager(self) -> Any: """Get or create the Velopack UpdateManager. Returns: - UpdateManager instance, or None if not installed via Velopack + UpdateManager instance, or ``None`` when the application is + not running from a Velopack installation. + + Raises: + RuntimeError: If the ``UpdateManager`` could not be created + for a reason *other* than the app not being installed + (e.g. a genuine SDK or configuration problem). """ if self._velopack_manager is not None: return self._velopack_manager + if self._velopack_not_installed: + return None + try: - options = velopack.UpdateOptions() - options.allow_version_downgrade = False - options.explicit_channel = self._config.channel_name + options = velopack.UpdateOptions( + AllowVersionDowngrade=False, + MaximumDeltasBeforeFallback=0, + ) + options.ExplicitChannel = self._config.channel_name self._velopack_manager = velopack.UpdateManager( self._config.repo_url, options, ) return self._velopack_manager + except RuntimeError as e: + if self._NOT_INSTALLED_SENTINEL in str(e).lower(): + logger.debug('Not a Velopack install: %s', e) + self._velopack_not_installed = True + return None + logger.warning('Velopack manager creation failed: %s', e) + raise except Exception as e: - logger.debug('Failed to create Velopack manager: %s', e) - return None + logger.warning('Velopack manager creation failed: %s', e) + raise RuntimeError(f'Failed to create Velopack UpdateManager: {e}') from e def _on_before_uninstall(version: str) -> None: # noqa: ARG001 diff --git a/tests/unit/qt/conftest.py b/tests/unit/qt/conftest.py index 6163ef3..77afb03 100644 --- a/tests/unit/qt/conftest.py +++ b/tests/unit/qt/conftest.py @@ -7,4 +7,4 @@ import pytest -pytest.importorskip("PySide6.QtWidgets", reason="PySide6 requires system Qt libraries") +pytest.importorskip('PySide6.QtWidgets', reason='PySide6 requires system Qt libraries') diff --git a/tests/unit/test_updater.py b/tests/unit/test_updater.py index 76df689..44d437c 100644 --- a/tests/unit/test_updater.py +++ b/tests/unit/test_updater.py @@ -179,7 +179,6 @@ def test_custom_config(updater_with_config: Updater) -> None: @staticmethod def test_is_installed_not_velopack(updater: Updater) -> None: """Verify is_installed returns False in test environment.""" - # Tests run in non-Velopack environment with patch.object(updater, '_get_velopack_manager', return_value=None): assert updater.is_installed is False @@ -191,9 +190,9 @@ def test_is_installed_with_velopack(updater: Updater) -> None: assert updater.is_installed is True @staticmethod - def test_is_installed_handles_exception(updater: Updater) -> None: - """Verify is_installed returns False when exception occurs.""" - with patch.object(updater, '_get_velopack_manager', side_effect=Exception('Test')): + def test_is_installed_handles_runtime_error(updater: Updater) -> None: + """Verify is_installed returns False when RuntimeError is raised.""" + with patch.object(updater, '_get_velopack_manager', side_effect=RuntimeError('fail')): assert updater.is_installed is False @@ -454,3 +453,67 @@ def test_initialize_handles_exception() -> None: with patch('synodic_client.updater.velopack.App', return_value=mock_app): # Should not raise initialize_velopack() + + +class TestGetVelopackManager: + """Tests for _get_velopack_manager install detection via the SDK.""" + + _PATCH_OPTIONS = patch('synodic_client.updater.velopack.UpdateOptions') + + @staticmethod + def test_not_installed_returns_none(updater: Updater) -> None: + """Verify manager returns None when SDK says 'not properly installed'.""" + error = RuntimeError('This application is not properly installed: Could not auto-locate app manifest') + with ( + TestGetVelopackManager._PATCH_OPTIONS, + patch('synodic_client.updater.velopack.UpdateManager', side_effect=error), + ): + assert updater._get_velopack_manager() is None + + @staticmethod + def test_not_installed_sentinel_cached(updater: Updater) -> None: + """Verify that once detected as not-installed, the SDK is not called again.""" + error = RuntimeError('This application is not properly installed') + with ( + TestGetVelopackManager._PATCH_OPTIONS, + patch('synodic_client.updater.velopack.UpdateManager', side_effect=error) as mock_cls, + ): + updater._get_velopack_manager() + updater._get_velopack_manager() + mock_cls.assert_called_once() + + @staticmethod + def test_real_error_propagates(updater: Updater) -> None: + """Verify non-install RuntimeErrors propagate instead of returning None.""" + error = RuntimeError('Some other SDK failure') + with ( + TestGetVelopackManager._PATCH_OPTIONS, + patch('synodic_client.updater.velopack.UpdateManager', side_effect=error), + pytest.raises(RuntimeError, match='Some other SDK failure'), + ): + updater._get_velopack_manager() + + @staticmethod + def test_non_runtime_error_propagates(updater: Updater) -> None: + """Verify non-RuntimeError exceptions are wrapped and propagated.""" + error = ValueError('bad config') + with ( + TestGetVelopackManager._PATCH_OPTIONS, + patch('synodic_client.updater.velopack.UpdateManager', side_effect=error), + pytest.raises(RuntimeError, match='Failed to create Velopack UpdateManager'), + ): + updater._get_velopack_manager() + + @staticmethod + def test_success_caches_manager(updater: Updater) -> None: + """Verify successful manager creation is cached.""" + mock_manager = MagicMock() + with ( + TestGetVelopackManager._PATCH_OPTIONS, + patch('synodic_client.updater.velopack.UpdateManager', return_value=mock_manager) as mock_cls, + ): + result1 = updater._get_velopack_manager() + result2 = updater._get_velopack_manager() + assert result1 is mock_manager + assert result2 is mock_manager + mock_cls.assert_called_once() From 6c1a429df5342acd1e85b0359d045ee99e4d7ce8 Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Sun, 15 Feb 2026 18:44:47 -0800 Subject: [PATCH 3/3] Update Logging/Velopack Order --- synodic_client/application/qt.py | 10 ++++++---- synodic_client/updater.py | 25 +++++++++++++++++++++---- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/synodic_client/application/qt.py b/synodic_client/application/qt.py index 79b25ea..df47b1e 100644 --- a/synodic_client/application/qt.py +++ b/synodic_client/application/qt.py @@ -122,6 +122,12 @@ def application(*, uri: str | None = None, dev_mode: bool = False) -> None: # Activate dev-mode namespacing before anything reads config paths. set_dev_mode(dev_mode) + # Configure logging before Velopack so install/uninstall hooks and + # first-run diagnostics are captured in the log file. + configure_logging() + logger = logging.getLogger('synodic_client') + _install_exception_hook(logger) + if not dev_mode: # Initialize Velopack early, before any UI. # Console window suppression for subprocesses is handled by the @@ -129,10 +135,6 @@ def application(*, uri: str | None = None, dev_mode: bool = False) -> None: initialize_velopack() register_protocol(sys.executable) - configure_logging() - logger = logging.getLogger('synodic_client') - _install_exception_hook(logger) - if uri: logger.info('Received URI: %s', uri) diff --git a/synodic_client/updater.py b/synodic_client/updater.py index c1bb0ac..f1e49d6 100644 --- a/synodic_client/updater.py +++ b/synodic_client/updater.py @@ -104,6 +104,13 @@ def __init__(self, current_version: Version, config: UpdateConfig | None = None) self._velopack_manager: Any = None self._velopack_not_installed: bool = False + logger.info( + 'Updater created: version=%s, channel=%s, repo=%s', + self._current_version, + self._config.channel_name, + self._config.repo_url, + ) + @property def state(self) -> UpdateState: """Current state of the update process.""" @@ -193,6 +200,7 @@ def download_update(self, progress_callback: Callable[[int], None] | None = None return False self._state = UpdateState.DOWNLOADING + logger.info('Starting update download for %s', self._update_info._velopack_info) try: manager = self._get_velopack_manager() @@ -323,6 +331,12 @@ def _get_velopack_manager(self) -> Any: self._config.repo_url, options, ) + logger.debug( + 'Velopack manager created: app_id=%s, version=%s, portable=%s', + self._velopack_manager.get_app_id(), + self._velopack_manager.get_current_version(), + self._velopack_manager.get_is_portable(), + ) return self._velopack_manager except RuntimeError as e: if self._NOT_INSTALLED_SENTINEL in str(e).lower(): @@ -336,7 +350,7 @@ def _get_velopack_manager(self) -> Any: raise RuntimeError(f'Failed to create Velopack UpdateManager: {e}') from e -def _on_before_uninstall(version: str) -> None: # noqa: ARG001 +def _on_before_uninstall(version: str) -> None: """Velopack hook: called before the app is uninstalled. Removes the ``synodic://`` URI protocol handler registration. @@ -344,10 +358,12 @@ def _on_before_uninstall(version: str) -> None: # noqa: ARG001 Args: version: The current version string (provided by Velopack). """ + logger.info('Velopack uninstall hook fired for version %s', version) try: remove_protocol() + logger.info('Protocol handler removed successfully') except Exception: - logger.debug('Protocol removal failed during uninstall hook', exc_info=True) + logger.warning('Protocol removal failed during uninstall hook', exc_info=True) def initialize_velopack() -> None: @@ -360,10 +376,11 @@ def initialize_velopack() -> None: On Windows, the uninstall hook removes the ``synodic://`` URI protocol. Protocol registration happens on every app launch (see ``qt.application``). """ + logger.info('Initializing Velopack (exe=%s)', sys.executable) try: app = velopack.App() app.on_before_uninstall_fast_callback(_on_before_uninstall) app.run() - logger.debug('Velopack initialized') + logger.info('Velopack initialized successfully') except Exception as e: - logger.debug('Velopack initialization skipped: %s', e) + logger.info('Velopack initialization skipped (not a Velopack install): %s', e)