fix(state): forward @change listener exceptions to on_exception

README.rst

@@ -26,8 +26,7 @@ trame-server can be installed with `pip <https://pypi.org/project/trame-server/>
 
 .. code-block:: bash
 
-    pip install --upgrade trame-server
-
+    pip install trame-server
 
 Usage
 -----------------------------------------------------------
@@ -59,7 +58,7 @@ and are prefixed with ``on_*``.
 * **on_server_exited**    : Trame is exiting its event loop
 
 * **on_error**            : JS error forwarded (msg:str) on the python
-* **on_exception**        : Server exception occurred when interacting with the UI. The exception instance is provided as arg.
+* **on_exception**        : Server exception occurred when interacting with the UI, in a ``@state.change`` listener (sync or async) or in a controller task (``ctrl.add_task``). The exception instance is provided as arg.
 
 * **on_server_reload**    : If callback registered it can be use to hot_reload methods like the UI.
 
@@ -83,3 +82,23 @@ Enjoying trame?
 -----------------------------------------------------------
 
 Share your experience `with a testimonial <https://github.com/Kitware/trame/issues/18>`_ or `with a brand approval <https://github.com/Kitware/trame/issues/19>`_.
+
+Development
+-----------------------------------------------------------
+
+Install the library in dev mode
+
+.. code-block:: bash
+
+    # Create venv and install all dependencies
+    uv sync --all-extras --dev
+
+    # Activate environment
+    source .venv/bin/activate
+
+    # Install commit analysis
+    pre-commit install
+    pre-commit install --hook-type commit-msg
+
+    # Run pre-commit checks
+    pre-commit run --all-files

examples/exceptions/app_trame.py

@@ -1,9 +1,10 @@
+import asyncio
 import logging
 import sys
 import traceback
 
 from trame.app import TrameApp
-from trame.decorators import life_cycle
+from trame.decorators import change, life_cycle
 from trame.ui.html import DivLayout
 from trame.widgets import html
 
@@ -14,17 +15,25 @@
 class MonitorException(TrameApp):
     def __init__(self, server=None):
         super().__init__(server)
+        self.server.state.test = 1
+        self.server.state.test_async = 1
+        self.server.state.allow_exception = False
         self._build_ui()
-        self.ctrl.on_exception.add(self.on_exception)
 
     def _build_ui(self):
         with DivLayout(self.server) as self.ui:
             html.Button("Divide by 0", click=self.divide_by_zero)
             html.Button("JS error", click="console.error('Yo')")
+            html.Button("Divide by 0 in change listener", click="test += 1")
+            html.Button("Divide by 0 in async change listener", click="test_async += 1")
 
     def divide_by_zero(self):
         return 1 / 0
 
+    @life_cycle.server_ready
+    def on_ready(self, **_):
+        self.state.allow_exception = True
+
     @life_cycle.exception
     def on_exception(self, exception):
         print(">" * 60)
@@ -42,6 +51,19 @@ def on_js_error(self, msg):
 
         print("JS error:", msg)
 
+    @change("test")
+    def test_is_changed(self, allow_exception, **_):
+        if not allow_exception:
+            return 0
+        return 1 / 0
+
+    @change("test_async")
+    async def test_async_is_changed(self, allow_exception, **_):
+        if not allow_exception:
+            return 0
+        await asyncio.sleep(0.1)
+        return 1 / 0
+
 
 def main():
     app = MonitorException()

pyproject.toml

@@ -73,6 +73,7 @@ xfail_strict = true
 minversion = "6.0"
 addopts = ["-ra", "--strict-config", "--strict-markers"]
 filterwarnings = ["error"]
+asyncio_default_fixture_loop_scope = "function"
 testpaths = [
     "tests",
 ]

src/trame_server/controller.py

@@ -65,8 +65,15 @@ class Controller:
     >>> ctrl.on_data_change.clear(set_only=True)  # add, remove, discard, clear
     """
 
-    def __init__(self, translator=None, internal=None, hot_reload=False):
+    def __init__(
+        self, translator=None, internal=None, hot_reload=False, exception_handler=None
+    ):
         super().__setattr__("__trame_hot_reload__", hot_reload)
+        # Child controllers always report to the root controller handler
+        super().__setattr__(
+            "_exception_handler",
+            share(internal, "_exception_handler", exception_handler),
+        )
         super().__setattr__("_translator", translator or Translator())
         super().__setattr__("_triggers", share(internal, "_triggers", {}))
         super().__setattr__(
@@ -387,9 +394,7 @@ def __call__(self, *args, **kwargs):
 
         # Schedule any task
         for task_fn in list(self.task_funcs):
-            results.append(
-                asynchronous.create_task(_safe_call(task_fn, *args, **kwargs))
-            )
+            results.append(self._create_task(_safe_call(task_fn, *args, **kwargs)))
 
         # Figure out return
         if self.func is None:
@@ -400,6 +405,34 @@ def __call__(self, *args, **kwargs):
             return results
         return result
 
+    def _create_task(self, coroutine):
+        task = asynchronous.get_event_loop().create_task(coroutine)
+        task.add_done_callback(self._on_task_done)
+        return task
+
+    def _on_task_done(self, task):
+        if task.cancelled():
+            return
+
+        exception = task.exception()
+        if exception is None:
+            return
+
+        # Never forward failures of on_exception itself to avoid infinite loops
+        handler = self.controller._exception_handler
+        if handler is not None and self.name != "on_exception":
+            try:
+                if handler(exception):
+                    return
+            except Exception:
+                logger.exception("Exception handler failed while reporting")
+
+        logger.error(
+            "Exception raised by %s task",
+            self.name,
+            exc_info=(type(exception), exception, exception.__traceback__),
+        )
+
     def once(self, func):
         """
         Add function to the set of functions to be called when

