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]asyncfnmain() -> 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 :)

[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]asyncfnmain() -> 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 :)

[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]asyncfnmain() -> 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 :)

[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]asyncfnmain() -> 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 :)

[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]asyncfnmain() -> 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 :)

[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]asyncfnmain() -> 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 :)

[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]asyncfnmain() -> 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 :)

[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]asyncfnmain() -> 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 :)