Conversation
gregorydemay
force-pushed
the
gdemay/XC-287-retry-layer
branch
from
b102478 to
0a0297a
Compare
| impl Client { | ||
| /// Create a new client returning custom errors. | ||
| pub fn new_with_error<CustomError: From<IcError>>( | ||
| ) -> impl Service<IcHttpRequestWithCycles, Response = IcHttpResponse, Error = CustomError> { |
There was a problem hiding this comment.
There are some subtle issues here at play, which is the reason why I needed yet another type of conversion to somewhat emulate map_err but with a concrete type:
- The main issue is that in order to be able to use
tower::retry, the service needs to beClone. - The current return type, because its an
impl Somethingcannot beClone. - To be clone, we would need to return the exact type, which we cannot do because
map_erronly takes a closure (FnOnce) and unfortunately no concrete type can implement theFnOncetrait (see Rust issue 29625). - We could try to use
BoxServiceorBoxCloneServiceonClientto do type erasure but this also won't work because it requires the future type of the service to beSend + 'staticwhich cannot be derived because the future ofClientis not exposed by theic_cdkso we have to resort to using<Box<dyn Future>>
All of that to say feel free to chime in if you have better ideas 🙈 .
There was a problem hiding this comment.
I can't say I have a better idea... Maybe @ninegua ?
lpahlavi
left a comment
lpahlavi
left a comment
There was a problem hiding this comment.
@gregorydemay Thanks a lot for this PR! Looks very good in my opinion.
| impl Client { | ||
| /// Create a new client returning custom errors. | ||
| pub fn new_with_error<CustomError: From<IcError>>( | ||
| ) -> impl Service<IcHttpRequestWithCycles, Response = IcHttpResponse, Error = CustomError> { |
There was a problem hiding this comment.
I can't say I have a better idea... Maybe @ninegua ?
| @@ -99,9 +99,11 @@ | |||
| //! # } | |||
| //! ``` | |||
There was a problem hiding this comment.
nit: maybe you could actually add an example with error conversion?
There was a problem hiding this comment.
good idea, done.
gregorydemay
left a comment
gregorydemay
left a comment
There was a problem hiding this comment.
Thanks @lpahlavi for the review!
| @@ -99,9 +99,11 @@ | |||
| //! # } | |||
| //! ``` | |||
There was a problem hiding this comment.
good idea, done.
lpahlavi
left a comment
lpahlavi
left a comment
There was a problem hiding this comment.
Thanks for adding the docs example, LGTM!
Follow-up on dfinity/evm-rpc-canister#370, dfinity/evm-rpc-canister#364, dfinity/evm-rpc-canister#374, and dfinity/evm-rpc-canister#375 to automatically retry requests by doubling its `max_response_bytes` when the response was too big.
2 participants
Follow-up on #370, #364, #374, and #375 to automatically retry requests by doubling its
max_response_byteswhen the response was too big.