|
| 1 | +# Plan: Classify authorization-required initialization errors |
| 2 | + |
| 3 | +**Date:** 2026-07-27 |
| 4 | +**Issue:** [#795](https://github.com/modelcontextprotocol/rust-sdk/issues/795) |
| 5 | +**Goal:** Let clients detect that MCP initialization requires re-authorization without downcasting a transport-specific error or matching multiple internal variants. Preserve the existing error details and `WWW-Authenticate` challenge. |
| 6 | +**Status:** Complete |
| 7 | + |
| 8 | +## Context |
| 9 | + |
| 10 | +An authorized streamable HTTP client can currently report the same caller action—start authorization again—through two different paths: |
| 11 | + |
| 12 | +- `StreamableHttpError::Auth(AuthError::AuthorizationRequired)` when the local OAuth manager has no usable credentials or cannot refresh them. |
| 13 | +- `StreamableHttpError::AuthRequired(AuthRequiredError)` when the MCP endpoint returns an HTTP 401 challenge. |
| 14 | + |
| 15 | +`ClientInitializeError` erases the concrete transport error into `DynamicTransportError`. Callers must therefore know the HTTP client's concrete error type, downcast it, and match both variants. |
| 16 | + |
| 17 | +The variants should remain distinct internally. The HTTP form carries a `WWW-Authenticate` header, while the OAuth-manager form describes local credential state and is only available with the `auth` feature. Merging them would either lose challenge data or make the streamable HTTP transport depend on OAuth even when OAuth support is disabled. |
| 18 | + |
| 19 | +The public API addition is: |
| 20 | + |
| 21 | +```rust |
| 22 | +impl ClientInitializeError { |
| 23 | + pub fn is_authorization_required(&self) -> bool; |
| 24 | +} |
| 25 | +``` |
| 26 | + |
| 27 | +Internally, `DynamicTransportError` inspects the standard `std::error::Error::source` chain. `ClientInitializeError` delegates for its `TransportError` variant and returns `false` for all other variants. The transport-level helper remains crate-internal until another public use case requires it. |
| 28 | + |
| 29 | +`AuthRequiredError` must participate in that source chain. It should implement `std::error::Error`, and `StreamableHttpError::AuthRequired` should expose it as its source. `StreamableHttpError::Auth(AuthError)` already exposes `AuthError` through `thiserror`. |
| 30 | + |
| 31 | +## Scope |
| 32 | + |
| 33 | +- Detect both authorization-required paths through one public predicate. |
| 34 | +- Work for direct and wrapped transport errors without requiring the caller to name the HTTP backend's error type. |
| 35 | +- Preserve existing public variants and challenge data. |
| 36 | +- Document the re-authorization pattern. |
| 37 | + |
| 38 | +## Non-goals |
| 39 | + |
| 40 | +- Do not merge or remove `AuthError::AuthorizationRequired` and `StreamableHttpError::AuthRequired`. |
| 41 | +- Do not classify `InsufficientScope`, transient refresh failures, session expiry, or arbitrary HTTP failures as authorization-required. |
| 42 | +- Do not automatically start an OAuth flow. |
| 43 | +- Do not add a generalized error-category framework or change the `Transport` trait. |
| 44 | +- Do not add the predicate to `RmcpError` until a caller demonstrates that the broader wrapper needs it. |
| 45 | + |
| 46 | +## File Map |
| 47 | + |
| 48 | +| File | Role | Status | |
| 49 | +| --- | --- | --- | |
| 50 | +| `crates/rmcp/src/transport/streamable_http_client.rs` | Verify the HTTP 401 detail error is visible through the standard source chain | Complete upstream | |
| 51 | +| `crates/rmcp/src/transport.rs` | Add transport-independent authorization-required classification | Complete | |
| 52 | +| `crates/rmcp/src/service/client.rs` | Add the initialization-error convenience predicate | Complete | |
| 53 | +| `crates/rmcp/tests/test_auth_error_classification.rs` | Exercise the public API and guard false positives | Complete | |
| 54 | +| `crates/rmcp/Cargo.toml` | Declare the feature-gated integration test | Complete | |
| 55 | +| `docs/OAUTH_SUPPORT.md` | Show callers how to branch back into authorization | Complete | |
| 56 | + |
| 57 | +## Tasks |
| 58 | + |
| 59 | +### 1. Add public-API regression tests |
| 60 | + |
| 61 | +**Files:** `crates/rmcp/tests/test_auth_error_classification.rs`, `crates/rmcp/Cargo.toml` |
| 62 | + |
| 63 | +**Description:** Add a feature-gated integration test that constructs public error values through `DynamicTransportError::from_parts` and verifies the intended classification before implementation. |
| 64 | + |
| 65 | +**Acceptance criteria:** |
| 66 | + |
| 67 | +- [x] `StreamableHttpError::Auth(AuthError::AuthorizationRequired)` classifies as authorization-required. |
| 68 | +- [x] `StreamableHttpError::AuthRequired(AuthRequiredError)` classifies as authorization-required. |
| 69 | +- [x] A `ClientInitializeError::TransportError` wrapping either path classifies as authorization-required. |
| 70 | +- [x] An unrelated `StreamableHttpError` classifies as false. |
| 71 | +- [x] `AuthError::TokenRefreshFailed` and `StreamableHttpError::InsufficientScope` classify as false. |
| 72 | +- [x] Non-transport `ClientInitializeError` variants classify as false. |
| 73 | +- [x] The test requires `auth`, `client`, and `transport-streamable-http-client`, but not a concrete reqwest or Unix-socket backend. |
| 74 | + |
| 75 | +### 2. Verify the HTTP challenge source chain |
| 76 | + |
| 77 | +**Files:** `crates/rmcp/src/transport/streamable_http_client.rs` |
| 78 | + |
| 79 | +**Description:** Confirm that `AuthRequiredError` implements `std::error::Error` and that `StreamableHttpError::AuthRequired` exposes it as its source. This prerequisite is already present on the latest `origin/main`. |
| 80 | + |
| 81 | +**Acceptance criteria:** |
| 82 | + |
| 83 | +- [x] `AuthRequiredError` remains constructible through `AuthRequiredError::new`. |
| 84 | +- [x] `www_authenticate_header` remains available and unchanged. |
| 85 | +- [x] `StreamableHttpError::source()` returns the contained `AuthRequiredError`. |
| 86 | +- [x] Existing error display behavior remains unchanged. |
| 87 | +- [x] No existing enum variants or fields are removed or renamed. |
| 88 | + |
| 89 | +### 3. Add transport-independent classification |
| 90 | + |
| 91 | +**Files:** `crates/rmcp/src/transport.rs` |
| 92 | + |
| 93 | +**Description:** Add a crate-internal `DynamicTransportError::is_authorization_required()`. Walk the inner error and its sources, recognizing `AuthError::AuthorizationRequired` when `auth` is enabled and `AuthRequiredError` when the streamable HTTP client is enabled. Keep the transport-level classifier internal until another public use case requires it. |
| 94 | + |
| 95 | +**Acceptance criteria:** |
| 96 | + |
| 97 | +- [x] Client callers do not need the transport type, backend error type, or a downcast. |
| 98 | +- [x] Direct and multiply wrapped matching errors are recognized. |
| 99 | +- [x] Feature-disabled builds compile; feature-specific checks are guarded locally. |
| 100 | +- [x] Unknown custom transport errors return false. |
| 101 | +- [x] The helper name describes classification rather than an authorization action. |
| 102 | + |
| 103 | +### 4. Add the initialization-error convenience API |
| 104 | + |
| 105 | +**Files:** `crates/rmcp/src/service/client.rs` |
| 106 | + |
| 107 | +**Description:** Add `ClientInitializeError::is_authorization_required()`, delegating only when the variant is `TransportError`. |
| 108 | + |
| 109 | +**Acceptance criteria:** |
| 110 | + |
| 111 | +- [x] The issue's caller can replace its downcast helper with `error.is_authorization_required()`. |
| 112 | +- [x] JSON-RPC, protocol negotiation, connection-closed, and cancellation failures return false. |
| 113 | +- [x] The method remains available consistently across client builds. |
| 114 | + |
| 115 | +### 5. Document the recovery pattern |
| 116 | + |
| 117 | +**Files:** `docs/OAUTH_SUPPORT.md` |
| 118 | + |
| 119 | +**Description:** Extend the authorized streamable HTTP client example with a focused error-handling snippet: |
| 120 | + |
| 121 | +```rust |
| 122 | +match client_service.serve(transport).await { |
| 123 | + Ok(client) => { |
| 124 | + // use the client |
| 125 | + } |
| 126 | + Err(error) if error.is_authorization_required() => { |
| 127 | + // return to the application's authorization flow |
| 128 | + } |
| 129 | + Err(error) => return Err(error.into()), |
| 130 | +} |
| 131 | +``` |
| 132 | + |
| 133 | +Explain that the predicate covers both missing/expired local authorization and a server 401 challenge, while preserving the original error for logging or detailed handling. |
| 134 | + |
| 135 | +**Acceptance criteria:** |
| 136 | + |
| 137 | +- [x] The example uses only public APIs. |
| 138 | +- [x] The text does not suggest retrying transient failures as re-authorization. |
| 139 | +- [x] The text does not imply that the SDK launches a browser or authorization flow automatically. |
| 140 | + |
| 141 | +### 6. Verify compatibility and quality |
| 142 | + |
| 143 | +**Files:** All files above |
| 144 | + |
| 145 | +**Description:** Run focused tests, the relevant feature-minimal builds, and repository-wide formatting/lint checks. |
| 146 | + |
| 147 | +**Acceptance criteria:** |
| 148 | + |
| 149 | +- [x] `cargo test -p rmcp --test test_auth_error_classification --no-default-features --features "auth,client,transport-streamable-http-client"` passes. |
| 150 | +- [x] `cargo check -p rmcp --no-default-features --features client` passes. |
| 151 | +- [x] `cargo check -p rmcp --no-default-features --features "client,transport-streamable-http-client"` passes. |
| 152 | +- [x] `cargo test -p rmcp --all-features` passes. |
| 153 | +- [x] `cargo clippy -p rmcp --all-targets --all-features -- -D warnings` passes. |
| 154 | +- [x] `cargo fmt --all --check` passes. |
| 155 | +- [x] The public change is additive and does not require a SemVer exemption. |
| 156 | + |
| 157 | +## Open Questions |
| 158 | + |
| 159 | +No question blocks implementation. Before opening the PR, post the proposed additive API on #795 so the reporter and maintainers can confirm the method name. If maintainers want structured detail later, a separate API can return a reason or challenge reference without changing this predicate. |
0 commit comments