Skip to content
This repository was archived by the owner on Aug 16, 2021. It is now read-only.

Allow disabling Msg variant in ErrorKind #200

Closed
leoschwarz opened this issue Jul 27, 2017 · 8 comments · Fixed by #228
Closed

Allow disabling Msg variant in ErrorKind #200

leoschwarz opened this issue Jul 27, 2017 · 8 comments · Fixed by #228

Comments

@leoschwarz
Copy link

leoschwarz commented Jul 27, 2017

The main reason why I'd want that is to force a more strict handling and categorization of errors in my code, and it's currently my main pain point pushing me towards manual error type definition again. So instead of just writing Err("blabla".into()) (the main problem is that it's rather hard finding all these in your code base) I would have to think about adding or using a variant which describes the exact error better, and would subsequently allow downstream users of my crates to programatically handle some of the possible errors.

I'm not sure if just (optionally) removing the Msg variant would break things like chaining though?

@Yamakaky
Copy link
Contributor

That would be a good thing. Do you want to try a PR?

Not really, you can just use your own variant instead of using a string.

@leoschwarz
Copy link
Author

Ok, I'll try coming up with something in the next couple days.

At first I thought this could be done using a cargo feature, but then I realized this might be an issue if some crates in a dependency graph have it enabled while some don't. So it will probably have to be done in a way that it can be configured using the macro. Personally I would also like if #182 could be disabled individually.

Now I'm not sure if this kind of configuration is possible with the current macros, or if this is blocked on #170.

@Yamakaky
Copy link
Contributor

#170 is more long term, we can improve the macro meanwhile.

@faern
Copy link
Contributor

faern commented Aug 23, 2017

This is a feature I would really like as well. Any well designed library should not throw Msg errors IMO, and thus it should not be a variant in the ErrorKinds for libraries either.

@leoschwarz Did you get anywhere on this yet? Anything I can do to help?

@leoschwarz
Copy link
Author

leoschwarz commented Aug 23, 2017 via email

@Arnavion
Copy link

derive-error-chain now just requires the
Msg(String) variant to be added by hand (i.e. opt in) whereas I feel
this might not be wished here as most people will still want the Msg
variant.

To be clear, derive-error-chain has always needed the variant to be added by hand, since custom derives cannot modify the definition of the item they're applied to. It's just that it used to be required and I've made it optional.

(At one point when proc macros were not yet stabilized, they could modify the item definition, and back then derive-error-chain itself added the Msg member for the user. It's a lucky coincidence in retrospect that being forced to add it then allows it to be optional now.)

@faern
Copy link
Contributor

faern commented Aug 24, 2017

I personally wish we could make it opt-in in error-chain as well. But I realize that would probably break way too much code(?), so I guess the only change that will be acceptable is to change it from a forced variant to one you can opt out of?

Personally I don't see a problem with making it opt-in, since we do have semver to clearly state they are incompatible. But many crate maintainers tend not to agree :)

@faern
Copy link
Contributor

faern commented Aug 29, 2017

I have played with this a bit. But I have not found a nice way to do it yet. Not going to invest too much time since it's a nice to have feature, but not critical.

sgrif added a commit to sgrif/error-chain that referenced this issue Sep 30, 2017
While the `Msg` variant may be a useful option for some users, many will
prefer to stick to more descriptive error types, and won't want this
variant present. This allows a `skip_msg_variant` flag to be passed to
`error_chain!`, which will cause no `Msg` variant to be present in the
generated code.

I've also refactored the body of `impl_error_chain_processing` to not
care about the number of arguments other than the final branch, so more
cases can be added in the future without having to touch as many places
as I did.

Fixes rust-lang-deprecated#200.
sgrif added a commit to sgrif/error-chain that referenced this issue Oct 13, 2017

Verified

This commit was signed with the committer’s verified signature.
While the `Msg` variant may be a useful option for some users, many will
prefer to stick to more descriptive error types, and won't want this
variant present. This allows a `skip_msg_variant` flag to be passed to
`error_chain!`, which will cause no `Msg` variant to be present in the
generated code.

I've also refactored the body of `impl_error_chain_processing` to not
care about the number of arguments other than the final branch, so more
cases can be added in the future without having to touch as many places
as I did.

Fixes rust-lang-deprecated#200.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants