Skip to content

fix(mcp): fail fast on HTML content-type instead of waiting full connect_timeout - #36058

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/mcp-html-content-type-fail-fast
Closed

fix(mcp): fail fast on HTML content-type instead of waiting full connect_timeout#36058
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/mcp-html-content-type-fail-fast

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds a lightweight HTTP pre-flight check to MCPServerTask._run_http() that detects misconfigured MCP server URLs returning text/html (e.g. pointing at a web app root) and fails fast with an actionable error message, instead of waiting the full connect_timeout (default 60 s) before surfacing CancelledError.

Related Issue

Fixes #36052

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/mcp_tool.py: Added _preflight_content_type() static method on MCPServerTask that sends a HEAD request (with GET fallback on 405) and raises ConnectionError if the response Content-Type is text/html. Called from _run_http() before the MCP SDK handshake.
  • tests/tools/test_mcp_preflight_content_type.py: 6 tests covering HTML rejection (HEAD + GET fallback), JSON acceptance, missing Content-Type, network error passthrough, and header/verify forwarding.

How to Test

  1. Configure a misconfigured MCP server in config.yaml:
    mcp_servers:
      bad_endpoint:
        url: "https://example.com"  # returns text/html
  2. Start Hermes — the server should fail within ~5 s with: MCP server 'https://example.com' returned Content-Type: text/html; ...
  3. Configure a valid MCP server URL — connection proceeds normally.
  4. Run: pytest tests/tools/test_mcp_preflight_content_type.py -v

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: MCPServerTask._run_http (callers: 2 — _run dispatcher, SSE skip path)
  • Blast radius: LOW — pre-flight check is additive; all exceptions (except ConnectionError) are swallowed to preserve existing behavior
  • Related patterns: PR Soft-fail slow MCP discovery at startup #29764 (soft-fail slow MCP discovery at startup) addresses a complementary concern (slow servers, not wrong content type)

…ect_timeout

A misconfigured MCP server URL that returns text/html (e.g. pointing at
a web app root instead of an MCP endpoint) causes the MCP SDK to block
for the full connect_timeout (default 60 s) before surfacing
CancelledError.

Add a lightweight HEAD pre-flight check that detects text/html responses
in ≤5 s and raises ConnectionError with an actionable message. Non-HTML
responses, missing headers, and network errors pass through silently so
the normal MCP handshake proceeds unaffected.

Fixes NousResearch#36052
@alt-glitch alt-glitch added type/bug Something isn't working tool/mcp MCP client and OAuth P2 Medium — degraded but workaround exists labels May 31, 2026
@stanleycyang

Copy link
Copy Markdown

Reviewed as part of the open PR queue. This is superseded by #36064 for #36052. #36064 handles non-MCP content types in the actual httpx response path and has green checks; this PR currently has a failing test shard. Recommend closing this one.

@liuhao1024

Copy link
Copy Markdown
Contributor Author

@stanleycyang Thanks for the review. I see #36064 handles the content-type check in a different path and has green CI. Since both PRs are still open, I'll wait for the maintainer's decision. If #36064 is preferred, I'm happy to close this PR.

@teknium1

teknium1 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Merged via #37133 (#37133).

Your preflight-probe approach was the architecturally correct one — probing on a separate httpx client before the SDK connects sidesteps the ExceptionGroup wrapping that defeats hooks installed inside the MCP SDK transport. That mechanism is what shipped, and your commit's authorship is preserved on main (c914e4a).

The salvage hardened it on top: allow-list detection (application/json / text/event-stream) instead of a text/html-only denylist, a non-retryable NonMcpEndpointError so the reconnect-backoff loop is skipped entirely (fails in ~0.07s, not after 3 retries), ssl_verify/client_cert/header forwarding, HEAD→GET fallback on 405/501, and a rewritten test suite that drives the real method. You're credited as a co-author. Thanks for the fix and for getting there first!

@teknium1 teknium1 closed this Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bad HTTP MCP endpoint returning HTML blocks discovery for 60s before CancelledError

4 participants