refactor: make EmitLoop error message clearer (#232)

* refactor: adjust traceback of callback

* merge

* style(pre-commit.ci): auto fixes [...]

* slot repr

* fix: fix type

* coverage

* style(pre-commit.ci): auto fixes [...]

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

src/psygnal/_exceptions.py

@@ -1,11 +1,47 @@
+from __future__ import annotations
+
+from typing import TYPE_CHECKING, Callable
+
+from ._weak_callback import WeakCallback
+
+if TYPE_CHECKING:
+    from ._signal import SignalInstance
+
+MSG = """
+While emitting signal {sig!r}, an error occurred in callback {cb!r}.
+The args passed to the callback were: {args!r}
+This is not a bug in psygnal.  See {err!r} above for details.
+"""
+
+
 class EmitLoopError(Exception):
     """Error type raised when an exception occurs during a callback."""
 
-    def __init__(self, slot_repr: str, args: tuple, exc: BaseException) -> None:
-        self.slot_repr = slot_repr
+    def __init__(
+        self,
+        cb: WeakCallback | Callable,
+        args: tuple,
+        exc: BaseException,
+        signal: SignalInstance | None = None,
+    ) -> None:
+        self.exc = exc
         self.args = args
         self.__cause__ = exc  # mypyc doesn't set this, but uncompiled code would
+        if signal is None:
+            sig_name = ""
+        else:
+            inst_class = signal.instance.__class__
+            mod = getattr(inst_class, "__module__", "")
+            sig_name = f"{mod}.{inst_class.__qualname__}.{signal.name}"
+        if isinstance(cb, WeakCallback):
+            cb_name = cb.slot_repr()
+        else:
+            cb_name = getattr(cb, "__qualname__", repr(cb))
         super().__init__(
-            f"calling {self.slot_repr} with args={args!r} caused "
-            f"{type(exc).__name__}: {exc}."
+            MSG.format(
+                sig=sig_name,
+                cb=cb_name,
+                args=args,
+                err=exc.__class__.__name__,
+            )
         )

src/psygnal/_queue.py

@@ -95,4 +95,4 @@ def emit_queued(thread: Thread | None = None) -> None:
         try:
             cb(args)
         except Exception as e:  # pragma: no cover
-            raise EmitLoopError(slot_repr=repr(cb), args=args, exc=e) from e
+            raise EmitLoopError(cb=cb, args=args, exc=e) from e

src/psygnal/_signal.py

@@ -990,7 +990,7 @@ def _run_emit_loop(self, args: tuple[Any, ...]) -> None:
                         caller.cb(args)
                     except Exception as e:
                         raise EmitLoopError(
-                            slot_repr=repr(caller), args=args, exc=e
+                            cb=caller, args=args, exc=e, signal=self
                         ) from e
 
         return None

src/psygnal/_weak_callback.py

@@ -188,6 +188,8 @@ def __init__(
         on_ref_error: RefErrorChoice = "warn",
     ) -> None:
         self._key: str = WeakCallback.object_key(obj)
+        self._obj_module: str = getattr(obj, "__module__", None) or ""
+        self._obj_qualname: str = getattr(obj, "__qualname__", "")
         self._object_repr: str = WeakCallback.object_repr(obj)
         self._max_args: int | None = max_args
         self._alive: bool = True
@@ -237,6 +239,9 @@ def _strong_ref() -> _T:
 
             return _strong_ref
 
+    def slot_repr(self) -> str:
+        return f"{self._obj_module}.{self._obj_qualname}"
+
     @staticmethod
     def object_key(obj: Any) -> str:
         """Return a unique key for an object.
@@ -394,10 +399,14 @@ def __init__(
         self._func_ref = self._try_ref(obj.__func__, finalize)
         self._args = args
         self._kwargs = kwargs or {}
-
         if args:
             self._object_repr = f"{self._object_repr}{(*args,)!r}".replace(")", " ...)")
 
+    def slot_repr(self) -> str:
+        obj = self._obj_ref()
+        func_name = getattr(self._func_ref(), "__name__", "<method>")
+        return f"{self._obj_module}.{obj.__class__.__qualname__}.{func_name}"
+
     def cb(self, args: tuple[Any, ...] = ()) -> None:
         obj = self._obj_ref()
         func = self._func_ref()
@@ -442,10 +451,13 @@ def __init__(
         self._obj_ref = self._try_ref(obj.__self__, finalize)
         self._func_name = obj.__name__
         self._args = args
-
         if args:
             self._object_repr = f"{self._object_repr}{(*args,)!r}".replace(")", " ...)")
 
+    def slot_repr(self) -> str:
+        obj = self._obj_ref()
+        return f"{obj.__class__.__qualname__}.{self._func_name}"
+
     def cb(self, args: tuple[Any, ...] = ()) -> None:
         func = getattr(self._obj_ref(), self._func_name, None)
         if func is None:
@@ -474,9 +486,12 @@ def __init__(
         self._key += f".__setattr__({attr!r})"
         self._obj_ref = self._try_ref(obj, finalize)
         self._attr = attr
-
         self._object_repr += f".__setattr__({attr!r}, ...)"
 
+    def slot_repr(self) -> str:
+        obj = self._obj_ref()
+        return f"setattr({obj.__class__.__qualname__}, {self._attr!r}, ...)"
+
     def cb(self, args: tuple[Any, ...] = ()) -> None:
         obj = self._obj_ref()
         if obj is None:
@@ -510,9 +525,12 @@ def __init__(
         self._key += f".__setitem__({key!r})"
         self._obj_ref = self._try_ref(obj, finalize)
         self._itemkey = key
-
         self._object_repr += f".__setitem__({key!r}, ...)"
 
+    def slot_repr(self) -> str:
+        obj = self._obj_ref()
+        return f"{obj.__class__.__qualname__}.__setitem__({self._itemkey!r}, ...)"
+
     def cb(self, args: tuple[Any, ...] = ()) -> None:
         obj = self._obj_ref()
         if obj is None:

tests/test_weak_callable.py

@@ -1,5 +1,6 @@
 import gc
 from functools import partial
+from typing import Any
 from unittest.mock import Mock
 from weakref import ref
 
@@ -178,7 +179,7 @@ def func(x):
     assert dp.keywords == p.keywords
 
 
-def test_queued_callbacks():
+def test_queued_callbacks() -> None:
     from psygnal._queue import QueuedCallback
 
     def func(x):
@@ -189,3 +190,27 @@ def func(x):
 
     assert qcb.dereference() is func
     assert qcb(1) == 1
+
+
+def test_cb_raises() -> None:
+    from psygnal import EmitLoopError
+
+    m = str(EmitLoopError(weak_callback(print), (1,), RuntimeError("test")))
+    assert "an error occurred in callback 'module.print'" in m
+    m = str(EmitLoopError(print, (1,), RuntimeError("test")))
+    assert " an error occurred in callback 'print'" in m
+
+    class T:
+        x = 1
+
+        def __setitem__(self, *_: Any) -> Any:
+            pass
+
+    t = T()
+    cb = weak_callback(setattr, t, "x")
+    m = str(EmitLoopError(cb, (2,), RuntimeError("test")))
+    assert 'an error occurred in callback "setattr' in m
+
+    cb = weak_callback(t.__setitem__, "x")
+    m = str(EmitLoopError(cb, (2,), RuntimeError("test")))
+    assert ".T.__setitem__" in m