Initialize Client #5
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new Rust client library for the BAM API, providing HTTP client functionality with error handling, retry logic, and configuration management.
- Adds core client implementation with async HTTP methods and exponential backoff retry logic
- Implements comprehensive error types using
thiserrorfor better error handling - Provides configurable client settings including timeout and retry behavior
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Adds dependencies for HTTP client (reqwest), serialization (serde), error handling (thiserror), and async runtime (tokio). Contains invalid edition "2024". |
| src/lib.rs | Declares new modules (client, config, error) and defines API version constant |
| src/error.rs | Defines comprehensive error types for the BAM API client with helper methods |
| src/config.rs | Provides configuration struct with builder pattern for client customization. Documentation incorrectly references "Jito API". |
| src/client.rs | Implements main HTTP client with retry logic and validator endpoint. Documentation incorrectly references "Jito API". |
| Cargo.lock | Auto-generated dependency lockfile reflecting new dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,47 @@ | |||
| use std::time::Duration; | |||
|
|
|||
| /// Configuration for the Jito API client | |||
There was a problem hiding this comment.
The documentation says "Configuration for the Jito API client" but the struct is part of the BAM API client. This should be "Configuration for the BAM API client" to match the actual purpose of this library.
| /// Configuration for the Jito API client | |
| /// Configuration for the BAM API client |
| } | ||
|
|
||
| impl BamApiClient { | ||
| /// Create a new Jito API client with the given configuration |
There was a problem hiding this comment.
The documentation says "Create a new Jito API client with the given configuration" but this is a BAM API client. This should be "Create a new BAM API client with the given configuration" to match the actual client type.
| /// Create a new Jito API client with the given configuration | |
| /// Create a new BAM API client with the given configuration |
| let mut retries = 0; | ||
| let max_retries = if self.config.retry_enabled { | ||
| self.config.max_retries | ||
| } else { | ||
| 0 | ||
| }; | ||
|
|
||
| loop { | ||
| let mut request = self.client.request(method.clone(), url); | ||
|
|
||
| if let Some(body) = body { | ||
| request = request.json(body); | ||
| } | ||
|
|
||
| let response = request.send().await?; | ||
|
|
||
| match self.handle_response(response).await { | ||
| Ok(data) => return Ok(data), | ||
| Err(e) => { | ||
| if retries >= max_retries || !self.should_retry(&e) { | ||
| return Err(e); | ||
| } | ||
| retries += 1; | ||
| // Exponential backoff | ||
| let delay = Duration::from_millis(100 * 2u64.pow(retries)); | ||
| tokio::time::sleep(delay).await; |
There was a problem hiding this comment.
[nitpick] The exponential backoff calculation uses retries which starts at 0 and is incremented after the check. This means the first retry will use 2^1 = 2 (200ms delay), second retry uses 2^2 = 4 (400ms), etc. However, when retries reaches max_retries, the loop exits, so if max_retries is 3, you only get 3 retry attempts with delays of 200ms, 400ms, and 800ms. This logic appears correct, but consider whether the retry count semantics are clear - are we retrying 3 times in addition to the initial request (4 total attempts), or making 3 total attempts? Currently it's 4 total attempts (1 initial + 3 retries).
| let url = format!( | ||
| "{}/api/{}{}{}", | ||
| self.config.base_url, | ||
| crate::API_VERSION, | ||
| endpoint, |
There was a problem hiding this comment.
[nitpick] The URL construction concatenates endpoint directly after the API version with no separator. If endpoint is "/validators" (as used in get_validators), the resulting URL will be .../api/v1/validators... which is correct. However, the code would be more robust and clear if it explicitly handled the leading slash to prevent potential issues if an endpoint without a leading slash is passed.
| let url = format!( | |
| "{}/api/{}{}{}", | |
| self.config.base_url, | |
| crate::API_VERSION, | |
| endpoint, | |
| // Ensure endpoint starts with a single leading slash | |
| let normalized_endpoint = if endpoint.starts_with('/') { | |
| endpoint.to_string() | |
| } else { | |
| format!("/{}", endpoint) | |
| }; | |
| let url = format!( | |
| "{}/api/{}{}{}", | |
| self.config.base_url, | |
| crate::API_VERSION, | |
| normalized_endpoint, |
No description provided.