feat(client): add disconnect_reason API
#1246
Conversation
This commit adds an API to get the error reason why the backend was disconnected. Close #1196
| @@ -57,7 +57,7 @@ pub enum Error { | |||
| InvalidResponse(Mismatch<String>), | |||
| /// The background task has been terminated. | |||
| #[error("The background task been terminated because: {0}; restart required")] | |||
| RestartNeeded(String), | |||
| RestartNeeded(Arc<Error>), | |||
There was a problem hiding this comment.
This is awkward to match on but definitely better than a String I reckon..
| /// # Cancel-safety | ||
| /// | ||
| /// This method is not cancel-safe | ||
| pub async fn disconnect_reason(&self) -> Error { |
There was a problem hiding this comment.
I'm a little worried to expose this in the public API but changed this to a RwLock and it should only require the write lock once.
This entire thing is to be &self and not &mut self and I'm tempted to move this tokio watch/broadcast but it doesn't work because of these APIs requires &mut self as well......
core/src/client/async_client/mod.rs
Outdated
| arc_err | ||
| } | ||
| ReadErrorOnce::Read(arc_err) => { | ||
| *write = ReadErrorOnce::Read(arc_err.clone()); |
There was a problem hiding this comment.
state is moved on the match and must be "initialized again here"
client/ws-client/src/tests.rs
Outdated
| @@ -68,8 +68,12 @@ async fn method_call_with_wrong_id_kind() { | |||
| let client = | |||
| WsClientBuilder::default().id_format(IdKind::String).build(&uri).with_default_timeout().await.unwrap().unwrap(); | |||
|
|
|||
| let err: Result<String, Error> = client.request("o", rpc_params![]).with_default_timeout().await.unwrap(); | |||
| assert!(matches!(err, Err(Error::RestartNeeded(e)) if e == "request ID=0 is not a pending call")); | |||
| match client.request::<String, _>("o", rpc_params![]).with_default_timeout().await.unwrap() { | |||
There was a problem hiding this comment.
nit: Maybe assert_matches! cleans this up a bit, but since its part of testing I wouldn't worry too much about it
There was a problem hiding this comment.
I wanted to show that working with Error::RestartNeeded(Arc) can be awkward :P
core/src/client/async_client/mod.rs
Outdated
| }; | ||
|
|
||
| let mut write = self.0.write().await; | ||
| let state = std::mem::replace(&mut *write, ReadErrorOnce::Unreachable); |
There was a problem hiding this comment.
We expect this to be called exactly once right?
Otherwise, we'll get to the unreachable below, would something like AsyncRwLock<Option<RreadErrorOnce>> get rid of the ReadErrorOnce::Unreachable?
There was a problem hiding this comment.
yeah fair enough,
Ideally we would remove this AsyncRwLock with something like https://docs.rs/async-once-cell/0.5.3/async_once_cell/struct.Lazy.html# but it requires a bunch of unstable features but would be nice to get rid of for sure.
There was a problem hiding this comment.
Looks clean as it is, no need to rely on unstable features for now :D
This commit adds an API to get the error reason why the backend was disconnected.
Close #1196