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

Redesign fmsg to be less verbose and more useful. #32

Open
Southclaws opened this issue Jun 5, 2023 · 3 comments
Open

Redesign fmsg to be less verbose and more useful. #32

Southclaws opened this issue Jun 5, 2023 · 3 comments

Comments

@Southclaws
Copy link
Owner

A common use-case for Fault is to wrap user-facing errors with fmsg.WithDesc where the second argument is a well-formed sentence of copy intended to be displayed to non-technical end-users.

One issue with this API is that it conflates internal error message wrapping with end-user copy.

It has two APIs:

  • fmsg.With for serving the same purpose as errors.Wrap(err, "message")
  • fmsg.WithDesc which does the same as above but with one extra argument: a message for the end-user.

This frequently results in code that looks like this:

return nil, fault.Wrap(fault.New("user account not found"),
	fmsg.WithDesc("not found",
		"No account could be found with the specified email address."),

And when the error is logged, the flattened form is:

not found: user account not found

Which is useless duplication of concepts, adding noise to the logs and making tracking issues more annoying.

There are two problems here:

  • When declaring a new error, you do not need to immediately provide a "contextual" internal error message
  • When you want to decorate an error chain with a message intended for the end-user, you're forced to provide a "contextual" internal error message

So I think it would be best if either fmsg was split into two packages or it provided two separate APIs for internal and external error messages. Something like:

return nil, fault.Wrap(fault.New("user account not found"),
	fmsg.WithExternal("No account could be found with the specified email address."),

For when you're creating a brand new error that already has an internal message (user account not found)

Or, for when you're wrapping an existing error and wish to provide some additional internal context as well as an external end-user message:

return nil, fault.Wrap(err,
	fmsg.WithInternal("failed to verify signature"),
	fmsg.WithExternal("Your signature could not be verified, please try again."),

I'm not set on WithInternal and WithExternal though, I'd prefer to keep short concise names that don't add too much noise to error decoration sites.

@matdurand
Copy link
Contributor

Btw the readme is wrong, using With instead of WithDesc in this example
image

@matdurand
Copy link
Contributor

matdurand commented Jun 20, 2023

After a bit of use, I agree with this assessment. Having both the internal and external message using the same withDesc is often repetitive in my use cases.

What about WithMsg and WithFriendlyMsg (or even WithFriendly as an alias for simplicity)

@Southclaws
Copy link
Owner Author

WithFriendly seems nice, I'd like to write a new package for this really with a more well thought out API. Don't want to break the v1 promise though...

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

2 participants