fix(mcp): tolerate non-dict result shapes in InstrumentedStreamWriter.send (#4038) - #4396
fix(mcp): tolerate non-dict result shapes in InstrumentedStreamWriter.send (#4038)#4396Anai-Guo wants to merge 1 commit into
Conversation
….send (traceloop#4038) send() read request.result as a dict and assumed content[0] was a text block. Because send() is wrapped in @dont_throw, any raise there was swallowed and the wrapped send() was never awaited, so the MCP message was dropped instead of delivered. Read isError/content defensively for dict- and object-shaped results.
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe MCP instrumentation now supports mapping- and object-shaped response results. It safely extracts error text and sets span status. Tests cover missing, empty, and non-text content, plus non-error responses. ChangesMCP result compatibility
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fixes #4038.
The bug
InstrumentedStreamWriter.send()readsrequest.resultas a dict and assumes thefirst content block is a text block:
send()is decorated with@dont_throw, so anything raised here is swallowed andlogged at DEBUG — which means
await self.__wrapped__.send(item)on the last line isnever reached and the MCP message is silently dropped. The span is also left with
the instrumentation's own exception as its status description, masking the real tool
error.
Four shapes hit this, all reachable from a normal MCP server:
request.resultmainisErrorwith an image/resource first content blockKeyError: 'text'isErrorwith an emptycontentlistIndexErrorisErrorwith nocontentkeyKeyError: 'content'TypeError: ... is not iterableCallToolResult.contentis a list ofContentBlock, and onlyTextContentcarries.text— so an error result whose content is an image is enough to lose the response.The fix
Two small module-level helpers read the result defensively for both dict- and
object-shaped payloads, and the error description falls back to
""rather thanraising. The
isErrorsemantics are unchanged (is True, as before).This also lines the writer path up with
_execute_and_handle_result(
instrumentation.py:343,:420), which already useshasattr(result, "isError")—the inconsistency the issue notes.
Verification
Ran against the released
opentelemetry-instrumentation-mcp==0.62.1, which isbyte-identical to this file on
main, so old-vs-new is a like-for-like comparison.New
tests/test_stream_writer_result_shapes.py(6 cases: the four shapes above, thealready-working text-block shape, and a non-error result):
4 failed, 2 passed— the four broken shapes never forward the message6 passedExisting MCP package suite (
test_error_type.py,test_fastmcp.py,test_fastmcp_attributes.py,test_fastmcp_server_span.py):5 passedboth beforeand after.
uvx ruff@0.4.0 checkclean against the package's ownpyproject.toml(line-length120,
select = ["E", "F", "W"]).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests