-
Notifications
You must be signed in to change notification settings - Fork 796
feature: bubble up jsonrpc error response via trait #2122
Conversation
okay this now provides 2 traits:
the core idea here is that if the direction is good, I'll go through and update all the docs |
cfdaad6
to
a6e592b
Compare
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.
Really nice and useful, basically the same pattern that middleware uses to give access to inner layers but for errors, to avoid the nested match abomination...
(We could/shoudl've done the refactor in a separate PR but OK w/ me)
ethers-providers/src/errors.rs
Outdated
pub trait RpcError: Error + Debug + Send + Sync { | ||
fn as_error_response(&self) -> Option<&JsonRpcError>; | ||
} | ||
|
||
pub trait MiddlewareError: Error + Sized + Send + Sync { | ||
type Inner: MiddlewareError; | ||
|
||
fn from_err(e: Self::Inner) -> Self; | ||
|
||
fn as_inner(&self) -> Option<&Self::Inner>; | ||
|
||
fn as_provider_error(&self) -> Option<&ProviderError> { | ||
self.as_inner()?.as_provider_error() | ||
} | ||
|
||
fn from_provider_err(p: ProviderError) -> Self { | ||
Self::from_err(Self::Inner::from_provider_err(p)) | ||
} | ||
|
||
fn as_error_response(&self) -> Option<&JsonRpcError> { | ||
MiddlewareError::as_error_response(self.as_inner()?) | ||
} | ||
} | ||
|
||
#[derive(Debug, Error)] | ||
/// An error thrown when making a call to the provider | ||
pub enum ProviderError { | ||
/// An internal error in the JSON RPC Client | ||
#[error("{0}")] | ||
JsonRpcClientError(Box<dyn crate::RpcError + Send + Sync>), | ||
|
||
/// An error during ENS name resolution | ||
#[error("ens name not found: {0}")] | ||
EnsError(String), | ||
|
||
/// Invalid reverse ENS name | ||
#[error("reverse ens name not pointing to itself: {0}")] | ||
EnsNotOwned(String), | ||
|
||
#[error(transparent)] | ||
SerdeJson(#[from] serde_json::Error), | ||
|
||
#[error(transparent)] | ||
HexError(#[from] hex::FromHexError), | ||
|
||
#[error(transparent)] | ||
HTTPError(#[from] reqwest::Error), | ||
|
||
#[error("custom error: {0}")] | ||
CustomError(String), | ||
|
||
#[error("unsupported RPC")] | ||
UnsupportedRPC, | ||
|
||
#[error("unsupported node client")] | ||
UnsupportedNodeClient, | ||
|
||
#[error("Attempted to sign a transaction with no available signer. Hint: did you mean to use a SignerMiddleware?")] | ||
SignerUnavailable, | ||
} |
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.
amazing
Main question is: what breaking changes does this have? Seems like..none? If so, let's go ahead |
I can break these into 2 if you want, shoulda done that in the first place tbh
the breaking changes are
|
a6e592b
to
4ab2e09
Compare
Sounds good. We'll need to update |
re-organization has been moved into #2159 |
I went ahead and split, as it won't generate conflicts because that branch is based on this one 👍 I can also easily clean up any conflicts so lfg |
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.
lgtm, does this affect how the RetryProvider needs to check for serde/reqwest error?
it really doesn't affect it at all. But now that you mention it, we could add |
okay I added |
failures are unrelated and this is good to go 🎉 like other recent PRs, this is a breaking change, and will require action from 3rd-party crates |
supreseded #2159 |
Motivation
Currently there's no way to get JSON-RPC error response out of the provider or middleware error. They're type-erased in the ProviderError.
Solution
This adds a
ClientError
trait which helps castProviderError
toJsonRpcError
.I consider this to be a bit of an unpleasant kludge. I'm not sure it should be merged. Hypothetically this would allow us to access and decode revert data from contracts tho, which is definitely a big user need.
PR Checklist