Skip to content

Commit 66aac2c

Browse files
feat: add serde_path_to_error for better deserialization error messages (#349)
* fix(redis-enterprise): correct master_persistence type from String to bool The Redis Enterprise API returns master_persistence as a boolean value (false), not a string. This mismatch was causing deserialization errors when fetching database information. Fixes #347 * feat: add serde_path_to_error for better deserialization error messages - Add serde_path_to_error dependency to both redis-cloud and redis-enterprise crates - Update handle_response methods to use serde_path_to_error::deserialize - Provides exact field path in error messages (e.g., 'master_persistence') - Makes debugging type mismatches much easier Example improvement: Before: 'invalid type: string "should-be-bool", expected a boolean' After: 'Failed to deserialize field 'master_persistence': invalid type: string "should-be-bool", expected a boolean' This will help catch field type issues like #347 more quickly in the future.
1 parent 873bf09 commit 66aac2c

File tree

6 files changed

+113
-11
lines changed

6 files changed

+113
-11
lines changed

Cargo.lock

Lines changed: 27 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/redis-cloud/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ async-trait = { workspace = true }
1919
reqwest = { workspace = true }
2020
serde = { workspace = true }
2121
serde_json = { workspace = true }
22+
serde_path_to_error = "0.1"
2223
thiserror = { workspace = true }
2324
tokio = { workspace = true }
2425
tracing = { workspace = true }

crates/redis-cloud/src/client.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -309,15 +309,21 @@ impl CloudClient {
309309
let status = response.status();
310310

311311
if status.is_success() {
312-
// Try to get the response text first for debugging
313-
let text = response.text().await.map_err(|e| {
312+
// Get the response bytes for better error reporting
313+
let bytes = response.bytes().await.map_err(|e| {
314314
RestError::ConnectionError(format!("Failed to read response: {}", e))
315315
})?;
316316

317-
// Try to parse as JSON
318-
serde_json::from_str::<T>(&text).map_err(|e| {
319-
// If parsing fails, include the actual response for debugging
320-
RestError::JsonError(e)
317+
// Use serde_path_to_error for better deserialization error messages
318+
let deserializer = &mut serde_json::Deserializer::from_slice(&bytes);
319+
serde_path_to_error::deserialize(deserializer).map_err(|err| {
320+
let path = err.path().to_string();
321+
// Use ConnectionError to provide detailed error message with field path
322+
RestError::ConnectionError(format!(
323+
"Failed to deserialize field '{}': {}",
324+
path,
325+
err.inner()
326+
))
321327
})
322328
} else {
323329
let text = response.text().await.unwrap_or_default();

crates/redis-enterprise/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ async-trait = { workspace = true }
1919
reqwest = { workspace = true }
2020
serde = { workspace = true }
2121
serde_json = { workspace = true }
22+
serde_path_to_error = "0.1"
2223
serde_urlencoded = { workspace = true }
2324
thiserror = { workspace = true }
2425
tokio = { workspace = true }

crates/redis-enterprise/src/client.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,19 @@ impl EnterpriseClient {
512512
/// Handle HTTP response
513513
async fn handle_response<T: DeserializeOwned>(&self, response: Response) -> Result<T> {
514514
if response.status().is_success() {
515-
response.json::<T>().await.map_err(Into::into)
515+
// Get the response bytes for better error reporting
516+
let bytes = response.bytes().await.map_err(Into::<RestError>::into)?;
517+
518+
// Use serde_path_to_error for better deserialization error messages
519+
let deserializer = &mut serde_json::Deserializer::from_slice(&bytes);
520+
serde_path_to_error::deserialize(deserializer).map_err(|err| {
521+
let path = err.path().to_string();
522+
RestError::ParseError(format!(
523+
"Failed to deserialize field '{}': {}",
524+
path,
525+
err.inner()
526+
))
527+
})
516528
} else {
517529
let status = response.status();
518530
let text = response.text().await.unwrap_or_default();
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
use redis_enterprise::bdb::DatabaseInfo;
2+
3+
#[test]
4+
fn test_serde_path_to_error_improvement() {
5+
// Create a test JSON with incorrect type for master_persistence
6+
// Previously this would give a generic error, now it should show the exact field path
7+
let bad_json = r#"{
8+
"uid": 1,
9+
"name": "test-db",
10+
"type": "redis",
11+
"memory_size": 1073741824,
12+
"port": 12000,
13+
"status": "active",
14+
"master_persistence": "should-be-bool",
15+
"data_persistence": "disabled"
16+
}"#;
17+
18+
// Test with standard serde_json
19+
let standard_result: Result<DatabaseInfo, _> = serde_json::from_str(bad_json);
20+
assert!(standard_result.is_err());
21+
let standard_error = standard_result.unwrap_err();
22+
println!("Standard serde_json error: {}", standard_error);
23+
24+
// Test with serde_path_to_error
25+
let deserializer = &mut serde_json::Deserializer::from_str(bad_json);
26+
let path_result: Result<DatabaseInfo, _> = serde_path_to_error::deserialize(deserializer);
27+
assert!(path_result.is_err());
28+
29+
let path_error = path_result.unwrap_err();
30+
println!("Improved error with path:");
31+
println!(" Field path: {}", path_error.path());
32+
println!(" Error: {}", path_error.inner());
33+
34+
// Verify the path includes "master_persistence"
35+
assert!(path_error.path().to_string().contains("master_persistence"));
36+
}
37+
38+
#[test]
39+
fn test_correct_types_work() {
40+
// Test that correctly typed JSON still works
41+
let good_json = r#"{
42+
"uid": 1,
43+
"name": "test-db",
44+
"type": "redis",
45+
"memory_size": 1073741824,
46+
"port": 12000,
47+
"status": "active",
48+
"master_persistence": false,
49+
"data_persistence": "disabled"
50+
}"#;
51+
52+
let result: Result<DatabaseInfo, _> = serde_json::from_str(good_json);
53+
assert!(result.is_ok());
54+
55+
let db = result.unwrap();
56+
assert_eq!(db.uid, 1);
57+
assert_eq!(db.name, "test-db".to_string());
58+
assert_eq!(db.master_persistence, Some(false));
59+
}

0 commit comments

Comments
 (0)