Utilize reqwest::retry::Builder to implement retries

To replace our ~broken homegrown retry logic, we here make use of
`reqwest`'s `retry` functionlity that was introduced with its recent
v0.12.23 release. While we can't reproduce our full functionlity from
before (namely not retrying some error variants), this will probably
still work better as our previous version that did always block on the
first request. We set some default values here (intended for the main
use in LDK Node), 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

@@ -13,6 +13,7 @@ use crate::types::{
 };
 const APPLICATION_OCTET_STREAM: &str = "application/octet-stream";
 const DEFAULT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
+const DEFAULT_RETRIES: u32 = 10;
 
 /// 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.
@@ -25,9 +26,9 @@ pub struct VssClient {
 
 impl VssClient {
 	/// Constructs a [`VssClient`] using `base_url` as the VSS server endpoint.
-	pub fn new(base_url: String) -> Self {
-		let client = build_client();
-		Self::from_client(base_url, client)
+	pub fn new(base_url: String) -> Result<Self, VssError> {
+		let client = build_client(&base_url)?;
+		Ok(Self::from_client(base_url, client))
 	}
 
 	/// Constructs a [`VssClient`] from a given [`reqwest::Client`], using `base_url` as the VSS server endpoint.
@@ -38,9 +39,11 @@ impl VssClient {
 	/// Constructs a [`VssClient`] using `base_url` as the VSS server endpoint.
 	///
 	/// HTTP headers will be provided by the given `header_provider`.
-	pub fn new_with_headers(base_url: String, header_provider: Arc<dyn VssHeaderProvider>) -> Self {
-		let client = build_client();
-		Self { base_url, client, header_provider }
+	pub fn new_with_headers(
+		base_url: String, header_provider: Arc<dyn VssHeaderProvider>,
+	) -> Result<Self, VssError> {
+		let client = build_client(&base_url)?;
+		Ok(Self { base_url, client, header_provider })
 	}
 
 	/// Returns the underlying base URL.
@@ -128,11 +131,19 @@ impl VssClient {
 	}
 }
 
-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);
+	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

@@ -45,7 +45,7 @@ mod tests {
 			.create();
 
 		// Create a new VssClient with the mock server URL.
-		let client = VssClient::new(base_url);
+		let client = VssClient::new(base_url).unwrap();
 
 		let actual_result = client.get_object(&get_request).await.unwrap();
 
@@ -82,7 +82,7 @@ mod tests {
 			"headerkey".to_string(),
 			"headervalue".to_string(),
 		)])));
-		let client = VssClient::new_with_headers(base_url, header_provider);
+		let client = VssClient::new_with_headers(base_url, header_provider).unwrap();
 
 		let actual_result = client.get_object(&get_request).await.unwrap();
 
@@ -120,7 +120,7 @@ mod tests {
 			.create();
 
 		// Create a new VssClient with the mock server URL.
-		let vss_client = VssClient::new(base_url);
+		let vss_client = VssClient::new(base_url).unwrap();
 		let actual_result = vss_client.put_object(&request).await.unwrap();
 
 		let expected_result = &mock_response;
@@ -155,7 +155,7 @@ mod tests {
 			.create();
 
 		// Create a new VssClient with the mock server URL.
-		let vss_client = VssClient::new(base_url);
+		let vss_client = VssClient::new(base_url).unwrap();
 		let actual_result = vss_client.delete_object(&request).await.unwrap();
 
 		let expected_result = &mock_response;
@@ -196,7 +196,7 @@ mod tests {
 			.create();
 
 		// Create a new VssClient with the mock server URL.
-		let client = VssClient::new(base_url);
+		let client = VssClient::new(base_url).unwrap();
 
 		let actual_result = client.list_key_versions(&request).await.unwrap();
 
@@ -210,7 +210,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);
+		let vss_client = VssClient::new(base_url).unwrap();
 
 		// NoSuchKeyError
 		let error_response = ErrorResponse {
@@ -237,7 +237,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);
+		let vss_client = VssClient::new(base_url).unwrap();
 
 		// GetObjectResponse with None value
 		let mock_response = GetObjectResponse { value: None, ..Default::default() };
@@ -258,7 +258,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);
+		let vss_client = VssClient::new(base_url).unwrap();
 
 		// Invalid Request Error
 		let error_response = ErrorResponse {
@@ -318,7 +318,7 @@ mod tests {
 	#[tokio::test]
 	async fn test_auth_err_handling() {
 		let base_url = mockito::server_url();
-		let vss_client = VssClient::new(base_url);
+		let vss_client = VssClient::new(base_url).unwrap();
 
 		// Invalid Request Error
 		let error_response = ErrorResponse {
@@ -390,7 +390,9 @@ 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(), header_provider);
+		let client =
+			VssClient::new_with_headers("http://localhost/notused".to_string(), header_provider)
+				.unwrap();
 		let result = client.get_object(&get_request).await;
 
 		assert!(matches!(result, Err(VssError::AuthError { .. })));
@@ -399,7 +401,7 @@ mod tests {
 	#[tokio::test]
 	async fn test_conflict_err_handling() {
 		let base_url = mockito::server_url();
-		let vss_client = VssClient::new(base_url);
+		let vss_client = VssClient::new(base_url).unwrap();
 
 		// Conflict Error
 		let error_response = ErrorResponse {
@@ -432,7 +434,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);
+		let vss_client = VssClient::new(base_url).unwrap();
 
 		// Internal Server Error
 		let error_response = ErrorResponse {
@@ -492,7 +494,7 @@ mod tests {
 	#[tokio::test]
 	async fn test_internal_err_handling() {
 		let base_url = mockito::server_url();
-		let vss_client = VssClient::new(base_url);
+		let vss_client = VssClient::new(base_url).unwrap();
 
 		let error_response =
 			ErrorResponse { error_code: 999, message: "UnknownException".to_string() };