Add more concrete error types

[thomastaylor312]

Proposed change

The error types from the latest version have been a great improvement, but they still squash a lot of useful errors. Before this crate hits 1.0, I'd really like to add some additional error types that map to all the error codes or at least mark the error kind enums as non-exhaustive so things can be added in the future.

Use case

There are several here, but one concrete thing I ran into recently was when creating a KV bucket. If I wanted to check if the bucket existed (via get_key_value) so I could log something to the user if it didn't already exist and then create the bucket, However, the error kind gets swallowed up in the JetStream error type so I can't check for something like BucketDoesNotExist.

Contribution

If I can make some time, I'd love to add at least some of the error kinds

[Jarema]

Thanks for raising this question.

There are few things to discuss here:

Current errors

We purposely didn't expose all possible errors as separate enum variants, while wrapping the underlying details so they are still accesible. The goal was to expose those errors, against which users can do something.
I'm interested to hear what errors do you miss?
If you could give some examples, it would help.
Is it a rare case, or something you need to do often?
Does downcasting on source help?

non-exhaustive errors

We were considering it. However I feel it affects the ergonomics of the client, so we made the call to not have them exhaustive.

1.0.0

I feel that in Rust ecosystem, especially when you are having <1.0 dependencies, it's a hard task to be on >1.0.
So for now, we decided to not land 1.0 technically, as some dependencies can introeduce breaking changes that will affect our API, like rustls which happens already few times.

Additionaly - technically speaking, expanding config structs of streams and consumer configs, or kv config, is a breaking change. And as you know - we do expand them every single Server non-patch release.

Instead, I think, a keeping the client on 0.x version with clear API guarantees is the way to go. The guarantees would be: we will expand struct configs and enum error varians when necessary, we might break TLS direct rustls configs (if rustls introduces breaking changes), but all those changes will be well documented, should be caught during compilation, easy to fix, with info how in each release whenever those happen. Never introduce silent breaking changes.
That way we can also leverage some quality of life improvements happening in async ecosystem, like async traits, or hopefully coming this year - async closures, which would make events callback on connection so much nicer experience.

[thomastaylor312]

All that reasoning sounds fine to me for sure. I think my main goal is that I don't want to have to search an error string to match on an error type. Some errors that I can think of off the top of my head right now:

• Store/stream already exists
• Stream capacity full

Many of the errors here could be useful as well: https://github.com/nats-io/nats-server/blob/ac86e01c1ab345f8f60e0cf9892782aa151eef1b/server/errors.go#L21. I know there are more than that, but I can start keeping track as I run into them and putting them here if that will help.

[Jarema]

I understand your point. However, you should be able to downcast then, rather than match string. I'll write example of that little later and will make sure it's the case. Anyway - it definately make sense to expand some of the enums. The idea is - to expose only those which make sense for user of the library.

[thomastaylor312]

What type do I downcast to? Are you talking about the ErrorCode type for jetstream?

[Jarema]

Yes, that is what I mean. It should be able to downcast to the ErrorCode, which handles all errors returned from the server.

[hseeberger]

So if I have an async_nats::jetstream::kv::UpdateError, which is an alias for async_nats::error::Error, how can I access the source to then downcast to a async_nats::jetstream::Error which gives me the error code?

[Jarema]

@thomastaylor312 @hseeberger I realized while producing example for how to downcast the error that despite many errors having source (sometimes there is simply nothing to source), the Error did not implement the source() method.

Fixed it #1215

However I realized that there are more improvements that can be made.

Example usage that works with the fix:

    #[tokio::test]
    async fn error_source() {
        let server = nats_server::run_server("tests/configs/jetstream.conf");
        let client = ConnectOptions::new()
            .connect(server.client_url())
            .await
            .unwrap();

        let context = async_nats::jetstream::new(client);

        let result = match context
            .create_key_value(async_nats::jetstream::kv::Config {
                bucket: "test".into(),
                history: 10,
                num_replicas: 3,
                ..Default::default()
            })
            .await
        {
            Ok(value) => value,
            Err(err) => {
                println!("error: {}", err);
                println!("kind: {}", err.kind());
                println!("source: {}", err.source().unwrap());
                let js_err = err
                    .source()
                    .unwrap()
                    .downcast_ref::<async_nats::jetstream::context::CreateStreamError>()
                    .unwrap();
                println!("jetstream error: {}", js_err);
                if let CreateStreamErrorKind::JetStream(err) = js_err.kind() {
                    println!("jetstream error code: {:?}", err.error_code());
                }
                return;
            }
        };
    }

[thomastaylor312]

Yeah, that is still a bit rough to do in practice because of all the error/options to handle. Curious what ideas you've had to make it better!