Skip to content

diag(mcp): per-request phase logging on the streamable-HTTP transport#5819

Merged
waleedlatif1 merged 2 commits into
stagingfrom
diag/mcp-transport-fetch-logging
Jul 21, 2026
Merged

diag(mcp): per-request phase logging on the streamable-HTTP transport#5819
waleedlatif1 merged 2 commits into
stagingfrom
diag/mcp-transport-fetch-logging

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Adds per-request phase logging to the pinned MCP transport fetch (createPinnedMcpFetch), mirroring the OAuth guarded-fetch instrumentation that isolated the /oauth/start body stall.
  • Logs each transport request (host + method), the response headers with TTFB, and failures.
  • Why: the logs prove our client's first connect to these servers consistently hangs to the 30s client timeout while a normal client's initialize returns in ~120 ms — but we haven't isolated where it stalls. This logging shows whether the first connect hangs before response headers (connect/request phase) or after (the SDK's stream/body read) — the data needed to root-cause it before touching the single-IP-pin SSRF control.

Type of Change

  • Diagnostic (temporary logging; no behavior change)

Testing

Type-check + biome clean. No behavior change — wraps the existing pinned fetch with logging only.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Mirrors the OAuth guarded-fetch logging that isolated the /oauth/start body
stall. Logs each transport request (host+method), the response headers with
TTFB, and failures — so a first-connect stall shows whether it hangs before
response headers (connect/request) or after (the SDK's stream read), which
isolates the client-side first-connect stall we haven't yet root-caused.
@vercel

vercel Bot commented Jul 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 21, 2026 9:49pm

Request Review

@cursor

cursor Bot commented Jul 21, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Logging-only wrapper around existing fetch; no timeout, pinning, or response handling changes.

Overview
Adds diagnostic instrumentation around the pinned fetch returned by createPinnedMcpFetch, following the same phase-logging pattern already used on OAuth guarded fetches.

Each transport call now logs request start (host + method), response headers (status + TTFB), or a failure (elapsed ms and error metadata) via a new McpTransportFetch logger. The underlying pinned fetch and close behavior are unchanged—only a wrapper that rethrows errors after logging.

The intent is to tell whether first initialize hangs before response headers (connect/request) or after (SDK stream/body read), without changing SSRF pinning or transport semantics.

Reviewed by Cursor Bugbot for commit a12bac5. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds phase logging to the pinned MCP transport fetch. The main changes are:

  • Logs each transport request with its host and method.
  • Logs response status and time to first byte.
  • Logs structured error details before rethrowing failures.

Confidence Score: 5/5

This looks safe to merge.

  • The updated failure log captures the error name and nested cause code for expected transport failures.
  • The wrapper preserves the original response, rejection, and dispatcher cleanup behavior.
  • No blocking issues were found in the changed code.

Important Files Changed

Filename Overview
apps/sim/lib/mcp/pinned-fetch.ts Wraps the pinned fetch with request, response-header, timing, and structured failure logs while preserving response and error propagation.

Reviews (2): Last reviewed commit: "diag(mcp): include error name/code in tr..." | Re-trigger Greptile

Comment thread apps/sim/lib/mcp/pinned-fetch.ts
Comment thread apps/sim/lib/mcp/pinned-fetch.ts
Distinguishes abort vs connect-refused vs TLS vs timeout in the failure-phase
data.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit a12bac5. Configure here.

@waleedlatif1
waleedlatif1 merged commit 9fb9ac5 into staging Jul 21, 2026
20 checks passed
@waleedlatif1
waleedlatif1 deleted the diag/mcp-transport-fetch-logging branch July 22, 2026 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant