Repository navigation
Conversation
github-actions
Bot
added
T-core
| assert!( | ||
| matches!( | ||
| error, | ||
| AuthError::ProtectedResourceMetadataInvalid { | ||
| ref url, | ||
| ref reason, | ||
| } if url.as_str() | ||
| == "https://mcp.example.com/.well-known/oauth-protected-resource" | ||
| && matches!( | ||
| reason.as_ref(), | ||
| ProtectedResourceMetadataError::NotAMetadataDocument | ||
| ) | ||
| ), | ||
| "unexpected protected resource metadata error: {error:?}" | ||
| ); |
There was a problem hiding this comment.
Since this PR intentionally changes the text users see in the logs, would it be worth keeping a to_string() assertion alongside each matches!?
| fn validate_resource_metadata_resource( | ||
| &self, | ||
| resource_metadata_url: &Url, | ||
| metadata: &ResourceServerMetadata, | ||
| ) -> Result<(), AuthError> { | ||
| let invalid = |reason| AuthError::ProtectedResourceMetadataInvalid { | ||
| url: resource_metadata_url.clone(), | ||
| reason: Box::new(reason), | ||
| }; |
There was a problem hiding this comment.
Did you consider having the validator return only the reason, then letting the caller add the URL in the Advertised branch, where it's actually needed?
| /// Why a protected resource metadata document cannot be used for this resource. | ||
| #[derive(Debug, Clone, PartialEq, Eq, Error)] | ||
| #[non_exhaustive] | ||
| pub enum ProtectedResourceMetadataError { |
There was a problem hiding this comment.
AuthError and the other auth types are re-exported from rmcp::transport, but the new reason enum is only available as rmcp::transport::auth::ProtectedResourceMetadataError. So callers matching on reason have to import from two different paths.
| ResourceHasFragment { resource: Url }, | ||
|
|
||
| /// The advertised resource identifier does not identify the requested resource. | ||
| #[error("resource mismatch: reference '{expected}', permitted '{actual}'")] |
There was a problem hiding this comment.
The fields were renamed to expected and actual, but the message still says reference and permitted. Using the same terms in both places makes the log line consistent with the fields.
| #[error("resource mismatch: reference '{expected}', permitted '{actual}'")] | |
| #[error("resource mismatch: expected '{expected}', got '{actual}'")] |
This branch has not been deployed
2 participants
Summary
MetadataError(String)with structuredAuthErrorvariantssourceAuthErrorbelow Clippy'sresult_large_errthresholdTesting
cargo test --all-featurescargo clippy --all-targets --all-features -- -D warningsFixes #1265