Utilize reqwest::retry::Builder to implement HTTP retries

We here make use of `reqwest`'s `retry` functionlity that was introduced
with its recent v0.12.23 release. As we can't fully replace the old
'application-level' behavior just yet, we configure it to only apply for
error that arent INTERNAL_SERVER_ERROR, which is the status VSS uses to
send error responses.

We set some default values here, but note that these can always can be
overridden by the user when using the `from_client` constructor.

Author: tnull

Cargo.toml

@@ -18,7 +18,7 @@ lnurl-auth = ["dep:bitcoin", "dep:url", "dep:serde", "dep:serde_json", "reqwest/
 
 [dependencies]
 prost = "0.11.6"
-reqwest = { version = "0.12", default-features = false, features = ["rustls-tls"] }
+reqwest = { version = "0.12.23", default-features = false, features = ["rustls-tls"] }
 tokio = { version = "1", default-features = false, features = ["time"] }
 rand = "0.8.5"
 async-trait = "0.1.77"
@@ -33,7 +33,7 @@ chacha20-poly1305 = "0.1.2"
 
 [target.'cfg(genproto)'.build-dependencies]
 prost-build = { version = "0.11.3" }
-reqwest =  { version = "0.12", default-features = false, features = ["rustls-tls", "blocking"] }
+reqwest =  { version = "0.12.23", default-features = false, features = ["rustls-tls", "blocking"] }
 
 [dev-dependencies]
 mockito = "0.28.0"

src/client.rs

@@ -15,6 +15,7 @@ use crate::util::retry::{retry, RetryPolicy};
 
 const APPLICATION_OCTET_STREAM: &str = "application/octet-stream";
 const DEFAULT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
+const DEFAULT_RETRIES: u32 = 2;
 
 /// Thin-client to access a hosted instance of Versioned Storage Service (VSS).
 /// The provided [`VssClient`] API is minimalistic and is congruent to the VSS server-side API.
@@ -31,9 +32,9 @@ where
 
 impl<R: RetryPolicy<E = VssError>> VssClient<R> {
 	/// Constructs a [`VssClient`] using `base_url` as the VSS server endpoint.
-	pub fn new(base_url: String, retry_policy: R) -> Self {
-		let client = build_client();
-		Self::from_client(base_url, client, retry_policy)
+	pub fn new(base_url: String, retry_policy: R) -> Result<Self, VssError> {
+		let client = build_client(&base_url)?;
+		Ok(Self::from_client(base_url, client, retry_policy))
 	}
 
 	/// Constructs a [`VssClient`] from a given [`reqwest::Client`], using `base_url` as the VSS server endpoint.
@@ -51,9 +52,9 @@ impl<R: RetryPolicy<E = VssError>> VssClient<R> {
 	/// HTTP headers will be provided by the given `header_provider`.
 	pub fn new_with_headers(
 		base_url: String, retry_policy: R, header_provider: Arc<dyn VssHeaderProvider>,
-	) -> Self {
-		let client = build_client();
-		Self { base_url, client, retry_policy, header_provider }
+	) -> Result<Self, VssError> {
+		let client = build_client(&base_url)?;
+		Ok(Self { base_url, client, retry_policy, header_provider })
 	}
 
 	/// Returns the underlying base URL.
@@ -164,11 +165,30 @@ impl<R: RetryPolicy<E = VssError>> VssClient<R> {
 	}
 }
 
-fn build_client() -> Client {
-	Client::builder()
+fn build_client(base_url: &str) -> Result<Client, VssError> {
+	let url = reqwest::Url::parse(base_url).map_err(|_| VssError::InvalidUrlError)?;
+	let host_str = url.host_str().ok_or(VssError::InvalidUrlError)?.to_string();
+	// TODO: Once the response `payload` is available in `classify_fn`, we should filter out any
+	// error types for which retrying doesn't make sense (i.e., `NoSuchKeyError`,
+	// `InvalidRequestError`,`ConflictError`).
+	let retry = reqwest::retry::for_host(host_str)
+		.max_retries_per_request(DEFAULT_RETRIES)
+		.classify_fn(|req_rep| match req_rep.status() {
+			// VSS uses INTERNAL_SERVER_ERROR when sending back error repsonses. These are
+			// currently still covered by our `RetryPolicy`, so we tell `reqwest` not to retry them.
+			Some(reqwest::StatusCode::INTERNAL_SERVER_ERROR) => req_rep.success(),
+			Some(reqwest::StatusCode::BAD_REQUEST) => req_rep.success(),
+			Some(reqwest::StatusCode::UNAUTHORIZED) => req_rep.success(),
+			Some(reqwest::StatusCode::CONFLICT) => req_rep.success(),
+			Some(reqwest::StatusCode::OK) => req_rep.success(),
+			_ => req_rep.retryable(),
+		});
+	let client = Client::builder()
 		.timeout(DEFAULT_TIMEOUT)
 		.connect_timeout(DEFAULT_TIMEOUT)
 		.read_timeout(DEFAULT_TIMEOUT)
+		.retry(retry)
 		.build()
-		.unwrap()
+		.unwrap();
+	Ok(client)
 }

src/error.rs

@@ -28,6 +28,9 @@ pub enum VssError {
 	/// There is an unknown error, it could be a client-side bug, unrecognized error-code, network error
 	/// or something else.
 	InternalError(String),
+
+	/// The provided URL is invalid.
+	InvalidUrlError,
 }
 
 impl VssError {
@@ -67,6 +70,9 @@ impl Display for VssError {
 			VssError::InternalError(message) => {
 				write!(f, "InternalError: {}", message)
 			},
+			VssError::InvalidUrlError => {
+				write!(f, "The provided URL is invalid")
+			},
 		}
 	}
 }

tests/tests.rs

@@ -48,7 +48,7 @@ mod tests {
 			.create();
 
 		// Create a new VssClient with the mock server URL.
-		let client = VssClient::new(base_url, retry_policy());
+		let client = VssClient::new(base_url, retry_policy()).unwrap();
 
 		let actual_result = client.get_object(&get_request).await.unwrap();
 
@@ -85,7 +85,8 @@ mod tests {
 			"headerkey".to_string(),
 			"headervalue".to_string(),
 		)])));
