Pubsub checks #153
Pubsub checks #153
Conversation
LeoPatOZ
force-pushed
the
pubsub-checks
branch
from
790dadf to
7b9cdfb
Compare
| Timeout, | ||
| #[error("RPC call failed after exhausting all retry attempts: {0}")] | ||
| RetryFailure(Arc<RpcError<TransportErrorKind>>), | ||
| RpcError(Arc<RpcError<TransportErrorKind>>), |
There was a problem hiding this comment.
Clearer as this error might not happen when 'retrying'
There was a problem hiding this comment.
Wondering if it still makes sense to have RetryFailure?
There was a problem hiding this comment.
Maybe doesn't serve any purpose
LeoPatOZ
force-pushed
the
pubsub-checks
branch
from
8076ee9 to
45fa778
Compare
|
Removing approval due to this edge case:
|
| .map_err(Error::from)? | ||
| .map_err(Error::from) |
There was a problem hiding this comment.
I realised a slight mistake in the multi provider PR - any RPC error was being returned as a Timeout error
src/robust_provider.rs
Outdated
| for (idx, provider) in self.providers.iter().enumerate() { | ||
| // Skip providers that don't support pubsub if required | ||
| if require_pubsub && !Self::supports_pubsub(provider) { | ||
| if idx == 0 { | ||
| info!("Primary provider doesn't support pubsub, skipping"); | ||
| } else { | ||
| info!("Fallback provider {} doesn't support pubsub, skipping", idx); | ||
| } |
There was a problem hiding this comment.
Seems having the primary provider in a separate field is useful after all... No matter, leave things as-is for now.
src/robust_provider.rs
Outdated
| // Verify that the WS connection is actually dead before proceeding | ||
| // Retry until it fails to ensure the connection is truly dead | ||
| let mut retries = 0; | ||
| loop { | ||
| let verification = ws_provider.get_block_number().await; | ||
| if verification.is_err() { | ||
| break; | ||
| } | ||
| sleep(Duration::from_millis(50)).await; | ||
| retries += 1; | ||
| assert!(retries < 50, "WS provider should have failed after anvil dropped"); | ||
| } |
There was a problem hiding this comment.
Is this truly necessary? Won't WS provider fail immediately with BackendGone?
There was a problem hiding this comment.
I didn't have it initially but the test was super flaky on the CI. There just wasn't an error when calling subscribe after the drop , which indicated that the WS provider was still up. I'm not sure of another way to kill the anvil node :/
There was a problem hiding this comment.
Then instead relying on dropping Anvil node, you could use alloy::providers::mock::Asserter to force an error on provider_1
Won't work, Asserter doesn't support pubsub
Towards #6