Skip to content

Commit 4b2c6c5

Browse files
committed
feat: make error types Clone for tower-resilience compatibility
Make CloudError and RestError Clone by converting non-cloneable error types (reqwest::Error and serde_json::Error) to String representations. Changes: - Add Clone derive to CloudError and RestError - Change Request/RequestFailed variants from reqwest::Error to String - Change JsonError/SerializationError variants from serde_json::Error to String - Add manual From<reqwest::Error> and From<serde_json::Error> implementations - Fix manual error construction sites to use .to_string() Benefits: - Enables tower-resilience middleware (retry, circuit breaker) integration - Preserves full error messages for debugging - No breaking changes to public API - All existing tests pass This allows redisctl users to compose CloudClient and EnterpriseClient with tower-resilience patterns that require Clone errors, improving the developer experience for both crates.
1 parent 4ba7817 commit 4b2c6c5

File tree

6 files changed

+144
-22
lines changed

6 files changed

+144
-22
lines changed

Cargo.lock

Lines changed: 41 additions & 7 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
@@ -39,3 +39,4 @@ tokio = { workspace = true, features = ["rt", "macros", "test-util"] }
3939
pretty_assertions = { workspace = true }
4040
serial_test = { workspace = true }
4141
tower = { version = "0.5", features = ["timeout", "limit", "retry", "buffer"] }
42+
tower-resilience = { version = "0.3.8", features = ["retry"] }

crates/redis-cloud/src/lib.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -340,10 +340,10 @@ pub use users::UsersHandler as UserHandler;
340340
// Re-export error types
341341
use thiserror::Error;
342342

