-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RPC Consistency Proposal #2473
Open
acerone85
wants to merge
51
commits into
master
Choose a base branch
from
1897-rpc-consistency-proposal
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+431
−7
Open
RPC Consistency Proposal #2473
Changes from 24 commits
Commits
Show all changes
51 commits
Select commit
Hold shift + click to select a range
a8f5f41
Start structuring middleware
acerone85 778dd7c
Use a static atomic to keep track of the current block height
acerone85 d1b1c33
Add changelog entry
acerone85 b77506c
Add tests and fix middleware
acerone85 b764cd4
Update crates/fuel-core/src/graphql_api/api_service.rs
acerone85 d9091e6
Set headers in fuel client
acerone85 90c411f
AxumRequest/AxumResponse -> http::Request/http::Response
acerone85 625bf8f
Fix compilation
acerone85 085d173
Extension instead of Axum middleware
acerone85 a98ef9b
Merge branch 'master' into 1897-rpc-consistency-proposal
xgreenx c95f09a
Add response header
acerone85 6099651
Better handling of response
acerone85 3b81537
Use BlockHeight type when possible
acerone85 11e6876
Fix compilation error
acerone85 88b9c52
Use Request data to retrieve required block height
acerone85 c5f82ff
Remove useless .into
acerone85 844e4e6
Move up const definitions
acerone85 71f8087
Remove useless .into()
acerone85 cbea694
Inject request data in subscription handler
acerone85 da7f057
Improvements
acerone85 f5e63f6
Add documentation
acerone85 5f4500b
Do not fetch the read view twice
acerone85 37dd63e
Remove commented code
acerone85 9913620
Typo
acerone85 f849eb5
Update crates/fuel-core/src/graphql_api/api_service.rs
acerone85 1fddd80
Improve setting headers in client
acerone85 e1f71d4
Revert to setting header in graphql extension when possible
acerone85 be7f927
Update comment
acerone85 0b1e31f
Remove use of Arc<Mutex<_>>
acerone85 8e1b0a5
Remove stray comment
acerone85 abe13c2
WIP
acerone85 eae1e22
Example of how it can be implemented via `extensions`
xgreenx 0a3da1f
Merge branch 'master' into 1897-rpc-consistency-proposal
acerone85 e7e1db7
Add current height header on failed requests
acerone85 6b76769
Return current level after request has been executed
acerone85 b6f206d
WIP: Use only graphql extensions: tests still to be adjusted
acerone85 2f52aaa
Merge branch 'master' into 1897-rpc-consistency-proposal
acerone85 cd3be50
Add capability to the client to set required_fuel_block_height + fix …
acerone85 dbc5cdc
Fix other tests
acerone85 c5db12f
Rename test file
acerone85 24b8d69
Fix BlockHeight base format in extension error
acerone85 4a8d839
Fix typo
acerone85 51ec4a9
Merge branch 'master' into 1897-rpc-consistency-proposal
acerone85 37b03f1
Fix compilation error after rename
acerone85 396a953
Reference follow-up issue
acerone85 af0c6d6
Fix rustfmt
acerone85 b1e89d6
Fix subscriptions feature
acerone85 6e50f3b
Use HashMap::new instead of Default::default
acerone85 dcec472
Downgrade netlink-proto
acerone85 8c6af1f
Revert "Downgrade netlink-proto"
acerone85 818a717
Merge branch 'master' into 1897-rpc-consistency-proposal
acerone85 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,6 +77,12 @@ use pagination::{ | |
PaginatedResult, | ||
PaginationRequest, | ||
}; | ||
use reqwest::header::{ | ||
AsHeaderName, | ||
HeaderMap, | ||
HeaderValue, | ||
IntoHeaderName, | ||
}; | ||
use schema::{ | ||
balance::BalanceArgs, | ||
blob::BlobByIdArgs, | ||
|
@@ -149,12 +155,24 @@ pub mod types; | |
|
||
type RegisterId = u32; | ||
|
||
#[derive(Debug, derive_more::Display, derive_more::From)] | ||
#[non_exhaustive] | ||
/// Error occurring during interaction with the FuelClient | ||
// anyhow::Error is wrapped inside a custom Error type, | ||
// so that we can specific error variants in the future. | ||
pub enum Error { | ||
/// Unknown or not expected(by architecture) error. | ||
#[from] | ||
Other(anyhow::Error), | ||
} | ||
|
||
#[derive(Debug, Clone)] | ||
pub struct FuelClient { | ||
client: reqwest::Client, | ||
#[cfg(feature = "subscriptions")] | ||
cookie: std::sync::Arc<reqwest::cookie::Jar>, | ||
url: reqwest::Url, | ||
headers: HeaderMap, | ||
} | ||
|
||
impl FromStr for FuelClient { | ||
|
@@ -182,13 +200,18 @@ impl FromStr for FuelClient { | |
client, | ||
cookie, | ||
url, | ||
headers: HeaderMap::new(), | ||
}) | ||
} | ||
|
||
#[cfg(not(feature = "subscriptions"))] | ||
{ | ||
let client = reqwest::Client::new(); | ||
Ok(Self { client, url }) | ||
Ok(Self { | ||
client, | ||
url, | ||
headers: HeaderMap::new(), | ||
}) | ||
} | ||
} | ||
} | ||
|
@@ -221,6 +244,23 @@ impl FuelClient { | |
Self::from_str(url.as_ref()) | ||
} | ||
|
||
pub fn set_header( | ||
&mut self, | ||
key: impl IntoHeaderName, | ||
value: impl TryInto<HeaderValue>, | ||
) -> Result<&mut Self, Error> { | ||
let header_value: HeaderValue = value | ||
.try_into() | ||
.map_err(|_err| anyhow::anyhow!("Cannot parse value for header"))?; | ||
self.headers.insert(key, header_value); | ||
Ok(self) | ||
} | ||
|
||
pub fn remove_header(&mut self, key: impl AsHeaderName) -> &mut Self { | ||
self.headers.remove(key); | ||
self | ||
} | ||
|
||
/// Send the GraphQL query to the client. | ||
pub async fn query<ResponseData, Vars>( | ||
&self, | ||
|
@@ -230,9 +270,12 @@ impl FuelClient { | |
Vars: serde::Serialize, | ||
ResponseData: serde::de::DeserializeOwned + 'static, | ||
{ | ||
let response = self | ||
.client | ||
.post(self.url.clone()) | ||
let mut request_builder = self.client.post(self.url.clone()); | ||
for (header_name, header_value) in self.headers.iter() { | ||
request_builder = request_builder.header(header_name, header_value); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use
|
||
|
||
let response = request_builder | ||
.run_graphql(q) | ||
.await | ||
.map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we now have an proper
Error
, maybe we should introduce specific variant instead ofanyhow
?