fix: report message handler failures instead of transport errors

src/pact/_server.py

@@ -282,23 +282,58 @@ def do_POST(self) -> None:
             self.send_error(400, "Bad Request")
             return
 
-        self.send_response(200, "OK")
-
-        message = self.server.handler(description, data)
+        # 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
 
-        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)
@@ -491,7 +526,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()
 

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(

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
@@ -73,6 +74,71 @@ 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()
+
+
+@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)
@@ -132,3 +198,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()

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
@@ -238,6 +239,47 @@ 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""})
+    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)
+
+
 def test_logs(verifier: Verifier) -> None:
     logs = verifier.logs
     assert logs == ""