343-
#[derive(Error, Debug)]
343+
#[derive(Error, Debug, Clone)]
344344
pub enum CloudError {
345345
#[error("HTTP request failed: {0}")]
346-
Request(#[from] reqwest::Error),
346+
Request(String),
347347

348348
#[error("Bad Request (400): {message}")]
349349
BadRequest { message: String },
@@ -373,7 +373,19 @@ pub enum CloudError {
373373
ConnectionError(String),
374374

375375
#[error("JSON error: {0}")]
376-
JsonError(#[from] serde_json::Error),
376+
JsonError(String),
377+
}
378+
379+
impl From<reqwest::Error> for CloudError {
380+
fn from(err: reqwest::Error) -> Self {
381+
CloudError::Request(err.to_string())
382+
}
383+
}
384+
385+
impl From<serde_json::Error> for CloudError {
386+
fn from(err: serde_json::Error) -> Self {
387+
CloudError::JsonError(err.to_string())
388+
}
377389
}
378390

379391
pub type Result<T> = std::result::Result<T, CloudError>;
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
//! Test for tower-resilience retry integration
2+
//!
3+
//! This test verifies that CloudClient works with tower-resilience retry middleware
4+
//! now that CloudError implements Clone.
5+
6+
#![cfg(feature = "tower-integration")]
7+
8+
use redis_cloud::CloudClient;
9+
use redis_cloud::tower_support::ApiRequest;
10+
use serde_json::json;
11+
use std::sync::Arc;
12+
use std::sync::atomic::{AtomicU32, Ordering};
13+
use std::time::Duration;
14+
use tower::{ServiceBuilder, ServiceExt};
15+
use tower_resilience::retry::RetryLayer;
16+
use wiremock::matchers::{method, path};
17+
use wiremock::{Mock, MockServer, ResponseTemplate};
18+
19+
#[tokio::test]
20+
async fn test_tower_resilience_retry_with_cloneable_error() {
21+
let mock_server = MockServer::start().await;
22+
let counter = Arc::new(AtomicU32::new(0));
23+
let counter_clone = counter.clone();
24+
25+
// Mock endpoint that fails twice then succeeds
26+
Mock::given(method("GET"))
27+
.and(path("/subscriptions"))
28+
.respond_with(move |_req: &wiremock::Request| {
29+
let count = counter_clone.fetch_add(1, Ordering::SeqCst);
30+
if count < 2 {
31+
// First two requests fail with 503
32+
ResponseTemplate::new(503).set_body_json(json!({
33+
"error": "Service temporarily unavailable"
34+
}))
35+
} else {
36+
// Third request succeeds
37+
ResponseTemplate::new(200).set_body_json(json!({"subscriptions": []}))
38+
}
39+
})
40+
.mount(&mock_server)
41+
.await;
42+
43+
let client = CloudClient::builder()
44+
.api_key("test-key")
45+
.api_secret("test-secret")
46+
.base_url(mock_server.uri())
47+
.build()
48+
.expect("Failed to create client");
49+
50+
// Build retry layer with tower-resilience - now works because CloudError is Clone!
51+
let retry_layer = RetryLayer::<redis_cloud::CloudError>::builder()
52+
.name("cloud-api-retry")
53+
.max_attempts(3)
54+
.exponential_backoff(Duration::from_millis(10))
55+
.build();
56+
57+
let service = ServiceBuilder::new()
58+
.layer(retry_layer)
59+
.service(client.into_service());
60+
61+
let response = service
62+
.oneshot(ApiRequest::get("/subscriptions"))
63+
.await
64+
.expect("Should succeed after retries");
65+
66+
assert_eq!(response.status, 200);
67+
// Should have made 3 attempts (2 failures + 1 success)
68+
assert_eq!(counter.load(Ordering::SeqCst), 3);
69+
}

crates/redis-enterprise/src/client.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,7 @@ impl EnterpriseClient {
184184
trace!("Response status: {}", response.status());
185185

186186
if response.status().is_success() {
187-
let text = response
188-
.text()
189-
.await
190-
.map_err(crate::error::RestError::RequestFailed)?;
187+
let text = response.text().await?;
191188
Ok(text)
192189
} else {
193190
let status = response.status();
@@ -222,10 +219,7 @@ impl EnterpriseClient {
222219
);
223220

224221
if response.status().is_success() {
225-
let bytes = response
226-
.bytes()
227-
.await
228-
.map_err(crate::error::RestError::RequestFailed)?;
222+
let bytes = response.bytes().await?;
229223
Ok(bytes.to_vec())
230224
} else {
231225
let status = response.status();
@@ -505,7 +499,7 @@ impl EnterpriseClient {
505499
url, error
506500
))
507501
} else {
508-
RestError::RequestFailed(error)
502+
RestError::RequestFailed(error.to_string())
509503
}
510504
}
511505

crates/redis-enterprise/src/error.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
33
use thiserror::Error;
44

5-
#[derive(Error, Debug)]
5+
#[derive(Error, Debug, Clone)]
66
pub enum RestError {
77
#[error("Invalid URL: {0}")]
88
InvalidUrl(String),
99

1010
#[error("HTTP request failed: {0}")]
11-
RequestFailed(#[from] reqwest::Error),
11+
RequestFailed(String),
1212

1313
#[error("Authentication failed")]
1414
AuthenticationFailed,
@@ -17,7 +17,7 @@ pub enum RestError {
1717
ApiError { code: u16, message: String },
1818

1919
#[error("Serialization error: {0}")]
20-
SerializationError(#[from] serde_json::Error),
20+
SerializationError(String),
2121

2222
#[error("Parse error: {0}")]
2323
ParseError(String),
@@ -41,6 +41,18 @@ pub enum RestError {
4141
ServerError(String),
4242
}
4343

44+
impl From<reqwest::Error> for RestError {
45+
fn from(err: reqwest::Error) -> Self {
46+
RestError::RequestFailed(err.to_string())
47+
}
48+
}
49+
50+
impl From<serde_json::Error> for RestError {
51+
fn from(err: serde_json::Error) -> Self {
52+
RestError::SerializationError(err.to_string())
53+
}
54+
}
55+
4456
impl RestError {
4557
/// Check if this is a not found error
4658
pub fn is_not_found(&self) -> bool {

0 commit comments

Comments
 (0)