Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No apparent way to have an error enum variant documentation generated from a display string #445

Open
hgomersall opened this issue Mar 18, 2024 · 5 comments

Comments

@hgomersall
Copy link

hgomersall commented Mar 18, 2024

If one wishes to have the enum variant documented in the library documentation (i.e. from cargo doc) as well as having nice run-time error messages, my understanding from the documentation and playing around a bit is that one must have two separate lines - a doc line and a snafu(display()) line. I find myself repeatedly doing this and having to replicate the error string twice with minor perturbations which is a bit irritating.

So this is what I often write most commonly:

#[derive(Debug, Snafu)]
pub enum FooBarError {
    /// The foo-bar will not frobinate properly given the alignment of splines.
    #[snafu(display("The foo-bar will not frobinate properly given the alignment of splines: {source}"))]
    FooBarFrobinationFailure { source: SomeSourceError },
}

Or a slightly less common, but still pretty common:

#[derive(Debug, Snafu)]
pub enum FooBarError {
    /// The foo-bar will not frobinate properly given the alignment of splines.
    #[snafu(display("The foo-bar will not frobinate properly given the alignment of splines ({spline_alignment})."))]
    FooBarFrobinationFailure { spline_alignment: SplineAlignment },
}

Both of these result in a load of extra writing to get both the doc string and the error string.

I would be nice if the two could be consolidated somewhat and boiler plate reduced. Something like the following might be nice:

#[derive(Debug, Snafu)]
pub enum FooBarError {
    /// The foo-bar will not frobinate properly given the alignment of splines.
    #[snafu(display_doc_with_source)]
    FooBarFrobinationFailure { source: SomeSourceError },
}

which implicitly appends the source to the doc string error to create the display

And a more general version:

#[derive(Debug, Snafu)]
pub enum FooBarError {
    /// The foo-bar will not frobinate properly given the alignment of splines.
    #[snafu(display_doc_with("(for spline alignment {spline_alignment})"))]
    FooBarFrobinationFailure { spline_alignment: SplineAlignment },
}

If I missed something obvious, please do let me know!

@Enet4
Copy link
Collaborator

Enet4 commented Mar 18, 2024

which implicitly appends the source to the doc string error to create the display

That would be ill advised as per the recommendations from the error handling WG. If an error contains an exposed source error (the default when the field is named source), then displaying the error should not include the source error's message. Using a proper error reporter will already give you the error's cause chain in a nicer format with greater flexibility. Snafu itself used to suggest that approach, but the examples have been changed to no longer append the source message to the error type.

The following example would follow this recommendation by using an error reporter and letting it traverse the chain to eventually expand the message implemented by SomeSourceError.

#[derive(Debug, Snafu)]
pub enum FooBarError {
    /// The foo-bar will not frobinate properly given the alignment of splines
    FooBarFrobinationFailure { source: SomeSourceError },
}

On the second example, The following is succinct and should give you a sufficiently clear message.

#[derive(Debug, Snafu)]
pub enum FooBarError {
    /// The foo-bar will not frobinate properly for spline alignment {spline_alignment}
    FooBarFrobinationFailure { spline_alignment: SplineAlignment },
}

A sub-macro to concatenate the doc-comment with an additional templated string could probably exist (though I'll let @shepmaster chime in because macros are not my forte), but one may question the added value of introducing this construct. We would be increasing the complexity of the macro to save a few words, right?

@hgomersall
Copy link
Author

That would be ill advised as per the recommendations from the error handling WG.

That's interesting. I wasn't familiar with those recommendations for the error handling WG. So presumably Snafu implements the Error trait pointing fn source() at the inner source if it's present? What would be a decent error reporter library?

On the second example, The following is succinct and should give you a sufficiently clear message.

#[derive(Debug, Snafu)]
pub enum FooBarError {
    /// The foo-bar will not frobinate properly for spline alignment {spline_alignment}
    FooBarFrobinationFailure { spline_alignment: SplineAlignment },
}

Does that work? I.e will Snafu translate that into a correct error string? The generated docs show the format string as {spline_alignment} (as one would expect).

We would be increasing the complexity of the macro to save a few words, right?

You're not really wrong - I've been doing these errors for years without feeling the need to raise an issue, so it's clearly marginal, but I finally got fed up with repeating myself today!

@Enet4
Copy link
Collaborator

Enet4 commented Mar 18, 2024

That would be ill advised as per the recommendations from the error handling WG.

That's interesting. I wasn't familiar with those recommendations for the error handling WG. So presumably Snafu implements the Error trait pointing fn source() at the inner source if it's present?

Yes, and you can select another field when it is not named source.

What would be a decent error reporter library?

You can use the report API in snafu itself. Other error handling libraries such as anyhow and eyre also offer reports, though they do not differ that much from Snafu's.

Does that work? I.e will Snafu translate that into a correct error string? The generated docs show the format string as {spline_alignment} (as one would expect).

I proposed that myself. :)

@hgomersall
Copy link
Author

Ok, I think this is sufficient workaround for my irritation. It will be interesting to hear if @shepmaster has an opinion, but I think I'm broadly happy.

Great work on Snafu in any case (a happy user since 2019)!

@shepmaster
Copy link
Owner

As my initial thought, I'm hesitant to add something like "combine the docstring with some more text to generate the Display format string". It feels relatively uncommon that that's exactly what you want.

For example, maybe my docstring is "hello world" and I want the error to be "welcome to the hello world error" — now I want to prepend and append. Or maybe I need to change some casing or punctuation in the middle anyway.

That being said, having the {foo} text in the docstring always felt odd, and some way of avoiding that would be nice. I wish there was a way to trivially grep the code base of everyone that uses SNAFU to see how prevalent using the docstring as a prefix string of the error string was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants