fault icon indicating copy to clipboard operation
fault copied to clipboard

Redesign fmsg to be less verbose and more useful.

Open Southclaws opened this issue 2 years ago • 3 comments

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.

Southclaws avatar Jun 05 '23 08:06 Southclaws

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

matdurand avatar Jun 09 '23 14:06 matdurand

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)

matdurand avatar Jun 20 '23 01:06 matdurand

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...

Southclaws avatar Jul 03 '23 12:07 Southclaws