Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changes/added/3183.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add unchecked constructor for FuelClient
5 changes: 4 additions & 1 deletion bin/e2e-test-client/src/test_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use anyhow::{
use fuel_core_chain_config::ContractConfig;
use fuel_core_client::client::{
FuelClient,
normalize_url,
types::{
CoinType,
TransactionStatus,
Expand Down Expand Up @@ -72,7 +73,9 @@ impl TestContext {
}

fn new_client(default_endpoint: String, wallet: &ClientConfig) -> FuelClient {
FuelClient::new(wallet.endpoint.clone().unwrap_or(default_endpoint)).unwrap()
let endpoint = wallet.endpoint.clone().unwrap_or(default_endpoint);
let url = normalize_url(&endpoint).unwrap();
FuelClient::new_unchecked(url.as_str()).unwrap()
}
}

Expand Down
5 changes: 3 additions & 2 deletions bin/e2e-test-client/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ use std::{
str::FromStr,
time::Duration,
};
use tempfile::TempDir; // Used for writing assertions // Run programs
use tempfile::TempDir;
// Used for writing assertions // Run programs
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use tempfile::TempDir;
// Used for writing assertions // Run programs
use tempfile::TempDir;


// Use Jemalloc
#[global_allocator]
Expand Down Expand Up @@ -81,7 +82,7 @@ async fn works_in_multinode_local_env() {
let producer_bound_addr = producer.node.bound_address.to_string();
let validator_bound_addr = validator.node.bound_address.to_string();

config.wallet_a.endpoint = Some(producer_bound_addr.clone());
config.wallet_a.endpoint = Some(producer_bound_addr);
config.wallet_b.endpoint = Some(validator_bound_addr);

// save config file
Expand Down
4 changes: 2 additions & 2 deletions bin/fuel-core-client/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ struct CliArgs {

impl CliArgs {
async fn exec(&self) {
let client =
FuelClient::new(self.endpoint.as_str()).expect("expected valid endpoint");
let client = FuelClient::new_unchecked(self.endpoint.as_str())
.expect("expected valid endpoint");
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.

match &self.command {
Command::Transaction(sub_cmd) => match sub_cmd {
Expand Down
91 changes: 41 additions & 50 deletions crates/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ impl Default for AWSClientManager {
}

/// Normalizes a URL string by ensuring it has an http(s) scheme and the `/v1/graphql` path.
fn normalize_url(url_str: &str) -> anyhow::Result<Url> {
pub fn normalize_url(url_str: &str) -> anyhow::Result<Url> {
let mut raw_url = url_str.to_string();
if !raw_url.starts_with("http") {
raw_url = format!("http://{raw_url}");
Expand Down Expand Up @@ -383,17 +383,36 @@ pub fn from_strings_errors_to_std_error(errors: Vec<String>) -> io::Error {
}

impl FuelClient {
#[deprecated(since = "0.47.1", note = "Use `new_unchecked` instead")]
pub fn new(url: impl AsRef<str>) -> anyhow::Result<Self> {
Self::from_str(url.as_ref())
}

pub fn new_unchecked(url: impl AsRef<str>) -> anyhow::Result<Self> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just deprecate the new method, we want people to start to use the new constructors with multiple URLs.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant we don't need new_unchecked anymore. we have method that accepts multiple endpoints

let url = Url::parse(url.as_ref())
.map_err(anyhow::Error::msg)
.with_context(|| format!("Invalid fuel-core URL: {:?}", url.as_ref()))?;

Ok(Self {
transport: FailoverTransport::new(vec![url])?,
require_height: ConsistencyPolicy::Auto {
height: Arc::new(Mutex::new(None)),
},
chain_state_info: Default::default(),
#[cfg(feature = "rpc")]
rpc_client: None,
#[cfg(feature = "rpc")]
aws_client: AWSClientManager::new(),
})
}

#[cfg(feature = "rpc")]
pub async fn new_with_rpc<G: AsRef<str>, R: AsRef<str>>(
graph_ql_urls: impl Iterator<Item = G>,
rpc_url: R,
) -> anyhow::Result<Self> {
let urls: Vec<_> = graph_ql_urls
.map(|str| normalize_url(str.as_ref()))
.map(|str| Url::parse(str.as_ref()))
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
.try_collect()?;
let mut client = Self::with_urls(&urls)?;
let mut raw_rpc_url = <R as AsRef<str>>::as_ref(&rpc_url).to_string();
Expand All @@ -406,13 +425,29 @@ impl FuelClient {
Ok(client)
}

#[cfg(feature = "rpc")]
pub async fn new_unchecked_with_rpc<G: AsRef<str>, R: AsRef<str>>(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new_with_rpc is not released, you can make this function automatically not affect the URL

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that but felt like it was inconsistent. I can change it though.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this function, we have new_with_rpc which is enough

graph_ql_urls: impl Iterator<Item = G>,
rpc_url: R,
) -> anyhow::Result<Self> {
let urls: Vec<_> = graph_ql_urls
.map(|str| Url::parse(str.as_ref()))
.try_collect()?;
let mut client = Self::with_urls(&urls)?;
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
let raw_rpc_url = <R as AsRef<str>>::as_ref(&rpc_url).to_string();
let rpc_client = ProtoBlockAggregatorClient::connect(raw_rpc_url).await?;
client.rpc_client = Some(rpc_client);
client.aws_client = AWSClientManager::new();
Ok(client)
}
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated

pub fn with_urls(urls: &[impl AsRef<str>]) -> anyhow::Result<Self> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is only used by us, so we can also alter its behaviour and do not do normalization.

if urls.is_empty() {
return Err(anyhow!("Failed to create FuelClient. No URL is provided."));
}
let urls = urls
.iter()
.map(|url| normalize_url(url.as_ref()))
.map(|url| Url::parse(url.as_ref()))
.collect::<Result<Vec<_>, _>>()?;
Ok(Self {
transport: FailoverTransport::new(urls)?,
Expand Down Expand Up @@ -1944,58 +1979,14 @@ impl FuelClient {
mod tests {
use super::*;

#[test]
fn with_urls_normalizes_urls_to_graphql_endpoint() {
// Given
let urls = &["http://localhost:8080", "http://example.com:4000"];

// When
let client = FuelClient::with_urls(urls).expect("should create client");

// Then
assert_eq!(
client.get_default_url().as_str(),
"http://localhost:8080/v1/graphql"
);
}

#[test]
fn with_urls_adds_http_scheme_if_missing() {
// Given
let urls = &["localhost:8080"];

// When
let client = FuelClient::with_urls(urls).expect("should create client");

// Then
assert_eq!(
client.get_default_url().as_str(),
"http://localhost:8080/v1/graphql"
);
}

#[test]
fn with_urls_overwrites_existing_path() {
// Given - URLs that already have some path
let urls = &["http://localhost:8080/some/path", "http://example.com/api"];

// When
let client = FuelClient::with_urls(urls).expect("should create client");

// Then - path should be normalized to /v1/graphql
assert_eq!(
client.get_default_url().as_str(),
"http://localhost:8080/v1/graphql"
);
}

#[test]
fn new_and_with_urls_produce_same_url() {
// Given
let url = "http://localhost:8080";
let url = "http://localhost:8080/v1/graphql";

// When
let client_new = FuelClient::new(url).expect("should create client via new");
let client_new =
FuelClient::new_unchecked(url).expect("should create client via new");
let client_with_urls =
FuelClient::with_urls(&[url]).expect("should create client via with_urls");

Expand Down
2 changes: 1 addition & 1 deletion tests/tests/dos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ async fn heavy_tasks_doesnt_block_graphql() {

let node = FuelService::new_node(config).await.unwrap();
let url = format!("http://{}/v1/graphql", node.bound_address);
let client = FuelClient::new(url.clone()).unwrap();
let client = FuelClient::new_unchecked(url.clone()).unwrap();
client.produce_blocks(NUM_OF_BLOCKS, None).await.unwrap();

// Given
Expand Down
Loading