src/trame_server/core.py

@@ -103,7 +103,10 @@ def __init__(
         # Shared state + reserve internal keys
         if parent_server is None:
             self._state = State(
-                self.translator, commit_fn=self._push_state, hot_reload=self.hot_reload
+                self.translator,
+                commit_fn=self._push_state,
+                hot_reload=self.hot_reload,
+                exception_handler=self._handle_exception,
             )
             for key in [
                 "scripts",
@@ -128,7 +131,11 @@ def __init__(
 
         # Controller
         if parent_server is None:
-            self._controller = Controller(self.translator, hot_reload=self.hot_reload)
+            self._controller = Controller(
+                self.translator,
+                hot_reload=self.hot_reload,
+                exception_handler=self._handle_exception,
+            )
         else:
             self._controller = Controller(
                 self.translator,
@@ -138,7 +145,11 @@ def __init__(
 
         # Server only context
         if parent_server is None:
-            self._context = State(self.translator, hot_reload=self.hot_reload)
+            self._context = State(
+                self.translator,
+                hot_reload=self.hot_reload,
+                exception_handler=self._handle_exception,
+            )
         else:
             self._context = State(
                 self.translator,
@@ -149,6 +160,20 @@ def __init__(
         # UI (FIXME): use for translator
         self._ui = share(parent_server, "_ui", VirtualNodeManager(self, vn_constructor))
 
+    def _handle_exception(self, exception) -> bool:
+        """
+        Forward an exception raised by a state/context change listener
+        or a controller task to controller.on_exception
+        (i.e. @life_cycle.exception).
+
+        :return: True if the exception was forwarded, False otherwise
+        """
+        on_exception = self.controller.on_exception
+        if on_exception.exists() or on_exception.funcs_once:
+            on_exception(exception)
+            return True
+        return False
+
     def create_child_server(self, translator=None, prefix=None) -> Server:
         translator = translator or Translator(prefix=prefix)
         return Server(translator=translator, parent_server=self)

src/trame_server/state.py

@@ -180,8 +180,13 @@ def __init__(
         commit_fn=None,
         hot_reload=False,
         ready=False,
+        exception_handler=None,
     ):
         self._push_state_fn = commit_fn
+        # Child states always report to the root state handler
+        self._exception_handler = share(
+            internal, "_exception_handler", exception_handler
+        )
         self._hot_reload = hot_reload
         self._translator = translator or Translator()
         self._modified_keys = share(internal, "_modified_keys", set())
@@ -444,13 +449,41 @@ def _flush_pending_keys(self) -> set[str]:
             try:
                 coroutine = callback(**reverse_translated_state)
                 if inspect.isawaitable(coroutine):
-                    asynchronous.create_task(coroutine)
+                    task = asynchronous.get_event_loop().create_task(coroutine)
+                    task.add_done_callback(self._on_change_task_done)
             except Exception as e:
-                logger.warning("@change callback exception ignored: %s", e)
+                self._report_exception(e)
 
         self._state_listeners.clear()
         return _keys
 
+    def _report_exception(self, exception):
+        """
+        Forward an exception raised by a @change callback to the exception
+        handler (server.controller.on_exception) when available, otherwise
+        log it so it does not go unnoticed.
+        """
+        if self._exception_handler is not None:
+            try:
+                if self._exception_handler(exception):
+                    return
+            except Exception:
+                logger.exception("Exception handler failed while reporting")
+
+        logger.warning(
+            "@change callback exception ignored: %s",
+            exception,
+            exc_info=(type(exception), exception, exception.__traceback__),
+        )
+
+    def _on_change_task_done(self, task):
+        if task.cancelled():
+            return
+
+        exception = task.exception()
+        if exception is not None:
+            self._report_exception(exception)
+
     def flush(self):
         """
         Force pushing modified state and execute any @state.change listener