fix: type Signal dispatch contract with Protocol

bluetoothbot · bluetoothbot · commit 034cd229c39b · 2026-05-26T17:05:41.000Z
Signal._handlers was typed list[Callable[..., None]] and fire
accepted arbitrary **kwargs, so a typo at a fire site or a missing
parameter at a handler only blew up the moment a real service
event dispatched - hours into a run on a quiet network. mypy
could not catch dispatch mismatches when the contract shifted.

Lock the contract down: define a ServiceStateChangeHandler
Protocol describing the (zeroconf, service_type, name,
state_change) keyword signature, type Signal._handlers as a list
of that Protocol, and make Signal.fire keyword-only with the four
named parameters. register_handler / unregister_handler still
accept Callable[..., None] for back-compat and cast at the
boundary.
diff --git a/src/zeroconf/__init__.py b/src/zeroconf/__init__.py
@@ -54,6 +54,7 @@
 from ._services import (  # noqa # import needed for backwards compat
     ServiceListener,
     ServiceStateChange,
+    ServiceStateChangeHandler,
     Signal,
     SignalRegistrationInterface,
 )
@@ -112,6 +113,7 @@
     "ServiceListener",
     "ServiceNameAlreadyRegistered",
     "ServiceStateChange",
+    "ServiceStateChangeHandler",
     "Zeroconf",
     "ZeroconfServiceTypes",
     "__version__",
diff --git a/src/zeroconf/_services/__init__.py b/src/zeroconf/_services/__init__.py
@@ -24,7 +24,7 @@
 
 import enum
 from collections.abc import Callable
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING, Protocol, cast
 
 if TYPE_CHECKING:
     from .._core import Zeroconf
@@ -48,15 +48,40 @@ def update_service(self, zc: Zeroconf, type_: str, name: str) -> None:
         raise NotImplementedError
 
 
+class ServiceStateChangeHandler(Protocol):
+    """Callback contract dispatched by :class:`Signal` to service-state listeners."""
+
+    def __call__(
+        self,
+        *,
+        zeroconf: Zeroconf,
+        service_type: str,
+        name: str,
+        state_change: ServiceStateChange,
+    ) -> None: ...
+
+
 class Signal:
     __slots__ = ("_handlers",)
 
     def __init__(self) -> None:
-        self._handlers: list[Callable[..., None]] = []
-
-    def fire(self, **kwargs: Any) -> None:
+        self._handlers: list[ServiceStateChangeHandler] = []
+
+    def fire(
+        self,
+        *,
+        zeroconf: Zeroconf,
+        service_type: str,
+        name: str,
+        state_change: ServiceStateChange,
+    ) -> None:
         for h in self._handlers[:]:
-            h(**kwargs)
+            h(
+                zeroconf=zeroconf,
+                service_type=service_type,
+                name=name,
+                state_change=state_change,
+            )
 
     @property
     def registration_interface(self) -> SignalRegistrationInterface:
@@ -66,13 +91,13 @@ def registration_interface(self) -> SignalRegistrationInterface:
 class SignalRegistrationInterface:
     __slots__ = ("_handlers",)
 
-    def __init__(self, handlers: list[Callable[..., None]]) -> None:
+    def __init__(self, handlers: list[ServiceStateChangeHandler]) -> None:
         self._handlers = handlers
 
     def register_handler(self, handler: Callable[..., None]) -> SignalRegistrationInterface:
-        self._handlers.append(handler)
+        self._handlers.append(cast("ServiceStateChangeHandler", handler))
         return self
 
     def unregister_handler(self, handler: Callable[..., None]) -> SignalRegistrationInterface:
-        self._handlers.remove(handler)
+        self._handlers.remove(cast("ServiceStateChangeHandler", handler))
         return self
diff --git a/tests/test_services.py b/tests/test_services.py
@@ -261,3 +261,76 @@ def dummy():
 
     with pytest.raises(ValueError):
         interface.unregister_handler(dummy)
+
+
+def test_signal_fire_dispatches_documented_kwargs():
+    """Signal.fire forwards the four documented kwargs to handlers."""
+    signal = r.Signal()
+    captured: list[dict[str, Any]] = []
+
+    def handler(
+        *,
+        zeroconf: Any,
+        service_type: str,
+        name: str,
+        state_change: r.ServiceStateChange,
+    ) -> None:
+        captured.append(
+            {
+                "zeroconf": zeroconf,
+                "service_type": service_type,
+                "name": name,
+                "state_change": state_change,
+            }
+        )
+
+    signal.registration_interface.register_handler(handler)
+    sentinel = object()
+    signal.fire(
+        zeroconf=sentinel,  # type: ignore[arg-type]
+        service_type="_http._tcp.local.",
+        name="x._http._tcp.local.",
+        state_change=r.ServiceStateChange.Added,
+    )
+
+    assert captured == [
+        {
+            "zeroconf": sentinel,
+            "service_type": "_http._tcp.local.",
+            "name": "x._http._tcp.local.",
+            "state_change": r.ServiceStateChange.Added,
+        }
+    ]
+
+
+def test_signal_fire_rejects_unknown_kwarg():
+    """Signal.fire rejects keyword args outside the contract."""
+    signal = r.Signal()
+    signal.registration_interface.register_handler(lambda **_: None)
+
+    with pytest.raises(TypeError):
+        signal.fire(  # type: ignore[call-arg]
+            zerocnf=None,
+            service_type="_http._tcp.local.",
+            name="x._http._tcp.local.",
+            state_change=r.ServiceStateChange.Added,
+        )
+
+
+def test_signal_fire_rejects_positional_args():
+    """Signal.fire is keyword-only, so positional args are rejected."""
+    signal = r.Signal()
+    signal.registration_interface.register_handler(lambda **_: None)
+
+    with pytest.raises(TypeError):
+        signal.fire(  # type: ignore[misc]
+            None,
+            "_http._tcp.local.",
+            "x._http._tcp.local.",
+            r.ServiceStateChange.Added,
+        )
+
+
+def test_service_state_change_handler_protocol_exported():
+    """The handler Protocol is part of the public package surface."""
+    assert hasattr(r, "ServiceStateChangeHandler")
