From 6e2d9ed1348758e411376fe5956720b5d801590b Mon Sep 17 00:00:00 2001 From: Endika Iglesias Date: Mon, 3 Aug 2026 11:16:48 +0200 Subject: [PATCH] fix: report message handler failures instead of transport errors --- src/pact/_server.py | 28 +++++++++++++-- src/pact/verifier.py | 77 ++++++++++++++++++++++++++++++++++++++++-- tests/test_server.py | 42 +++++++++++++++++++++++ tests/test_verifier.py | 39 +++++++++++++++++++++ 4 files changed, 181 insertions(+), 5 deletions(-) diff --git a/src/pact/_server.py b/src/pact/_server.py index e05c9dfe4..be2c8435e 100644 --- a/src/pact/_server.py +++ b/src/pact/_server.py @@ -282,9 +282,21 @@ def do_POST(self) -> None: self.send_error(400, "Bad Request") return - self.send_response(200, "OK") + # The handler must be called before the response line is sent. Any + # exception it raises has to be reported as a 500, and not leave the + # client with a truncated 200 response. + try: + message = self.server.handler(description, data) + except Exception as e: + logger.exception("Message handler for %s raised an exception.", description) + self.send_error( + 500, + "Message handler failed", + f"{type(e).__name__}: {e}", + ) + return - message = self.server.handler(description, data) + self.send_response(200, "OK") metadata = message.get("metadata") or {} if content_type := message.get("content_type"): @@ -491,7 +503,17 @@ def do_POST(self) -> None: self.send_error(400, "Bad Request") return - self.server.handler(state, action, params) + try: + self.server.handler(state, action, params) + except Exception as e: + logger.exception("State handler for %s raised an exception.", state) + self.send_error( + 500, + "State handler failed", + f"{type(e).__name__}: {e}", + ) + return + self.send_response(200, "OK") self.end_headers() diff --git a/src/pact/verifier.py b/src/pact/verifier.py index c706c71c3..3f3bbc672 100644 --- a/src/pact/verifier.py +++ b/src/pact/verifier.py @@ -111,6 +111,65 @@ logger = logging.getLogger(__name__) +def _missing_contents_msg(name: str) -> str: + """ + Error message for a message handler dictionary without a `contents` key. + + Args: + name: + The name of the message whose handler is invalid. + """ + return ( + f"Message handler for {name!r} is missing the 'contents' key. Dictionary " + "values must be Message envelopes, such as {'contents': b'...', " + "'content_type': 'application/json'}, and not the raw payload." + ) + + +def _invalid_value_msg(name: str, value: object) -> str: + """ + Error message for a message handler value of an unsupported type. + + Args: + name: + The name of the message whose handler is invalid. + + value: + The offending value. + """ + return ( + f"Invalid message handler value for {name!r}: expected a callable, bytes, " + f"or a Message dictionary, got {type(value).__name__}." + ) + + +def _validate_message_handlers( + handler: dict[str, Callable[..., Message] | Message | bytes], +) -> None: + """ + Check that every value of a message handler dictionary is usable. + + The handler is only called during verification, at which point a failure is + reported by the underlying FFI as a failed interaction and its cause is + easily missed. + + Args: + handler: + The dictionary mapping message names to handler values. + + Raises: + TypeError: + If any value is neither a callable, bytes, nor a Message dictionary. + """ + for name, value in handler.items(): + if callable(value) or isinstance(value, bytes): + continue + if not isinstance(value, dict): + raise TypeError(_invalid_value_msg(name, value)) + if "contents" not in value: + raise TypeError(_missing_contents_msg(name)) + + class _ProviderTransport(TypedDict): """ Provider transport information. @@ -385,6 +444,12 @@ def message_handler( Raises: TypeError: If the handler or its values are invalid. + + KeyError: + If a message is requested which is not present in the + dictionary. As the handler is called during verification, this + is raised within the message relay server and surfaces as a + failed interaction. """ logger.debug( "Setting message handler for verifier", @@ -414,12 +479,19 @@ def _handler( return self if isinstance(handler, dict): + _validate_message_handlers(handler) def _handler( name: str, metadata: dict[str, Any] | None, ) -> Message: logger.info("Internal message produced called.") + if name not in handler: + msg = ( + f"No message handler for {name!r}. " + f"Known messages: {', '.join(sorted(handler))}" + ) + raise KeyError(msg) val = handler[name] if callable(val): @@ -430,14 +502,15 @@ def _handler( if isinstance(val, bytes): return Message(contents=val, metadata=None, content_type=None) if isinstance(val, dict): + if "contents" not in val: + raise TypeError(_missing_contents_msg(name)) return Message( contents=val["contents"], metadata=val.get("metadata"), content_type=val.get("content_type"), ) - msg = "Invalid message handler value" - raise TypeError(msg) + raise TypeError(_invalid_value_msg(name, val)) self._message_producer = MessageProducer(_handler) self.add_transport( diff --git a/tests/test_server.py b/tests/test_server.py index 3526ac1d5..b9ff5f098 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -73,6 +73,30 @@ async def test_message_post_http() -> None: assert handler.call_args.args == ("A simple message", {}) +@pytest.mark.asyncio +async def test_message_post_handler_raises() -> None: + """ + A failing handler must produce a 500, not a truncated 200. + + The response line used to be sent before the handler was called, so any + exception left the client with a half-written response and the Pact core + reported it as `error sending request for url`. See #1665. + """ + handler = MagicMock(side_effect=RuntimeError("handler is broken")) + server = MessageProducer(handler) + + with server: + async with aiohttp.ClientSession() as session: + async with session.post( + server.url, + data=json.dumps({"description": "A simple message"}), + ) as response: + assert response.status == 500 + assert "handler is broken" in await response.text() + + handler.assert_called_once() + + def test_callback_default_init() -> None: handler = MagicMock() server = StateCallback(handler) @@ -132,3 +156,21 @@ async def test_callback_post() -> None: "setup", {"id": 123}, ) + + +@pytest.mark.asyncio +async def test_callback_post_handler_raises() -> None: + """A failing state handler must produce a 500.""" + handler = MagicMock(side_effect=RuntimeError("state setup is broken")) + server = StateCallback(handler) + + with server: + async with aiohttp.ClientSession() as session: + async with session.post( + server.url, + json={"state": "user exists", "action": "setup", "params": {}}, + ) as response: + assert response.status == 500 + assert "state setup is broken" in await response.text() + + handler.assert_called_once() diff --git a/tests/test_verifier.py b/tests/test_verifier.py index b249d2958..5f6991310 100644 --- a/tests/test_verifier.py +++ b/tests/test_verifier.py @@ -238,6 +238,45 @@ def test_verify_message_only(verifier: Verifier) -> None: mock_add_transport.assert_not_called() +@pytest.mark.parametrize( + ("handler", "match"), + [ + pytest.param( + {"a-message": {"field": "value"}}, + r"missing the 'contents' key", + id="dict_without_contents", + ), + pytest.param( + {"a-message": 42}, + r"expected a callable, bytes, or a Message dictionary, got int", + id="unsupported_value_type", + ), + ], +) +def test_message_handler_invalid_dict_value( + verifier: Verifier, + handler: dict[str, Any], + match: str, +) -> None: + """ + Invalid handler values must be rejected when the handler is set. + + Deferring the error to verification time hides the cause behind a failed + interaction. See #1665. + """ + with pytest.raises(TypeError, match=match): + verifier.message_handler(handler) + + +def test_message_handler_unknown_message(verifier: Verifier) -> None: + """A message with no handler must name the messages which do have one.""" + verifier.message_handler({"a-message": b"", "b-message": b""}) + handler = verifier._message_producer._handler # noqa: SLF001 + + with pytest.raises(KeyError, match=r"Known messages: a-message, b-message"): + handler("c-message", None) + + def test_logs(verifier: Verifier) -> None: logs = verifier.logs assert logs == ""