Record post-chain result failures on the OpenTelemetry span

The context-tier OpenTelemetryMiddleware closes its span when the
middleware chain returns, but result dumping and spec-result
serialization ran after that in _on_request. A handler returning an
unsupported type or a malformed dict for a spec method surfaced as a
JSON-RPC error to the client while the span finished UNSET with no
error attributes, so default error-rate dashboards missed the failure.

Move dumping and serialization into the chain (a new _serialize helper
called from _inner) so the span observes these failures. Serializing
inside the chain also lets a middleware short-circuit own its result
shape directly, replacing the post-chain KeyError catch.

Also point the SEP-414 doc link at its spec-repo PR rather than the
repository root.

Author: Kludex

docs/advanced/opentelemetry.md

@@ -68,7 +68,7 @@ connected picture.
 When the client and the server both run the SDK, that connection is automatic. The client injects
 the [W3C trace context](https://www.w3.org/TR/trace-context/) into the request, and the server
 reads it back out, so the server span nests under the client span in the same trace. This is
-[SEP-414](https://github.com/modelcontextprotocol/modelcontextprotocol), and you get it without
+[SEP-414](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/414), and you get it without
 asking.
 
 If the inbound message carries no trace context, for example a request from a client that is not

src/mcp/server/runner.py

@@ -171,7 +171,6 @@ async def _on_request(
         meta = _extract_meta(params)
         version = self.connection.protocol_version
         ctx = self._make_context(dctx, method, params, meta, version)
-        is_spec_method = method in _methods.SPEC_CLIENT_METHODS
 
         async def _inner(ctx: ServerRequestContext[LifespanT, Any]) -> HandlerResult:
             # Read method/params off `ctx` so a middleware that rewrote them via
@@ -190,7 +189,7 @@ async def _inner(ctx: ServerRequestContext[LifespanT, Any]) -> HandlerResult:
             # the gate become a per-version legacy path then. Initialize runs inline
             # (read loop parked), so awaiting the peer anywhere on this path deadlocks.
             if method == "initialize":
-                return self._handle_initialize(params)
+                return self._serialize(method, version, self._handle_initialize(params))
             # Methods without a handler are METHOD_NOT_FOUND regardless of
             # initialization state: JSON-RPC 2.0 reserves -32601 for "not
             # available on this server", and clients probing a server before
@@ -209,25 +208,14 @@ async def _inner(ctx: ServerRequestContext[LifespanT, Any]) -> HandlerResult:
             if isinstance(result, ErrorData):
                 # Raise inside the chain so middleware observes the failure.
                 raise MCPError.from_error_data(result)
-            return result
+            # Dump and serialize inside the chain so the OpenTelemetry span (the
+            # outermost middleware) records a failing handler return shape too.
+            return self._serialize(method, version, result)
 
         call = self._compose_server_middleware(_inner)
+        # `_inner` already produced the wire dict; a middleware that short-circuited
+        # without `call_next` is trusted to return its own well-formed result.
         result = _dump_result(await call(ctx))
-        # TODO(L56): reject resultType values outside {"complete", "input_required"} unless the
-        # corresponding extension is in this request's _meta clientCapabilities.extensions; the
-        # explicit MUST-reject is client-side (basic/index.mdx ResultType), this enforces it proactively.
-        if is_spec_method:
-            try:
-                result = _methods.serialize_server_result(method, version, result)
-            except KeyError:
-                # Middleware short-circuited a wrong-version spec method without
-                # calling `call_next`; it owns the result shape.
-                pass
-            except ValidationError:
-                # Server bug, not client fault. Detail stays in the server log:
-                # pydantic messages echo the result body.
-                logger.exception("handler for %r returned an invalid result", method)
-                raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None
         if method == "initialize":
             # Commit only on chain success, so a middleware veto leaves no state.
             # Race-free: the read loop is parked until this call returns.
@@ -335,6 +323,28 @@ def _make_context(
             close_standalone_sse_stream=close_standalone_sse_stream,
         )
 
+    @staticmethod
+    def _serialize(method: str, version: str, result: HandlerResult) -> dict[str, Any]:
+        """Dump a handler result to the wire dict, serializing spec methods.
+
+        Runs inside the middleware chain so the OpenTelemetry span observes a
+        failing return shape (unsupported type, malformed spec result) as an
+        error rather than closing on a request that the client sees fail.
+        """
+        dumped = _dump_result(result)
+        # TODO(L56): reject resultType values outside {"complete", "input_required"} unless the
+        # corresponding extension is in this request's _meta clientCapabilities.extensions; the
+        # explicit MUST-reject is client-side (basic/index.mdx ResultType), this enforces it proactively.
+        if method not in _methods.SPEC_CLIENT_METHODS:
+            return dumped
+        try:
+            return _methods.serialize_server_result(method, version, dumped)
+        except ValidationError:
+            # Server bug, not client fault. Detail stays in the server log:
+            # pydantic messages echo the result body.
+            logger.exception("handler for %r returned an invalid result", method)
+            raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None
+
     @staticmethod
     def _negotiate_initialize(params: Mapping[str, Any] | None) -> tuple[InitializeRequestParams, str]:
         """Validate `initialize` params and pick the protocol version."""

tests/server/test_otel.py

@@ -11,6 +11,7 @@
 import anyio
 import pytest
 from mcp_types import (
+    INTERNAL_ERROR,
     INVALID_PARAMS,
     CallToolRequestParams,
     CallToolResult,
@@ -297,6 +298,28 @@ async def failing(ctx: Ctx, params: PaginatedRequestParams | None) -> Any:
     assert event.attributes["exception.type"] == "ValueError"
 
 
+@pytest.mark.anyio
+async def test_records_error_status_on_malformed_spec_result(server: SrvT, spans: SpanCapture):
+    """Result serialization runs inside the span, so a handler returning a
+    malformed dict for a spec method (INTERNAL_ERROR on the wire) is recorded
+    on the span rather than closing it as a success."""
+
+    async def bad_result(ctx: Ctx, params: PaginatedRequestParams | None) -> dict[str, Any]:
+        return {"tools": 42}
+
+    server.add_request_handler("tools/list", PaginatedRequestParams, bad_result)
+    async with connected_runner(server) as (client, _):
+        spans.clear()
+        with pytest.raises(MCPError) as exc:
+            await client.send_raw_request("tools/list", None)
+        assert exc.value.error.code == INTERNAL_ERROR
+    [span] = [s for s in spans.finished() if s.kind == SpanKind.SERVER]
+    assert span.status.status_code == StatusCode.ERROR
+    assert span.attributes is not None
+    assert span.attributes["error.type"] == str(INTERNAL_ERROR)
+    assert span.attributes["rpc.response.status_code"] == str(INTERNAL_ERROR)
+
+
 @pytest.mark.anyio
 async def test_passes_rewritten_context_through(server: SrvT, spans: SpanCapture):
     seen_arguments: dict[str, Any] = {}