4 ms·

I don't quite understand the issue about public error enums? Distinguishing variants is very useful if some causes are recoverable or - when writing webservers

by the8472 10mo ago

I don't quite understand the issue about public error enums? Distinguishing variants is very useful if some causes are recoverable or - when writing webservers - could be translated into different status codes. Often both are useful, something representing internal details for logging and a public interface.

IshKebab 10mo ago
I agree. Is he really trying to say that e.g. errors for `std::fs::read()` should not distinguish between "file not found" and "permission denied"? It's quite common to want to react to those programmatically. IMO Rust should provide something like thiserror for libraries, and also something like anyhow for applications. Maybe we can't design a perfect error library yet, but we can do waaay better than nothing. Something that covers 99% of uses would still be very useful, and there's plenty of precedent for that in the standard library.
- burntsushi 10mo ago
  I doubt epage is suggesting that. And note that in that case, the thing distinguishing the cause is not `std::io::Error`, but `std::io::ErrorKind`. The latter is not the error type, but something that forms a part of the I/O error type. It's very rare that `pub enum Error { ... }` is something I'd put into the public API of a library. epage is absolutely correct that it is an extensibility hazard. But having a sub-ordinate "kind" error enum is totally fine (assuming you mark it `#[non_exhaustive]`).
  - the8472 10mo ago
    It's not uncommon to have it on the error itself, rather than a details/kind auxiliary type. AWS SDK does it, nested even [0][1], diesel[2], password_hash[3]. [0] https://docs.rs/aws-smithy-runtime-api/1.9.3/aws_smithy_runtime_api/client/result/enum.SdkError.html https://docs.rs/aws-smithy-runtime-api/1.9.3/aws_smithy_runt... [1] https://docs.rs/aws-sdk-s3/1.119.0/aws_sdk_s3/operation/get_object/enum.GetObjectError.html https://docs.rs/aws-sdk-s3/1.119.0/aws_sdk_s3/operation/get_... [2] https://docs.rs/diesel/2.3.5/diesel/result/enum.Error.html https://docs.rs/diesel/2.3.5/diesel/result/enum.Error.html [3] https://docs.rs/password-hash/0.5.0/password_hash/errors/enum.Error.html https://docs.rs/password-hash/0.5.0/password_hash/errors/enu...
    - burntsushi 10mo ago
      Sure, I've done it too: https://docs.rs/ignore/latest/ignore/enum.Error.html https://docs.rs/ignore/latest/ignore/enum.Error.html It's not necessarily about frequency, but about extensibility. There's lot of grey area there. If you're very certain of your error domain and what kinds of details you want to offer, then the downside of a less extensible error type may never actualize. Similarly, if you're more open to more frequent semver incompatible releases, then the downside also may never actualize.
  - IshKebab 10mo ago
    Why is it an extensibility hazard (assuming you mark the `pub enum Error` as non-exhaustive)? I mean I don't see the difference between having the non-exhaustive enum at the top level vs in a subordinate 'kind'.
    - epage 10mo ago
      Take the example at https://docs.rs/thiserror/latest/thiserror/ https://docs.rs/thiserror/latest/thiserror/ - Struct variant fields are public, limiting how you evolve the fields and types - Struct variants need non_exhaustive - It shows using `from` on an error. What happens if you want to include more context? Or change your impl which can change the source error type None of this is syntactically unique to errors. This becomes people's first thought of what to do and libraries like thiserror make it easy and showcase it in their docs.
      - the8472 10mo ago
        Having private variants and fields would be useful, yeah. Std cheats a bit with its ErrorKind::Uncategorized unstable+hidden variant to have something unmatchable.
        IshKebab 10mo ago
        Probably because non-exhaustive enums didn't exist when it was written?
      - IshKebab 10mo ago
        > Struct variant fields are public, limiting how you evolve the fields and types But the whole point of thiserror style errors is to make the errors part of your public API. This is no different to having a normal struct (not error related) as part of your public API is it? > Struct variants need non_exhaustive Can't you just add that tag? I dunno, I've never actually used thiserror. Your third point makes sense though.
        epage 10mo ago

