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 a24d6ea..f1e49d6 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__) @@ -102,6 +102,14 @@ 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 + + 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: @@ -110,12 +118,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: @@ -187,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() @@ -284,42 +298,59 @@ 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, ) + 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(): + 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_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 +def _on_before_uninstall(version: str) -> None: """Velopack hook: called before the app is uninstalled. Removes the ``synodic://`` URI protocol handler registration. @@ -327,7 +358,12 @@ def _on_before_uninstall(version: str) -> None: # noqa: ARG001 Args: version: The current version string (provided by Velopack). """ - remove_protocol() + logger.info('Velopack uninstall hook fired for version %s', version) + try: + remove_protocol() + logger.info('Protocol handler removed successfully') + except Exception: + logger.warning('Protocol removal failed during uninstall hook', exc_info=True) def initialize_velopack() -> None: @@ -337,13 +373,14 @@ 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``). """ + logger.info('Initializing Velopack (exe=%s)', sys.executable) 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') + 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) 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()