traceloop / traceloop/openllmetry

🐛 Bug Report: MCP client send_request is @dont_throw'd, so a tracing error returns None and breaks the tool call

Open
#4,463 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
7.4k
Forks
1.1k
Avg merge
8d 14h
Merged PRs (30d)
2

Description

Which component is this bug for?

Traceloop SDK

📜 Description

In opentelemetry-instrumentation-mcp, the client send_request wrapper is decorated with @dont_throw:

def patch_mcp_client(self, tracer: Tracer):
    @dont_throw
    async def traced_method(wrapped, instance, args, kwargs):
        ...

dont_throw catches every Exception, logs it at DEBUG, and then falls through — so it returns None:

async def async_wrapper(*args, **kwargs):
    try:
        return await func(*args, **kwargs)
    except Exception as e:
        _handle_exception(e, func, logger)   # logger.debug(...), no return

dont_throw is safe on a wrapper whose return value is discarded, but BaseSession.send_request returns the RPC result. So any exception raised inside the instrumentation replaces the real CallToolResult with None, and the MCP SDK then dereferences it:

# mcp/client/session.py, ClientSession.call_tool
result = await self.send_request(..., types.CallToolResult, ...)
if not result.isError:          # AttributeError when instrumentation returned None

Impact

An error in tracing becomes an error in the traced call. The tool call fails with a confusing AttributeError that names neither MCP nor the instrumentation, and because the real cause is logged at DEBUG, any deployment running at INFO or above has no record of what actually went wrong.

Observed error

AttributeError: 'NoneType' object has no attribute 'isError'

Seen in production on a tools/call to a remote MCP server; the tool became unusable for that run while every other MCP tool kept working.

Two candidate throwers inside the wrapper

  1. carrier["traceparent"] is read unconditionally after injection:
carrier = {}
TraceContextTextMapPropagator().inject(carrier)
meta.traceparent = carrier["traceparent"]   # KeyError when nothing was injected

inject() writes no traceparent when the current span context is invalid or non-recording, so this raises KeyError.

  1. Post-call span decoration in _execute_and_handle_result reads result.content[0].text unguarded:
if hasattr(result, "isError") and result.isError:
    if len(result.content) > 0:
        span.set_status(Status(StatusCode.ERROR, f"{result.content[0].text}"))

This raises AttributeError for any non-text content block (image, resource, audio), all of which are valid MCP content types. This one is the more serious shape, because it happens after the RPC has already been made — so the result exists and is then thrown away.

File

  • packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/instrumentation.py
  • packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/utils.py

Method

  • McpInstrumentor.patch_mcp_client.traced_method
  • McpInstrumentor._execute_and_handle_result
  • dont_throw

Proposed fix

Any of these, in rough order of preference:

  1. Do not apply dont_throw to a wrapper whose return value is load-bearing. Instead, contain failures around the instrumentation work only, and always return the wrapped call's result.
  2. Give dont_throw (or a dont_throw_preserving_result variant) an explicit fallback: return await wrapped(*args, **kwargs) is not safe once the call may already have run, so the fix belongs inside the wrapper where the result is in scope.
  3. Independently, guard the two reads above: carrier.get("traceparent") and getattr(result.content[0], "text", None).

Reproduced against 0.53.3 and confirmed present in 0.62.3.

👟 Reproduction steps
  1. Instrument an MCP client with opentelemetry-instrumentation-mcp.
  2. Make a tools/call whose result contains a single non-text content block (for example an image or an embedded resource) and isError set, so post-call span decoration reads result.content[0].text.
  3. Observe that _execute_and_handle_result raises, dont_throw swallows it and returns None, and the call fails in ClientSession.call_tool with AttributeError: 'NoneType' object has no attribute 'isError'.

An equivalent path: invoke a tool while the current span context is non-recording, so carrier["traceparent"] raises KeyError before the RPC is even issued.

👍 Expected behavior

A failure inside tracing should never change the outcome of the traced call. send_request should return the MCP result it received (untraced or partially traced if necessary), and the swallowed instrumentation exception should be logged at a level visible in production rather than DEBUG.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/instrumentation.py and read patch_mcp_client.traced_method plus _execute_and_handle_result; then inspect dont_throw in utils.py and follow the reproduction steps. Done means instrumentation failures no longer replace the MCP RPC result with None, the carrier and content reads handle the shown cases, and the failure remains visible in production logs.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
observability
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.