by the8472 10mo ago

IshKebab 10mo ago

I agree. Is he really trying to say that e.g. errors for `std::fs::read()` should not distinguish between "file not found" and "permission denied"? It's quite common to want to react to those programmatically. IMO Rust should provide something like thiserror for libraries, and also something like anyhow for applications. Maybe we can't design a perfect error library yet, but we can do waaay better than nothing. Something that covers 99% of uses would still be very useful, and there's plenty of precedent for that in the standard library.

burntsushi 10mo ago
I doubt epage is suggesting that. And note that in that case, the thing distinguishing the cause is not `std::io::Error`, but `std::io::ErrorKind`. The latter is not the error type, but something that forms a part of the I/O error type. It's very rare that `pub enum Error { ... }` is something I'd put into the public API of a library. epage is absolutely correct that it is an extensibility hazard. But having a sub-ordinate "kind" error enum is totally fine (assuming you mark it `#[non_exhaustive]`).
- the8472 10mo ago
  It's not uncommon to have it on the error itself, rather than a details/kind auxiliary type. AWS SDK does it, nested even [0][1], diesel[2], password_hash[3]. [0] https://docs.rs/aws-smithy-runtime-api/1.9.3/aws_smithy_runtime_api/client/result/enum.SdkError.html https://docs.rs/aws-smithy-runtime-api/1.9.3/aws_smithy_runt... [1] https://docs.rs/aws-sdk-s3/1.119.0/aws_sdk_s3/operation/get_object/enum.GetObjectError.html https://docs.rs/aws-sdk-s3/1.119.0/aws_sdk_s3/operation/get_... [2] https://docs.rs/diesel/2.3.5/diesel/result/enum.Error.html https://docs.rs/diesel/2.3.5/diesel/result/enum.Error.html [3] https://docs.rs/password-hash/0.5.0/password_hash/errors/enum.Error.html https://docs.rs/password-hash/0.5.0/password_hash/errors/enu...
  - burntsushi 10mo ago
    Sure, I've done it too: https://docs.rs/ignore/latest/ignore/enum.Error.html https://docs.rs/ignore/latest/ignore/enum.Error.html It's not necessarily about frequency, but about extensibility. There's lot of grey area there. If you're very certain of your error domain and what kinds of details you want to offer, then the downside of a less extensible error type may never actualize. Similarly, if you're more open to more frequent semver incompatible releases, then the downside also may never actualize.
- IshKebab 10mo ago
  Why is it an extensibility hazard (assuming you mark the `pub enum Error` as non-exhaustive)? I mean I don't see the difference between having the non-exhaustive enum at the top level vs in a subordinate 'kind'.
  - epage 10mo ago
    Take the example at https://docs.rs/thiserror/latest/thiserror/ https://docs.rs/thiserror/latest/thiserror/ - Struct variant fields are public, limiting how you evolve the fields and types - Struct variants need non_exhaustive - It shows using `from` on an error. What happens if you want to include more context? Or change your impl which can change the source error type None of this is syntactically unique to errors. This becomes people's first thought of what to do and libraries like thiserror make it easy and showcase it in their docs.
    - the8472 10mo ago
      Having private variants and fields would be useful, yeah. Std cheats a bit with its ErrorKind::Uncategorized unstable+hidden variant to have something unmatchable.
      - IshKebab 10mo ago
        Probably because non-exhaustive enums didn't exist when it was written?
    - IshKebab 10mo ago
      > Struct variant fields are public, limiting how you evolve the fields and types But the whole point of thiserror style errors is to make the errors part of your public API. This is no different to having a normal struct (not error related) as part of your public API is it? > Struct variants need non_exhaustive Can't you just add that tag? I dunno, I've never actually used thiserror. Your third point makes sense though.
      - epage 10mo ago