Prevent Qt aborts while applying settings

.github/workflows/release.yml

@@ -75,6 +75,11 @@ jobs:
           pip install -r requirements.txt
           pip install "pyinstaller>=5.0" "Pillow>=10.0.0"
 
+      - name: Run tests
+        env:
+          QT_QPA_PLATFORM: offscreen
+        run: python -m unittest discover tests -v
+
       - name: Build with PyInstaller
         run: pyinstaller --clean --noconfirm bongo_cat.spec
 
@@ -189,6 +194,7 @@ jobs:
 
   release:
     name: Publish GitHub Release
+    if: startsWith(github.ref, 'refs/tags/v')
     needs: build
     runs-on: ubuntu-latest
     permissions:

bongo_cat/__init__.py

@@ -6,7 +6,7 @@
 from .utils import resource_path, setup_logging
 from . import animations
 
-__version__ = "2.0.5"
+__version__ = "2.0.6"
 __author__ = "luinbytes"
 __description__ = "Interactive desktop pet that responds to keyboard, mouse, and controller inputs"
 

bongo_cat/ui/main_window.py

@@ -1315,10 +1315,17 @@ def apply_settings(self):
         self.config.max_slaps = self.config.max_slaps_spinbox.value()
         self.config.invert_cat = self.config.invert_cat_checkbox.isChecked()
 
-        # Update skin
+        # Save every selection before refreshing runtime objects. If a Qt or
+        # audio object fails, a restart can still load the requested settings.
         selected_skin_id = self.config.skin_dropdown.currentData()
-        if selected_skin_id != self.config.current_skin:
-            self.config.current_skin = selected_skin_id
+        skin_changed = selected_skin_id != self.config.current_skin
+        self.config.current_skin = selected_skin_id
+        self.config.sound_enabled = self.config.sound_enabled_checkbox.isChecked()
+        self.config.sound_volume = self.config.sound_volume_slider.value()
+        self.config.save()
+
+        # Update skin
+        if skin_changed:
             self.skin_manager.load_skin(selected_skin_id)
             # Reload images and tray icon
             self.setup_cat_images()
@@ -1329,13 +1336,8 @@ def apply_settings(self):
                 self.tray_icon.setIcon(QtGui.QIcon(resource_path(icon_path)))
 
         # Update sound settings
-        self.config.sound_enabled = self.config.sound_enabled_checkbox.isChecked()
-        self.config.sound_volume = self.config.sound_volume_slider.value()
         self.sound_manager.enabled = self.config.sound_enabled
         self.sound_manager.set_volume(self.config.sound_volume / 100.0)
-
-        # Apply settings
-        self.config.save()
 
         # Stop footer callbacks while settings rewrite style and visibility.
         self.footer_animation.stop()
@@ -1365,6 +1367,19 @@ def apply_settings(self):
         if old_invert_cat != self.config.invert_cat:
             self.update_stretched_image()
 
+    def apply_settings_safely(self):
+        """Keep exceptions in the Qt Apply callback from aborting the app."""
+        try:
+            self.apply_settings()
+        except Exception as error:
+            logger.exception("Failed to apply settings")
+            QtWidgets.QMessageBox.critical(
+                self.settings_panel,
+                "Could not apply all settings",
+                "Bongo Cat is still running, but some settings may need a restart. "
+                f"Restart Bongo Cat and share bongo.log if this continues.\n\n{error}",
+            )
+
     def reset_counter_confirm(self):
         """Confirm before resetting the counter."""
         msg_box = QtWidgets.QMessageBox(self)
@@ -1557,7 +1572,7 @@ def setup_settings_panel(self):
             padding: 6px 12px;
             border-radius: 4px;
         """)
-        apply_button.clicked.connect(self.apply_settings)
+        apply_button.clicked.connect(self.apply_settings_safely)
 
         # Close button
         close_button = QtWidgets.QPushButton("Close")

tests/test_settings_apply_runtime.py

@@ -0,0 +1,67 @@
+"""Runtime regression test for exceptions raised while applying settings."""
+
+import importlib.util
+import os
+from pathlib import Path
+import subprocess
+import sys
+import tempfile
+import textwrap
+import unittest
+
+
+@unittest.skipUnless(importlib.util.find_spec("PyQt5"), "PyQt5 is not installed")
+class TestSettingsApplyRuntime(unittest.TestCase):
+    def test_apply_exception_does_not_abort_qt_process(self):
+        script = textwrap.dedent(
+            """
+            from PyQt5 import QtCore, QtWidgets
+            from bongo_cat.ui.main_window import BongoCatWindow
+
+            app = QtWidgets.QApplication([])
+            window = BongoCatWindow()
+            apply_button = next(
+                button
+                for button in window.settings_panel.findChildren(QtWidgets.QPushButton)
+                if button.text() == "Apply"
+            )
+            window.sound_manager.set_volume = lambda _volume: (_ for _ in ()).throw(
+                RuntimeError("injected Apply failure")
+            )
+            window.config.sound_enabled_checkbox.setChecked(False)
+            QtWidgets.QMessageBox.critical = lambda *_args: print(
+                "apply-error-dialog", flush=True
+            )
+            QtCore.QTimer.singleShot(0, apply_button.click)
+            QtCore.QTimer.singleShot(500, app.quit)
+            app.exec_()
+            print("after-apply-click", flush=True)
+            print(f"saved-sound-enabled={window.config.__class__().sound_enabled}", flush=True)
+            """
+        )
+
+        with tempfile.TemporaryDirectory() as appdata:
+            env = os.environ.copy()
+            env.update(
+                APPDATA=appdata,
+                PYNPUT_BACKEND="dummy",
+                QT_QPA_PLATFORM="offscreen",
+            )
+            result = subprocess.run(
+                [sys.executable, "-c", script],
+                cwd=Path(__file__).parents[1],
+                env=env,
+                capture_output=True,
+                text=True,
+                timeout=10,
+            )
+
+        self.assertEqual(0, result.returncode, result.stderr)
+        self.assertIn("apply-error-dialog", result.stdout)
+        self.assertIn("after-apply-click", result.stdout)
+        self.assertIn("saved-sound-enabled=False", result.stdout)
+        self.assertIn("injected Apply failure", result.stderr)
+
+
+if __name__ == "__main__":
+    unittest.main()