Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ErrorKind::DeserializeError to specialize ErrorKind::Message (extract::path::ErrorKind) #2720

Merged
merged 4 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion axum-extra/src/extract/optional_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ mod tests {
let res = client.get("/NaN").await;
assert_eq!(
res.text().await,
"Invalid URL: Cannot parse `\"NaN\"` to a `u32`"
"Invalid URL: Cannot parse `NaN` to a `u32`"
);
}
}
4 changes: 4 additions & 0 deletions axum/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
This allows middleware to add bodies to requests without needing to manually set `content-length` ([#2897])
- **breaking:** Remove `WebSocket::close`.
Users should explicitly send close messages themselves. ([#2974])
- **added:** Extend `FailedToDeserializePathParams::kind` enum with (`ErrorKind::DeserializeError`)
This new variant captures both `key`, `value`, and `message` from named path parameters parse errors,
instead of only deserialization error message in `ErrorKind::Message`. ([#2720])

[#2897]: https://github.com/tokio-rs/axum/pull/2897
[#2903]: https://github.com/tokio-rs/axum/pull/2903
Expand All @@ -28,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#2974]: https://github.com/tokio-rs/axum/pull/2974
[#2978]: https://github.com/tokio-rs/axum/pull/2978
[#2992]: https://github.com/tokio-rs/axum/pull/2992
[#2720]: https://github.com/tokio-rs/axum/pull/2720

# 0.8.0

Expand Down
52 changes: 50 additions & 2 deletions axum/src/extract/path/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,21 @@ impl<'de> Deserializer<'de> for PathDeserializer<'de> {
.got(self.url_params.len())
.expected(1));
}
visitor.visit_borrowed_str(&self.url_params[0].1)
let key = &self.url_params[0].0;
let value = &self.url_params[0].1;
visitor
.visit_borrowed_str(value)
.map_err(|e: PathDeserializationError| {
if let ErrorKind::Message(message) = &e.kind {
PathDeserializationError::new(ErrorKind::DeserializeError {
key: key.to_string(),
value: value.as_str().to_owned(),
message: message.to_owned(),
})
} else {
e
}
})
}

fn deserialize_unit<V>(self, visitor: V) -> Result<V::Value, Self::Error>
Expand Down Expand Up @@ -362,7 +376,19 @@ impl<'de> Deserializer<'de> for ValueDeserializer<'de> {
where
V: Visitor<'de>,
{
visitor.visit_borrowed_str(self.value)
visitor
.visit_borrowed_str(self.value)
.map_err(|e: PathDeserializationError| {
if let (ErrorKind::Message(message), Some(key)) = (&e.kind, self.key.as_ref()) {
PathDeserializationError::new(ErrorKind::DeserializeError {
key: key.key().to_owned(),
value: self.value.as_str().to_owned(),
message: message.to_owned(),
})
} else {
e
}
})
}

fn deserialize_bytes<V>(self, visitor: V) -> Result<V::Value, Self::Error>
Expand Down Expand Up @@ -608,6 +634,15 @@ enum KeyOrIdx<'de> {
Idx { idx: usize, key: &'de str },
}

impl<'de> KeyOrIdx<'de> {
fn key(&self) -> &'de str {
match &self {
Self::Key(key) => key,
Self::Idx { key, .. } => key,
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -926,4 +961,17 @@ mod tests {
}
);
}

#[test]
fn test_deserialize_key_value() {
test_parse_error!(
vec![("id", "123123-123-123123")],
uuid::Uuid,
ErrorKind::DeserializeError {
key: "id".to_owned(),
value: "123123-123-123123".to_owned(),
message: "UUID parsing failed: invalid group count: expected 5, found 3".to_owned(),
}
);
}
}
62 changes: 59 additions & 3 deletions axum/src/extract/path/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,16 @@ pub enum ErrorKind {
name: &'static str,
},

/// Failed to deserialize the value with a custom deserialization error.
DeserializeError {
/// The key at which the invalid value was located.
key: String,
/// The value that failed to deserialize.
value: String,
/// The deserializaation failure message.
message: String,
},

/// Catch-all variant for errors that don't fit any other variant.
Message(String),
}
Expand Down Expand Up @@ -331,20 +341,25 @@ impl fmt::Display for ErrorKind {
expected_type,
} => write!(
f,
"Cannot parse `{key}` with value `{value:?}` to a `{expected_type}`"
"Cannot parse `{key}` with value `{value}` to a `{expected_type}`"
),
ErrorKind::ParseError {
value,
expected_type,
} => write!(f, "Cannot parse `{value:?}` to a `{expected_type}`"),
} => write!(f, "Cannot parse `{value}` to a `{expected_type}`"),
ErrorKind::ParseErrorAtIndex {
index,
value,
expected_type,
} => write!(
f,
"Cannot parse value at index {index} with value `{value:?}` to a `{expected_type}`"
"Cannot parse value at index {index} with value `{value}` to a `{expected_type}`"
),
ErrorKind::DeserializeError {
key,
value,
message,
} => write!(f, "Cannot parse `{key}` with value `{value}`: {message}"),
}
}
}
Expand All @@ -369,6 +384,7 @@ impl FailedToDeserializePathParams {
pub fn body_text(&self) -> String {
match self.0.kind {
ErrorKind::Message(_)
| ErrorKind::DeserializeError { .. }
| ErrorKind::InvalidUtf8InPathParam { .. }
| ErrorKind::ParseError { .. }
| ErrorKind::ParseErrorAtIndex { .. }
Expand All @@ -383,6 +399,7 @@ impl FailedToDeserializePathParams {
pub fn status(&self) -> StatusCode {
match self.0.kind {
ErrorKind::Message(_)
| ErrorKind::DeserializeError { .. }
| ErrorKind::InvalidUtf8InPathParam { .. }
| ErrorKind::ParseError { .. }
| ErrorKind::ParseErrorAtIndex { .. }
Expand Down Expand Up @@ -917,4 +934,43 @@ mod tests {
let body = res.text().await;
assert_eq!(body, "a=foo b=bar c=baz");
}

#[crate::test]
async fn deserialize_error_single_value() {
let app = Router::new().route(
"/resources/{res}",
get(|res: Path<uuid::Uuid>| async move {
let _res = res;
}),
);

let client = TestClient::new(app);
let res = client.get("/resources/123123-123-123123").await;
let body = res.text().await;
assert_eq!(
body,
r#"Invalid URL: Cannot parse `res` with value `123123-123-123123`: UUID parsing failed: invalid group count: expected 5, found 3"#
);
}

#[crate::test]
async fn deserialize_error_multi_value() {
let app = Router::new().route(
"/resources/{res}/sub/{sub}",
get(
|Path((res, sub)): Path<(uuid::Uuid, uuid::Uuid)>| async move {
let _res = res;
let _sub = sub;
},
),
);

let client = TestClient::new(app);
let res = client.get("/resources/456456-123-456456/sub/123").await;
let body = res.text().await;
assert_eq!(
body,
r#"Invalid URL: Cannot parse `res` with value `456456-123-456456`: UUID parsing failed: invalid group count: expected 5, found 3"#
);
}
}