From 1163270a6ffa89e857770690e4af98ede05136e7 Mon Sep 17 00:00:00 2001 From: Max Fang Date: Tue, 11 Aug 2026 19:47:56 -0700 Subject: [PATCH] refactor!: normalize responses into a shared HttpResponse Normalize responses into a private HttpResponse (status + fully-read body) as soon as they leave the HTTP client, so the retry loops, status checking, and all endpoint methods are backend-agnostic. Only the code producing an HttpResponse now touches bitreq types, preparing to swap in alternative HTTP transports (see #97). Also deletes the bitreq::Response status predicates, four of which were dead code. Breaking: - BlockingClient::post_request is now private (it returned the raw bitreq::Response). - JSON and UTF-8 decode errors are now reported via Error::SerdeJson and a new Error::InvalidUtf8 variant rather than wrapped bitreq errors. License: MIT OR Apache-2.0 --- CHANGELOG.md | 1 + src/async.rs | 104 ++++++++++++++++++++---------------------------- src/blocking.rs | 74 ++++++++++------------------------ src/lib.rs | 96 +++++++++++++++++++++++++------------------- 4 files changed, 121 insertions(+), 154 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74ebfa84..4cbc6cbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Changed +* refactor(client)!: normalize responses into a shared `HttpResponse` [#243] * feat(client): enable request pipelining for `AsyncClient` [#245] * chore!: remove deprecated `BlockSummary` and `get_block` [#225] diff --git a/src/async.rs b/src/async.rs index 5c866520..0cf91ddc 100644 --- a/src/async.rs +++ b/src/async.rs @@ -43,11 +43,11 @@ use bitcoin::hashes::{sha256, Hash}; use bitcoin::hex::{DisplayHex, FromHex}; use bitcoin::{Address, Amount, Block, BlockHash, FeeRate, MerkleBlock, Script, Transaction, Txid}; -use bitreq::{Client, Method, Proxy, Request, RequestExt, Response}; +use bitreq::{Client, Method, Proxy, Request, RequestExt}; use crate::{ - duration_to_timeout_secs, is_retryable, is_success, sat_per_vbyte_to_feerate, AddressStats, - BlockInfo, BlockStatus, Builder, Error, EsploraTx, MempoolRecentTx, MempoolStats, MerkleProof, + duration_to_timeout_secs, sat_per_vbyte_to_feerate, AddressStats, BlockInfo, BlockStatus, + Builder, Error, EsploraTx, HttpResponse, MempoolRecentTx, MempoolStats, MerkleProof, OutputStatus, ScriptHashStats, SubmitPackageResult, TxStatus, Utxo, BASE_BACKOFF_MILLIS, }; @@ -150,22 +150,47 @@ impl AsyncClient { Ok(request) } - /// Sends a GET request to `url`, retrying on retryable status codes + /// Sends a single GET request to `path`. + async fn send_get(&self, path: &str) -> Result { + let request = self.build_request(Method::Get, path)?.with_pipelining(); + let response = request.send_async_with_client(&self.client).await?; + HttpResponse::from_bitreq(response) + } + + /// Sends a single POST request to `path` with `body` and query parameters. + async fn send_post( + &self, + path: &str, + body: Vec, + query_params: Option>, + ) -> Result { + let mut request = self.build_request(Method::Post, path)?.with_body(body); + + for (key, value) in query_params.unwrap_or_default() { + request = request.with_param(key, value); + } + + let response = request.send_async_with_client(&self.client).await?; + HttpResponse::from_bitreq(response) + } + + /// Sends a GET request to `path`, retrying on retryable status codes /// with exponential backoff until [`AsyncClient::max_retries`] is reached. - async fn get_with_retry(&self, path: &str) -> Result { + /// + /// Returns an [`Error::HttpResponse`] on a non-success status code. + async fn get_with_retry(&self, path: &str) -> Result { let mut delay = BASE_BACKOFF_MILLIS; let mut attempts = 0; - let request = self.build_request(Method::Get, path)?.with_pipelining(); - loop { - match request.clone().send_async_with_client(&self.client).await? { - response if attempts < self.max_retries && is_retryable(&response) => { - S::sleep(delay).await; - attempts += 1; - delay *= 2; - } - response => return Ok(response), + let response = self.send_get(path).await?; + + if attempts < self.max_retries && response.is_retryable() { + S::sleep(delay).await; + attempts += 1; + delay *= 2; + } else { + return response.error_for_status(); } } } @@ -180,14 +205,7 @@ impl AsyncClient { /// Returns an [`Error`] if the request fails or deserialization fails. async fn get_response(&self, path: &str) -> Result { let response = self.get_with_retry(path).await?; - - if !is_success(&response) { - let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; - let message = response.as_str().unwrap_or_default().to_string(); - return Err(Error::HttpResponse { status, message }); - } - - Ok(deserialize::(response.as_bytes())?) + Ok(deserialize::(&response.body)?) } /// Makes a GET request to `path`, returning `None` on a 404 response. @@ -215,14 +233,7 @@ impl AsyncClient { path: &str, ) -> Result { let response = self.get_with_retry(path).await?; - - if !is_success(&response) { - let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; - let message = response.as_str().unwrap_or_default().to_string(); - return Err(Error::HttpResponse { status, message }); - } - - response.json::().map_err(Error::BitReq) + response.json::() } /// Makes a GET request to `path`, returning `None` on a 404 response. @@ -250,13 +261,6 @@ impl AsyncClient { /// or consensus deserialization fails. async fn get_response_hex(&self, path: &str) -> Result { let response = self.get_with_retry(path).await?; - - if !is_success(&response) { - let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; - let message = response.as_str().unwrap_or_default().to_string(); - return Err(Error::HttpResponse { status, message }); - } - let hex_str = response.as_str()?; Ok(deserialize(&Vec::from_hex(hex_str)?)?) } @@ -282,13 +286,6 @@ impl AsyncClient { /// Returns an [`Error`] if the request fails. async fn get_response_text(&self, path: &str) -> Result { let response = self.get_with_retry(path).await?; - - if !is_success(&response) { - let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; - let message = response.as_str().unwrap_or_default().to_string(); - return Err(Error::HttpResponse { status, message }); - } - Ok(response.as_str()?.to_string()) } @@ -317,22 +314,9 @@ impl AsyncClient { path: &str, body: T, query_params: Option>, - ) -> Result { - let mut request: bitreq::Request = self.build_request(Method::Post, path)?.with_body(body); - - for (key, value) in query_params.unwrap_or_default() { - request = request.with_param(key, value); - } - - let response = request.send_async_with_client(&self.client).await?; - - if !is_success(&response) { - let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; - let message = response.as_str().unwrap_or_default().to_string(); - return Err(Error::HttpResponse { status, message }); - } - - Ok(response) + ) -> Result { + let response = self.send_post(path, body.into(), query_params).await?; + response.error_for_status() } /// Get a raw [`Transaction`] given its [`Txid`]. diff --git a/src/blocking.rs b/src/blocking.rs index 11e8091b..0bee2a55 100644 --- a/src/blocking.rs +++ b/src/blocking.rs @@ -28,13 +28,12 @@ //! [Esplora]: https://github.com/Blockstream/esplora/blob/master/API.md use std::collections::{HashMap, HashSet}; -use std::convert::TryFrom; use std::str::FromStr; use std::thread; use std::time::Duration; use bitcoin::consensus::encode::serialize_hex; -use bitreq::{Method, Proxy, Request, Response}; +use bitreq::{Method, Proxy, Request}; use bitcoin::block::Header as BlockHeader; use bitcoin::consensus::{deserialize, serialize, Decodable}; @@ -43,8 +42,8 @@ use bitcoin::hex::{DisplayHex, FromHex}; use bitcoin::{Address, Amount, Block, BlockHash, FeeRate, MerkleBlock, Script, Transaction, Txid}; use crate::{ - duration_to_timeout_secs, is_retryable, is_success, sat_per_vbyte_to_feerate, AddressStats, - BlockInfo, BlockStatus, Builder, Error, EsploraTx, MempoolRecentTx, MempoolStats, MerkleProof, + duration_to_timeout_secs, sat_per_vbyte_to_feerate, AddressStats, BlockInfo, BlockStatus, + Builder, Error, EsploraTx, HttpResponse, MempoolRecentTx, MempoolStats, MerkleProof, OutputStatus, ScriptHashStats, SubmitPackageResult, TxStatus, Utxo, BASE_BACKOFF_MILLIS, }; @@ -133,43 +132,38 @@ impl BlockingClient { /// /// This function will return an error either from the HTTP client, or the /// response's [`serde_json`] deserialization. - pub fn post_request>>( + fn post_request>>( &self, path: &str, body: T, query_params: Option>, - ) -> Result { + ) -> Result { let mut request = self.build_request(Method::Post, path)?.with_body(body); for (key, value) in query_params.unwrap_or_default() { request = request.with_param(key, value); } - let response = request.send()?; - - if !is_success(&response) { - let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; - let message = response.as_str().unwrap_or_default().to_string(); - return Err(Error::HttpResponse { status, message }); - } - - Ok(response) + let response = HttpResponse::from_bitreq(request.send()?)?; + response.error_for_status() } /// Sends a GET request to `url`, retrying on retryable status codes /// with exponential backoff until [`BlockingClient::max_retries`] is reached. - fn get_with_retry(&self, url: &str) -> Result { + fn get_with_retry(&self, url: &str) -> Result { let mut delay = BASE_BACKOFF_MILLIS; let mut attempts = 0; loop { - match self.build_request(Method::Get, url)?.send()? { - resp if attempts < self.max_retries && is_retryable(&resp) => { - thread::sleep(delay); - attempts += 1; - delay *= 2; - } - resp => return Ok(resp), + let response = + HttpResponse::from_bitreq(self.build_request(Method::Get, url)?.send()?)?; + + if attempts < self.max_retries && response.is_retryable() { + thread::sleep(delay); + attempts += 1; + delay *= 2; + } else { + return response.error_for_status(); } } } @@ -184,14 +178,7 @@ impl BlockingClient { /// Returns an [`Error`] if the request fails or deserialization fails. fn get_response(&self, path: &str) -> Result { let response = self.get_with_retry(path)?; - - if !is_success(&response) { - let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; - let message = response.as_str().unwrap_or_default().to_string(); - return Err(Error::HttpResponse { status, message }); - } - - Ok(deserialize::(response.as_bytes())?) + Ok(deserialize::(&response.body)?) } /// Makes a GET request to `path`, returning `None` on a 404 response. @@ -216,15 +203,8 @@ impl BlockingClient { /// or consensus deserialization fails. fn get_response_hex(&self, path: &str) -> Result { let response = self.get_with_retry(path)?; - - if !is_success(&response) { - let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; - let message = response.as_str().unwrap_or_default().to_string(); - return Err(Error::HttpResponse { status, message }); - } - let hex_str = response.as_str()?; - deserialize(&Vec::from_hex(hex_str)?).map_err(Error::BitcoinEncoding) + Ok(deserialize(&Vec::from_hex(hex_str)?)?) } /// Makes a GET request to `path`, returning `None` on a 404 response. @@ -252,14 +232,7 @@ impl BlockingClient { path: &'a str, ) -> Result { let response = self.get_with_retry(path)?; - - if !is_success(&response) { - let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; - let message = response.as_str().unwrap_or_default().to_string(); - return Err(Error::HttpResponse { status, message }); - } - - response.json::().map_err(Error::BitReq) + response.json::() } /// Makes a GET request to `path`, returning `None` on a 404 response. @@ -286,13 +259,6 @@ impl BlockingClient { /// Returns an [`Error`] if the request fails. fn get_response_text(&self, path: &str) -> Result { let response = self.get_with_retry(path)?; - - if !is_success(&response) { - let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; - let message = response.as_str().unwrap_or_default().to_string(); - return Err(Error::HttpResponse { status, message }); - } - Ok(response.as_str()?.to_string()) } diff --git a/src/lib.rs b/src/lib.rs index f1f8c008..9983cb6e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -130,46 +130,6 @@ const DEFAULT_MAX_RETRIES: usize = 6; #[cfg(feature = "async")] const DEFAULT_MAX_CONNECTIONS: usize = 10; -/// Check if the [`Response`] status code is informational (100-199). -#[allow(unused)] -#[cfg(any(feature = "blocking", feature = "async"))] -fn is_informational(response: &Response) -> bool { - (100..200).contains(&response.status_code) -} - -/// Check if the [`Response`] status code is successful (200-299). -#[cfg(any(feature = "blocking", feature = "async"))] -fn is_success(response: &Response) -> bool { - (200..300).contains(&response.status_code) -} - -/// Check if the [`Response`] status code is a redirection (300-399). -#[allow(unused)] -#[cfg(any(feature = "blocking", feature = "async"))] -fn is_redirection(response: &Response) -> bool { - (300..400).contains(&response.status_code) -} - -/// Check if the [`Response`] status code is a client error (400-499). -#[allow(unused)] -#[cfg(any(feature = "blocking", feature = "async"))] -fn is_client_error(response: &Response) -> bool { - (400..500).contains(&response.status_code) -} - -/// Check if the [`Response`] status code is a server error (500-599). -#[allow(unused)] -#[cfg(any(feature = "blocking", feature = "async"))] -fn is_server_error(response: &Response) -> bool { - (500..600).contains(&response.status_code) -} - -/// Check if the [`Response`] status code is retryable (429, 500, 503). -#[cfg(any(feature = "blocking", feature = "async"))] -fn is_retryable(response: &Response) -> bool { - RETRYABLE_ERROR_CODES.contains(&(response.status_code as u16)) -} - /// Convert a [`Duration`] to whole timeout seconds for `bitreq`. #[cfg(any(feature = "blocking", feature = "async"))] fn duration_to_timeout_secs(duration: Duration) -> u64 { @@ -352,6 +312,9 @@ pub enum Error { }, /// Invalid integer returned by the server. Parsing(std::num::ParseIntError), + /// Invalid UTF-8 in a response body expected to be text. + #[cfg(any(feature = "blocking", feature = "async"))] + InvalidUtf8(std::str::Utf8Error), /// Invalid status code, unable to convert to `u16`. StatusCode(TryFromIntError), /// Invalid Bitcoin consensus data returned by the server. @@ -384,6 +347,8 @@ impl fmt::Display for Error { write!(f, "HTTP error {status}: {message}") } Error::Parsing(e) => write!(f, "Failed to parse invalid number: {e}"), + #[cfg(any(feature = "blocking", feature = "async"))] + Error::InvalidUtf8(e) => write!(f, "Invalid UTF-8 in response body: {e}"), Error::StatusCode(e) => write!(f, "Invalid status code: {e}"), Error::BitcoinEncoding(e) => write!(f, "Invalid Bitcoin data: {e}"), Error::HexToArray(e) => write!(f, "Invalid hex to array conversion: {e}"), @@ -430,3 +395,54 @@ impl_error!(std::num::ParseIntError, Parsing, Error); impl_error!(bitcoin::consensus::encode::Error, BitcoinEncoding, Error); impl_error!(bitcoin::hex::HexToArrayError, HexToArray, Error); impl_error!(bitcoin::hex::HexToBytesError, HexToBytes, Error); + +/// A client-agnostic HTTP response: the status code plus the fully-read body. +/// +/// Both clients' endpoint methods operate on this type, so that only the code +/// that produces this is specific to the underlying HTTP client. +#[cfg(any(feature = "blocking", feature = "async"))] +struct HttpResponse { + status: u16, + body: Vec, +} + +#[cfg(any(feature = "blocking", feature = "async"))] +impl HttpResponse { + /// Converts a [`bitreq::Response`], whose body is already fully read. + fn from_bitreq(response: Response) -> Result { + let status = u16::try_from(response.status_code).map_err(Error::StatusCode)?; + let body = response.into_bytes(); + Ok(Self { status, body }) + } + + /// Whether the status code is successful (200-299). + fn is_success(&self) -> bool { + (200..300).contains(&self.status) + } + + /// Whether the status code is retryable (see [`RETRYABLE_ERROR_CODES`]). + fn is_retryable(&self) -> bool { + RETRYABLE_ERROR_CODES.contains(&self.status) + } + + /// Returns an [`Error::HttpResponse`] on a non-success status code. + fn error_for_status(self) -> Result { + if self.is_success() { + Ok(self) + } else { + let status = self.status; + let message = self.as_str().unwrap_or_default().to_string(); + Err(Error::HttpResponse { status, message }) + } + } + + /// The body interpreted as UTF-8 text. + fn as_str(&self) -> Result<&str, Error> { + std::str::from_utf8(&self.body).map_err(Error::InvalidUtf8) + } + + /// Deserializes the body as JSON. + fn json(&self) -> Result { + serde_json::from_slice(&self.body).map_err(Error::SerdeJson) + } +}