-		let client = VssClient::new_with_headers(base_url, retry_policy(), header_provider);
+		let client =
+			VssClient::new_with_headers(base_url, retry_policy(), header_provider).unwrap();
 
 		let actual_result = client.get_object(&get_request).await.unwrap();
 
@@ -123,7 +124,7 @@ mod tests {
 			.create();
 
 		// Create a new VssClient with the mock server URL.
-		let vss_client = VssClient::new(base_url, retry_policy());
+		let vss_client = VssClient::new(base_url, retry_policy()).unwrap();
 		let actual_result = vss_client.put_object(&request).await.unwrap();
 
 		let expected_result = &mock_response;
@@ -158,7 +159,7 @@ mod tests {
 			.create();
 
 		// Create a new VssClient with the mock server URL.
-		let vss_client = VssClient::new(base_url, retry_policy());
+		let vss_client = VssClient::new(base_url, retry_policy()).unwrap();
 		let actual_result = vss_client.delete_object(&request).await.unwrap();
 
 		let expected_result = &mock_response;
@@ -199,7 +200,7 @@ mod tests {
 			.create();
 
 		// Create a new VssClient with the mock server URL.
-		let client = VssClient::new(base_url, retry_policy());
+		let client = VssClient::new(base_url, retry_policy()).unwrap();
 
 		let actual_result = client.list_key_versions(&request).await.unwrap();
 
@@ -213,7 +214,7 @@ mod tests {
 	#[tokio::test]
 	async fn test_no_such_key_err_handling() {
 		let base_url = mockito::server_url();
-		let vss_client = VssClient::new(base_url, retry_policy());
+		let vss_client = VssClient::new(base_url, retry_policy()).unwrap();
 
 		// NoSuchKeyError
 		let error_response = ErrorResponse {
@@ -240,7 +241,7 @@ mod tests {
 	#[tokio::test]
 	async fn test_get_response_without_value() {
 		let base_url = mockito::server_url();
-		let vss_client = VssClient::new(base_url, retry_policy());
+		let vss_client = VssClient::new(base_url, retry_policy()).unwrap();
 
 		// GetObjectResponse with None value
 		let mock_response = GetObjectResponse { value: None, ..Default::default() };
@@ -261,7 +262,7 @@ mod tests {
 	#[tokio::test]
 	async fn test_invalid_request_err_handling() {
 		let base_url = mockito::server_url();
-		let vss_client = VssClient::new(base_url, retry_policy());
+		let vss_client = VssClient::new(base_url, retry_policy()).unwrap();
 
 		// Invalid Request Error
 		let error_response = ErrorResponse {
@@ -321,7 +322,7 @@ mod tests {
 	#[tokio::test]
 	async fn test_auth_err_handling() {
 		let base_url = mockito::server_url();
-		let vss_client = VssClient::new(base_url, retry_policy());
+		let vss_client = VssClient::new(base_url, retry_policy()).unwrap();
 
 		// Invalid Request Error
 		let error_response = ErrorResponse {
@@ -393,8 +394,12 @@ mod tests {
 	async fn test_header_provider_error() {
 		let get_request = GetObjectRequest { store_id: "store".to_string(), key: "k1".to_string() };
 		let header_provider = Arc::new(FailingHeaderProvider {});
-		let client =
-			VssClient::new_with_headers("notused".to_string(), retry_policy(), header_provider);
+		let client = VssClient::new_with_headers(
+			"http://localhost/vss".to_string(),
+			retry_policy(),
+			header_provider,
+		)
+		.unwrap();
 		let result = client.get_object(&get_request).await;
 
 		assert!(matches!(result, Err(VssError::AuthError { .. })));
@@ -403,7 +408,7 @@ mod tests {
 	#[tokio::test]
 	async fn test_conflict_err_handling() {
 		let base_url = mockito::server_url();
-		let vss_client = VssClient::new(base_url, retry_policy());
+		let vss_client = VssClient::new(base_url, retry_policy()).unwrap();
 
 		// Conflict Error
 		let error_response = ErrorResponse {
@@ -436,7 +441,7 @@ mod tests {
 	#[tokio::test]
 	async fn test_internal_server_err_handling() {
 		let base_url = mockito::server_url();
-		let vss_client = VssClient::new(base_url, retry_policy());
+		let vss_client = VssClient::new(base_url, retry_policy()).unwrap();
 
 		// Internal Server Error
 		let error_response = ErrorResponse {
@@ -496,7 +501,7 @@ mod tests {
 	#[tokio::test]
 	async fn test_internal_err_handling() {
 		let base_url = mockito::server_url();
-		let vss_client = VssClient::new(base_url, retry_policy());
+		let vss_client = VssClient::new(base_url, retry_policy()).unwrap();
 
 		let error_response =
 			ErrorResponse { error_code: 999, message: "UnknownException".to_string() };