From 4f8ef2925eef97c9c92f4978c4c0d4311793f4b9 Mon Sep 17 00:00:00 2001 From: Endika Iglesias Date: Mon, 3 Aug 2026 11:16:48 +0200 Subject: [PATCH 1/3] 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 == "" From 03541399477980b1de9283b5a248355c6137860b Mon Sep 17 00:00:00 2001 From: JP-Ellis Date: Tue, 11 Aug 2026 20:17:51 +1000 Subject: [PATCH 2/3] chore(tests): narrow message producer before accessing its handler The unknown-message test reached into `Verifier._message_producer`, which is typed as a union with `nullcontext`, so mypy rejected the attribute access and the typecheck job failed. Assisted-by: Claude Code:claude-opus-5[1m] --- tests/test_verifier.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_verifier.py b/tests/test_verifier.py index 5f6991310..196defbe4 100644 --- a/tests/test_verifier.py +++ b/tests/test_verifier.py @@ -10,6 +10,7 @@ import json import re +from contextlib import nullcontext from pathlib import Path from typing import Any from unittest.mock import patch @@ -271,7 +272,9 @@ def test_message_handler_invalid_dict_value( 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 + producer = verifier._message_producer # noqa: SLF001 + assert not isinstance(producer, nullcontext) + handler = producer._handler # noqa: SLF001 with pytest.raises(KeyError, match=r"Known messages: a-message, b-message"): handler("c-message", None) From 1352f4b54ca4f9e736ce5f96637c251d2a80262f Mon Sep 17 00:00:00 2001 From: JP-Ellis Date: Tue, 11 Aug 2026 20:20:56 +1000 Subject: [PATCH 3/3] fix: report unserialisable messages instead of transport errors Calling the message handler before the response line closed the window for exceptions raised by the handler, but not for exceptions raised while turning its return value into a response. A handler returning `str` contents, or metadata which is not JSON serialisable, still left the client with a truncated 200 and the Pact core still reported it as `error sending request for url`. The response is now serialised in full before anything is sent, so these failures surface as a 500 naming the cause. Assisted-by: Claude Code:claude-opus-5[1m] --- src/pact/_server.py | 47 +++++++++++++++++++++++++++++++++----------- tests/test_server.py | 42 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/src/pact/_server.py b/src/pact/_server.py index be2c8435e..f1f1d48de 100644 --- a/src/pact/_server.py +++ b/src/pact/_server.py @@ -296,21 +296,44 @@ def do_POST(self) -> None: ) return - self.send_response(200, "OK") - - metadata = message.get("metadata") or {} - if content_type := message.get("content_type"): - self.send_header("Content-Type", content_type) - if "contentType" not in metadata: + # The response is serialised in full before the response line is sent, + # for the same reason: a malformed message must be reported as a 500, + # and not leave the client with a truncated 200 response. + try: + contents = message.get("contents") or b"" + if not isinstance(contents, (bytes, bytearray)): + msg = ( + f"Message handler for {description!r} returned contents of " + f"type {type(contents).__name__}, expected bytes." + ) + raise TypeError(msg) # noqa: TRY301 + + metadata = message.get("metadata") or {} + content_type = message.get("content_type") + if content_type and "contentType" not in metadata: metadata["contentType"] = content_type - - if metadata: - self.send_header( - "Pact-Message-Metadata", - base64.b64encode(json.dumps(metadata).encode()).decode(), + encoded_metadata = ( + base64.b64encode(json.dumps(metadata).encode()).decode() + if metadata + else None + ) + except Exception as e: + logger.exception( + "Message from handler for %s could not be serialised.", + description, ) + self.send_error( + 500, + "Message handler failed", + f"{type(e).__name__}: {e}", + ) + return - contents = message.get("contents", b"") + self.send_response(200, "OK") + if content_type: + self.send_header("Content-Type", content_type) + if encoded_metadata: + self.send_header("Pact-Message-Metadata", encoded_metadata) self.send_header("Content-Length", str(len(contents))) self.end_headers() self.wfile.write(contents) diff --git a/tests/test_server.py b/tests/test_server.py index b9ff5f098..2764569f6 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -5,6 +5,7 @@ from __future__ import annotations import json +from typing import Any from unittest.mock import MagicMock import aiohttp @@ -97,6 +98,47 @@ async def test_message_post_handler_raises() -> None: handler.assert_called_once() +@pytest.mark.parametrize( + ("message", "match"), + [ + pytest.param( + {"contents": "not bytes", "metadata": None, "content_type": None}, + "expected bytes", + id="str_contents", + ), + pytest.param( + {"contents": b"", "metadata": {"key": object()}, "content_type": None}, + "TypeError", + id="unserialisable_metadata", + ), + ], +) +@pytest.mark.asyncio +async def test_message_post_unserialisable_message( + message: dict[str, Any], + match: str, +) -> None: + """ + A message which cannot be serialised must produce a 500. + + The handler itself succeeds here; it is the response which cannot be built, + and that must not leave the client with a truncated 200 either. + """ + handler = MagicMock(return_value=message) + 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 match in await response.text() + + handler.assert_called_once() + + def test_callback_default_init() -> None: handler = MagicMock() server = StateCallback(handler)