|
| 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:** In progress |
| 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 proposed additive API is: |
| 20 | + |
| 21 | +```rust |
| 22 | +impl DynamicTransportError { |
| 23 | + pub fn is_authorization_required(&self) -> bool; |
| 24 | +} |
| 25 | + |
| 26 | +impl ClientInitializeError { |
| 27 | + pub fn is_authorization_required(&self) -> bool; |
| 28 | +} |
| 29 | +``` |
| 30 | + |
| 31 | +`DynamicTransportError` will inspect the standard `std::error::Error::source` chain. `ClientInitializeError` will delegate for its `TransportError` variant and return `false` for all other variants. |
| 32 | + |
| 33 | +`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`. |
| 34 | + |
| 35 | +## Scope |
| 36 | + |
| 37 | +- Detect both authorization-required paths through one public predicate. |
| 38 | +- Work for direct and wrapped transport errors without requiring the caller to name the HTTP backend's error type. |
| 39 | +- Preserve existing public variants and challenge data. |
| 40 | +- Document the re-authorization pattern. |
| 41 | + |
| 42 | +## Non-goals |
| 43 | + |
| 44 | +- Do not merge or remove `AuthError::AuthorizationRequired` and `StreamableHttpError::AuthRequired`. |
| 45 | +- Do not classify `InsufficientScope`, transient refresh failures, session expiry, or arbitrary HTTP failures as authorization-required. |
| 46 | +- Do not automatically start an OAuth flow. |
| 47 | +- Do not add a generalized error-category framework or change the `Transport` trait. |
| 48 | +- Do not add the predicate to `RmcpError` until a caller demonstrates that the broader wrapper needs it. |
| 49 | + |
| 50 | +## File Map |
| 51 | + |
| 52 | +| File | Role | Status | |
| 53 | +| --- | --- | --- | |
| 54 | +| `crates/rmcp/src/transport/streamable_http_client.rs` | Make the HTTP 401 detail error visible through the standard source chain | Modify | |
| 55 | +| `crates/rmcp/src/transport.rs` | Add transport-independent authorization-required classification | Modify | |
| 56 | +| `crates/rmcp/src/service/client.rs` | Add the initialization-error convenience predicate | Modify | |
| 57 | +| `crates/rmcp/tests/test_auth_error_classification.rs` | Exercise the public API and guard false positives | New | |
| 58 | +| `crates/rmcp/Cargo.toml` | Declare the feature-gated integration test | Modify | |
| 59 | +| `docs/OAUTH_SUPPORT.md` | Show callers how to branch back into authorization | Modify | |
| 60 | + |
| 61 | +## Tasks |
| 62 | + |
| 63 | +### 1. Add public-API regression tests |
| 64 | + |
| 65 | +**Files:** `crates/rmcp/tests/test_auth_error_classification.rs`, `crates/rmcp/Cargo.toml` |
| 66 | + |
| 67 | +**Description:** Add a feature-gated integration test that constructs public error values through `DynamicTransportError::from_parts` and verifies the intended classification before implementation. |
| 68 | + |
| 69 | +**Acceptance criteria:** |
| 70 | + |
| 71 | +- [x] `StreamableHttpError::Auth(AuthError::AuthorizationRequired)` classifies as authorization-required. |
| 72 | +- [x] `StreamableHttpError::AuthRequired(AuthRequiredError)` classifies as authorization-required. |
| 73 | +- [x] A `ClientInitializeError::TransportError` wrapping either path classifies as authorization-required. |
| 74 | +- [x] An unrelated `StreamableHttpError` classifies as false. |
| 75 | +- [x] `AuthError::TokenRefreshFailed` and `StreamableHttpError::InsufficientScope` classify as false. |
| 76 | +- [x] Non-transport `ClientInitializeError` variants classify as false. |
| 77 | +- [x] The test requires `auth`, `client`, and `transport-streamable-http-client`, but not a concrete reqwest or Unix-socket backend. |
| 78 | + |
| 79 | +### 2. Expose the HTTP challenge through the error source chain |
| 80 | + |
| 81 | +**Files:** `crates/rmcp/src/transport/streamable_http_client.rs` |
| 82 | + |
| 83 | +**Description:** Derive or implement `std::error::Error` for `AuthRequiredError` with a concise display message, then mark the value inside `StreamableHttpError::AuthRequired` as its source. |
| 84 | + |
| 85 | +**Acceptance criteria:** |
| 86 | + |
| 87 | +- [x] `AuthRequiredError` remains constructible through `AuthRequiredError::new`. |
| 88 | +- [x] `www_authenticate_header` remains available and unchanged. |
| 89 | +- [x] `StreamableHttpError::source()` returns the contained `AuthRequiredError`. |
| 90 | +- [x] Error display does not print credentials or the challenge header. |
| 91 | +- [x] No existing enum variants or fields are removed or renamed. |
| 92 | + |
| 93 | +### 3. Add transport-independent classification |
| 94 | + |
| 95 | +**Files:** `crates/rmcp/src/transport.rs` |
| 96 | + |
| 97 | +**Description:** Add `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. |
| 98 | + |
| 99 | +**Acceptance criteria:** |
| 100 | + |
| 101 | +- [x] Callers do not need the transport type, backend error type, or a downcast. |
| 102 | +- [x] Direct and multiply wrapped matching errors are recognized. |
| 103 | +- [ ] Feature-disabled builds compile; feature-specific checks are guarded locally. |
| 104 | +- [x] Unknown custom transport errors return false. |
| 105 | +- [x] The method is documented as a classification predicate, not an authorization action. |
| 106 | + |
| 107 | +### 4. Add the initialization-error convenience API |
| 108 | + |
| 109 | +**Files:** `crates/rmcp/src/service/client.rs` |
| 110 | + |
| 111 | +**Description:** Add `ClientInitializeError::is_authorization_required()`, delegating only when the variant is `TransportError`. |
| 112 | + |
| 113 | +**Acceptance criteria:** |
| 114 | + |
| 115 | +- [x] The issue's caller can replace its downcast helper with `error.is_authorization_required()`. |
| 116 | +- [x] JSON-RPC, protocol negotiation, connection-closed, and cancellation failures return false. |
| 117 | +- [ ] The method remains available consistently across client builds. |
| 118 | + |
| 119 | +### 5. Document the recovery pattern |
| 120 | + |
| 121 | +**Files:** `docs/OAUTH_SUPPORT.md` |
| 122 | + |
| 123 | +**Description:** Extend the authorized streamable HTTP client example with a focused error-handling snippet: |
| 124 | + |
| 125 | +```rust |
| 126 | +match client_service.serve(transport).await { |
| 127 | + Ok(client) => { |
| 128 | + // use the client |
| 129 | + } |
| 130 | + Err(error) if error.is_authorization_required() => { |
| 131 | + // return to the application's authorization flow |
| 132 | + } |
| 133 | + Err(error) => return Err(error.into()), |
| 134 | +} |
| 135 | +``` |
| 136 | + |
| 137 | +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. |
| 138 | + |
| 139 | +**Acceptance criteria:** |
| 140 | + |
| 141 | +- [x] The example uses only public APIs. |
| 142 | +- [x] The text does not suggest retrying transient failures as re-authorization. |
| 143 | +- [x] The text does not imply that the SDK launches a browser or authorization flow automatically. |
| 144 | + |
| 145 | +### 6. Verify compatibility and quality |
| 146 | + |
| 147 | +**Files:** All files above |
| 148 | + |
| 149 | +**Description:** Run focused tests, the relevant feature-minimal builds, and repository-wide formatting/lint checks. |
| 150 | + |
| 151 | +**Acceptance criteria:** |
| 152 | + |
| 153 | +- [ ] `cargo test -p rmcp --test test_auth_error_classification --no-default-features --features "auth,client,transport-streamable-http-client"` passes. |
| 154 | +- [ ] `cargo check -p rmcp --no-default-features --features client` passes. |
| 155 | +- [ ] `cargo check -p rmcp --no-default-features --features "client,transport-streamable-http-client"` passes. |
| 156 | +- [ ] `cargo test -p rmcp --all-features` passes. |
| 157 | +- [ ] `cargo clippy -p rmcp --all-targets --all-features -- -D warnings` passes. |
| 158 | +- [ ] `cargo fmt --all --check` passes. |
| 159 | +- [ ] The public change is additive and does not require a SemVer exemption. |
| 160 | + |
| 161 | +## Open Questions |
| 162 | + |
| 163 | +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