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

Error wrappers #6

Open
achille-roussel opened this issue Mar 29, 2018 · 10 comments
Open

Error wrappers #6

achille-roussel opened this issue Mar 29, 2018 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@achille-roussel
Copy link
Contributor

achille-roussel commented Mar 29, 2018

(based on feedback from @andreiko)

We need a better way to compose or chain error wrappers, one idea that came in a discussion would be to use a Wrapper type which would look like

type Wrapper interface {
  Wrap(error) error
}

Such wrappers would be composable with a function like

func With(err error, wrappers ...Wrapper) error

Which would apply wrappers to an error and return the final result. Using the function would look like

errors.With(err,
  errors.Message("hello"),
  errors.Types("A", "B"),
  errors.Tags(...),
  ...
)

While this form of chaining is very flexible and allows it is also a bit verbose, the package name gets repeated over and over. Another approach could be to use the Wrap function as a constructor for an Error interface type which implements both error and methods to chain the wrap operations, for example:

type Error interface {
  error
  WithMessage(msg string) Error
  WithTypes(types ...string) Error
  WithTags(tags ...Tag) Error
  ...
}

which would be used as

errors.Wrap(err, "hello").
  WithTypes(...).
  WithTags(...)

May be best to experiment with one approach first, or provide both in the first place.

Feedback are welcome!

@abraithwaite
Copy link

abraithwaite commented Mar 29, 2018

Instead of:

func With(err error, wrappers ...Wrapper) error

I personally prefer the signature:

func With(wrappers ...Wrapper) func(error) error

so that you can compose them nicely with a function with a signature similar to:

func(fmt string, wrappers ...func(error) error)

Replace fmt with whatever problem specific args you need to supply. Great example of where this is used elsewhere is http middleware chaining.

@achille-roussel
Copy link
Contributor Author

I like this idea! 👍

What's your take on

func With(wrappers ...Wrapper) Wrapper

vs

func With(wrappers ...Wrapper) func(error) error

?

@andreiko
Copy link
Contributor

The first solution seems to be easier to implement and maintain. The problem is that appropriate package-level names like Tags, Stack and Types are already taken.

My best shot at the alternative names is this:

	return errors.With(
		err,
		errors.Prefix("message processing failed"), // calls Wrap
		errors.TypeList("Temporary"),
		errors.TagList(errors.T("t1", "v1")),
	)

Returning a richer interface from functions on the other hand would allow to use more semantic names: errors.Wrap(err, "message processing failed").WithTypes("Temporary").WithTags(errors.T("t1", "v1"))

@achille-roussel
Copy link
Contributor Author

achille-roussel commented Mar 30, 2018

@andreiko

How about a naming scheme like

func MessageWrapper(msg string) Wrapper { ... }

func TypesWrapper(types ...string) Wrapper { ... }

func TagsWrapper(tags ...Tag) Wrapper { ... }

?

A bit verbose but naming consistency may make the API friendlier to developers.

@andreiko
Copy link
Contributor

That's pretty semantical, too. An implementation could look like this:

package errors

type Wrapper interface {
	Wrap(error) error
}

func With(err error, wrappers ...Wrapper) error {
	for _, wrapper := range wrappers {
		err = wrapper.Wrap(err)
	}
	return err
}

func TagsWrapper(tags ...Tag) Wrapper {
	return tagsWrapper(tags)
}

type tagsWrapper []Tag

func (w tagsWrapper) Wrap(err error) error {
	return WithTags(err, []Tag(w)...)
}

func TypesWrapper(types ...string) Wrapper {
	return typesWrapper(types)
}

type typesWrapper []string

func (w typesWrapper) Wrap(err error) error {
	return WithTypes(err, []string(w)...)
}

func MessageWrapper(msg string) Wrapper {
	return messageWrapper(msg)
}

type messageWrapper string

func (w messageWrapper) Wrap(err error) error {
	return wrap(err, 3, string(w))
}

@andreiko
Copy link
Contributor

Another tricky part here is to know how many frames wrap() should skip. I used 3 in the example and it would work if someone calls Wrap with a messageWrapper as an argument, but the package API doesn't forbid calling it directly from client's code:

	return errors.MessageWrapper("message processing failed").Wrap(err)

In which case it would be appropriate to skip only 2 frames.

@achille-roussel
Copy link
Contributor Author

While I agree it’s nice to get the frame stack right, I wouldn’t worry too much about showing one too many frames. People can figure it out when reading the stack trace, and we can still solve this problem later on.

@abraithwaite
Copy link

abraithwaite commented Mar 30, 2018

What's your take on...

Either work fine for me :-)

Can always have

type WrapperFunc func(error) error

func (w WrapperFunc) Wrap(err error) error {
    return w(err)
}

@deankarn
Copy link

I know this is old but thought I'd post the solution @andreiko and I had come up with https://github.com/go-playground/errors

It avoids all the wrapping and much of the complexity.

@andreiko
Copy link
Contributor

andreiko commented Feb 22, 2019

I humbly refuse to take credit for that ☝️It was your solution @joeybloggs.

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

No branches or pull requests

7 participants