Don't treat non-success HTTP codes as transport errors

[SteffenDE]

The spec states that:

If the server cannot accept the input, it MUST return an HTTP error
status code (e.g., 400 Bad Request).
The HTTP response body MAY comprise a JSON-RPC error response that has no id.

But the rust-sdk treated any POST responses with non successful status as an error.

Before this patch, trying to call list_resources on a server that does return HTTP 400 with JSON-RPC error:

TransportSend(
    DynamicTransportError {
        transport_name: "rmcp::transport::worker::WorkerTransport<rmcp::transport::streamable_http_client::StreamableHttpClientWorker<reqwest::async_impl::client::Client>>",
        transport_type_id: TypeId(0xf7bacf3cf3889d471ead32d7a3cc8155),
        error: Client(
            reqwest::Error {
                kind: Status(
                    400,
                    None,
                ),
                url: "http://localhost:4000/mcp",
            },
        ),
    },
)

After this patch:

McpError(
    ErrorData {
        code: ErrorCode(
            -32601,
        ),
        message: "Method not found",
        data: Some(
            Object {
                "name": String("resources/list"),
            },
        ),
    },
)

Motivation and Context

Clients should be able to see the error the server sends. The code already checks for JSON content type, I assume the error_for_status for included by accident.

How Has This Been Tested?

Tests still seem to pass, but I did not add a new one. For testing, I created a demo client, but someone with more knowledge of the code should add a proper test case.

use anyhow::Result;
use rmcp::{
    ServiceExt,
    model::{ClientCapabilities, ClientInfo, Implementation},
    transport::StreamableHttpClientTransport,
};
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};

#[tokio::main]
async fn main() -> Result<()> {
    tracing_subscriber::registry()
        .with(
            tracing_subscriber::EnvFilter::try_from_default_env()
                .unwrap_or_else(|_| format!("debug,{}=debug", env!("CARGO_CRATE_NAME")).into()),
        )
        .with(tracing_subscriber::fmt::layer())
        .init();

    let transport = StreamableHttpClientTransport::from_uri("http://localhost:4000/mcp");
    let client_info = ClientInfo {
        protocol_version: Default::default(),
        capabilities: ClientCapabilities::default(),
        client_info: Implementation {
            name: "test unknown method client".to_string(),
            title: None,
            version: "0.0.1".to_string(),
            website_url: None,
            icons: None,
        },
    };

    let client = client_info.serve(transport).await.inspect_err(|e| {
        tracing::error!("client error during connection: {:?}", e);
    })?;

    let server_info = client.peer_info();
    tracing::info!("Connected to server: {server_info:#?}");

    tracing::info!("Calling list_resources (which the server doesn't support)...");
    match client.list_resources(Default::default()).await {
        Ok(result) => {
            tracing::info!("Success: {result:#?}");
        }
        Err(e) => {
            tracing::error!("Error calling list_resources: {e:#?}");
        }
    }

    client.cancel().await?;
    Ok(())
}

Breaking Changes

Could be seen as a breaking change if people relied on getting the transport error.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

[Copilot]

Pull Request Overview

This PR fixes HTTP transport error handling to properly distinguish between transport-level failures and application-level JSON-RPC errors returned via HTTP error status codes, aligning with the MCP specification.

• Removes error_for_status() calls that incorrectly treated HTTP error codes as transport errors
• Allows JSON-RPC error responses with HTTP 4xx/5xx status codes to be parsed and returned as proper MCP errors
• Enables clients to receive server-sent error details instead of generic HTTP transport errors

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
crates/rmcp/src/transport/common/reqwest/streamable_http_client.rs	Removes error_for_status() check to allow non-2xx responses to be processed as potential JSON-RPC errors
crates/rmcp/src/transport/common/reqwest/sse_client.rs	Removes error_for_status() check for SSE transport consistency

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[4t145]

Good catch, but should we have some log when response status is not success?

[SteffenDE]

I don’t think there’s a need to log, since it’s part of the spec to have non-200 codes and a client will still receive this as an error, so users can decide to log if they want.

[alexhancock]

It LGTM. Sorry for the delay in getting to it.

@SteffenDE Do you mind rebasing and having the commit message conform to https://www.conventionalcommits.org, then we can merge.

[SteffenDE]

@alexhancock No worries! We're still using the SSE code, so I don't want to update that branch for now. As it's only one line to remove

rust-sdk/crates/rmcp/src/transport/common/reqwest/streamable_http_client.rs

Line 124 in 95d3e3f

let response = response.error_for_status()?;

I think it's fine if I just close the PR and you do that change directly :)