Concrete error types for Core NATS #632
Conversation
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
| #[error("request timed out")] | ||
| TimedOut, | ||
| #[error("no responders")] | ||
| NoResponders, |
There was a problem hiding this comment.
I'm still in favor of having something like Status(StatusCode) for status errors.
There was a problem hiding this comment.
That could work but it will not, becase there are 5 or more different errors with status 408 and 409.
async-nats/src/client.rs
Outdated
| } | ||
|
|
||
| #[derive(Debug, Error)] | ||
| pub enum IoErrorKind { |
There was a problem hiding this comment.
Not sure about the cases in this one just yet
There was a problem hiding this comment.
Neither I'am.
Looking for better alternative over the weekend.
|
Lots of conflicts now, want to split out Publish and Request errors and land them in the into_future branch? |
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
async-nats/tests/client_tests.rs
Outdated
| // FIXME: we cant do PartialOrd, because mpsc errors are not PartialOrd | ||
| match err { | ||
| Err(RequestError::TimedOut) => {} | ||
| match err.kind() { |
async-nats/tests/client_tests.rs
Outdated
| .await | ||
| .unwrap_err(); | ||
| // FIXME: we cant do PartialOrd, because mpsc errors are not PartialOrd | ||
| match err.kind() { |
There was a problem hiding this comment.
You can do an assert(matches! instead here
async-nats/tests/client_tests.rs
Outdated
| .unwrap_err(); | ||
| .unwrap_err() | ||
| .kind(); | ||
| assert_eq!(RequestErrorKind::NoResponders, err); |
There was a problem hiding this comment.
Keep the err as the RequestError and dig into the field at assert time.
| assert_eq!(RequestErrorKind::NoResponders, err); | |
| assert_eq!(RequestErrorKind::NoResponders, err.kind()); |
async-nats/tests/client_tests.rs
Outdated
| tokio::time::Duration::from_millis(300), | ||
| client.request("test".into(), "request".into()), | ||
| ) | ||
| .await | ||
| .unwrap() | ||
| .unwrap_err(); | ||
| .unwrap_err() | ||
| .kind(); |
There was a problem hiding this comment.
| .kind(); |
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Jarema
force-pushed
the
jarema/concrete-errors
branch
from
2442cc1 to
02e8959
Compare
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
| } | ||
|
|
||
| impl FlushError { | ||
| fn with_source<E>(kind: FlushErrorKind, source: E) -> FlushError |
There was a problem hiding this comment.
I know this was my naming from slack but since its the only one.. new? 😁
| fn with_source<E>(kind: FlushErrorKind, source: E) -> FlushError | |
| fn new<E>(kind: FlushErrorKind, source: E) -> FlushError |
From FlushErrorKind without a source would just be a blanket From implementation, which you already have.
There was a problem hiding this comment.
In this case, yes, but for other Kind types we will need both methods - with and without source, and From would not always fit.
I would prefer to be consistent and have the naming the same across all errors.
Jarema
force-pushed
the
jarema/concrete-errors
branch
from
258a2ba to
05fe32b
Compare
This PR experiments with concrete error types.
Feel free to share ideas and feedback.