Skip to content

Conversation

@dorianvp
Copy link
Member

@dorianvp dorianvp commented Nov 6, 2025

Motivation

Solution

Tests

Specifications & References

Follow-up Work

PR Checklist

  • The PR name is suitable for the release notes.
  • The solution is tested.
  • The documentation is up to date.

map_err::Mapper: map_err::MapRpcError<MethodError>,
{
let id = self.id_counter.fetch_add(1, Ordering::SeqCst);
#[derive(Deserialize)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intentional for this to be defined within the send_request method?

Copy link
Contributor

Choose a reason for hiding this comment

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

i see, it's generic over R


let max_attempts = 5;
let mut attempts = 0;
loop {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like these changes are dropping the retry feature. Is it intentional?

T: std::fmt::Debug + Serialize,
R: std::fmt::Debug + for<'de> Deserialize<'de> + ResponseToError,
>(
async fn send_request<T, R, MethodError>(
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is looking large enough that i would recommend chopping it down into smaller functions, to keep it readable and testable.

code = err.code,
kind = ?err.kind(),
message = %err.message,
params = tracing::field::debug(&params),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we clear on the sensitivity of these values? can we freely log them in any case?

/// Json RPSee Error type.
#[derive(Serialize, Deserialize, Debug)]
#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct RpcError {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would put all these error types on jsonrpsee/connector.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants