derive_more icon indicating copy to clipboard operation
derive_more copied to clipboard

Errors with lots of string enum types are less readable than with `thiserror`

Open mmastrac opened this issue 6 months ago • 2 comments

Thanks for the awesome library. I've started moving over from thiserror, but there's one thing that's causing some pain. There is occasionally a need for error enums that look like this (note that this enum has been chopped up for brevity):

#[derive(Debug, PartialEq, Eq, derive_more::Display, derive_more::From, derive_more::Error)]
#[allow(clippy::enum_variant_names)]
pub enum ParseError {
    #[display(
        "Invalid DSN: scheme is expected to be either \"postgresql\" or \"postgres\", got {_0}"
    )]
    InvalidScheme(String),

    #[display("Invalid value for parameter \"{_0}\": \"{_1}\"")]
    InvalidParameter(String, String),

    #[display("Invalid percent encoding")]
    InvalidPercentEncoding,

    #[display("Invalid port: \"{_0}\"")]
    InvalidPort(String),

    #[display("Unexpected number of ports, must be either a single port or the same number as the host count: \"{_0}\"")]
    InvalidPortCount(String),

    #[display("Invalid TLS version: \"{_0}\"")]
    #[from]
    InvalidTLSVersion(#[error(source)] SslVersionParseError),
}

However, this fails to compile because String doesn’t implement std::error::Error and therefore cannot be used as an error source (via AsDynError).

I've got three possible solutions that would greatly improve my QoL using this library:

(option 1) It would be really helpful if derive_more::Error could ignore certain common primitive types like String when auto-detecting error sources—perhaps by treating them as non-sources by default, based on type name.

(option 2) If that’s not viable, would it be possible to support an enum-level attribute that lets us explicitly list types to ignore for source detection? For example:

#[error(ignore=String)]
pub enum ParseError {
   ...
}

(option 3) Alternatively, if an enum is marked with #[error(ignore)], perhaps #[error(source)] on individual fields could still override the ignore behavior? Currently, those field-level attributes appear to be silently ignored in such scenarios.

#[error(ignore)]
pub enum ParseError {
    // ... this would this work ...
    #[display("Invalid TLS version: \"{_0}\"")]
    #[from]
    InvalidTLSVersion(#[error(source)] SslVersionParseError),
}

mmastrac avatar Jun 03 '25 15:06 mmastrac

@mmastrac hmm... I've never hit such String-intense cases 🙃

Thank you for the suggestions!

(option 1) It would be really helpful if derive_more::Error could ignore certain common primitive types like String when auto-detecting error sources—perhaps by treating them as non-sources by default, based on type name.

@JelteF what do you think on this? I do like this idea, because it's fairly simple to implement and improves ergonomics in the majority of cases rather than worsens them by introducing some non-obvious attributes.

tyranron avatar Jun 03 '25 20:06 tyranron

Seems reasonable

JelteF avatar Jun 19 '25 21:06 JelteF