go: proposal: errors/errd: helper package for deferred error handlers

The try proposal (#32437) uses a placeholder fmt.HandleErrorf to annotate errors. We do not intend that to be the actual function name. It was a blank to be filled in. Here is one possible way to fill in that blank.

// Package errd provides error-handling functions.
// All of them are meant to be deferred,
// with the address of the function's error result
// as the first argument. For example:
//
//	defer errd.Trace(&err)
//
// All of the functions take action only when err != nil
// (meaning the function in which the defer appears is
// itself returning a non-nil error); they are no-ops when err == nil.
package errd // import "errors/errd"

// Trace prints the error and the current goroutine's stack trace.
// It does nothing when *errp == nil.
//
// Example:
//
//	defer errd.Trace(&err)
//
func Trace(errp *error)

// Apply applies the function f to the error (*errp = f(*errp)).
// It does nothing when *errp == nil.
//
// Example:
//
//	defer errd.Apply(&err, func(err error) error {
//		if err == io.EOF {
//			err = io.ErrUnexpectedEOF
//		}
//	})
//
func Apply(errp *error, f func(err error) error)

// Add adds context to the error.
// The result cannot be unwrapped to recover the original error.
// It does nothing when *errp == nil.
//
// Example:
//
//	defer errd.Add(&err, "copy %s %s", src, dst)
//
// This example is equivalent to:
//
//	defer errd.Apply(&err, func(err error) error {
//		err = fmt.Errorf("copy %s %s: %v", src, dst)
//	})
//
// See Wrap for an equivalent function that allows
// the result to be unwrapped.
func Add(errp *error, format string, args ...interface{})

// Wrap adds context to the error and allows
// unwrapping the result to recover the original error.
//
// Example:
//
//	defer errd.Wrap(&err, "copy %s %s", src, dst)
//
// This example is equivalent to:
//
//	defer errd.Apply(&err, func(err error) error {
//		err = fmt.Errorf("copy %s %s: %w", src, dst)
//	})
//
// See Add for an equivalent function that does not allow
// the result to be unwrapped.
func Wrap(errp *error, format string, args ...interface{})

Thoughts and improvements welcome.

/cc @mpvl @jba @neild

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 21
  • Comments: 39 (19 by maintainers)

Most upvoted comments

defer+err = deferr, obviously.

Of course, that stutters (defer deferr.Trace(...)), so the obvious fix is to allow zero-width spaces and call this package r: defer​r.Trace(…).

Why do you feel the need for a separate package, rather than putting these in errors? (Aside from the implementation detail of accessing formatting.)

I don’t see why Trace is here. It has a few problems:

  • It’s generally a mistake to both print an error and return it. That typically leads to multiple output lines per error, creating log spam.
  • Where does Trace print to? Stdout? Stderr? The log? I usually log errors when I print them, but a command-line tool might prefer stderr. If this package is only going to have one function for this, I don’t see an obvious best choice.
  • Printing the full stack trace will likely lead to duplicate traces, since the error is being returned up the stack where other Trace calls can get ahold of it. Consider printing just the current frame.

Shouldn’t the two formatting functions end in f?

I’ve seen the name Annotatef for what you’re calling Add. That is a bit long, but Notef isn’t, and pairs nicely with Wrapf.

I made a quick hack and the results are interesting:

https://gitlab.com/beoran/errd

So now I can handle an error with where closer has a Close method like this: defer errd.New(&err).Add("closer %v", closer).Close(closer).Log().Do() The Do() was neccesary to avoid having to wrap everything in a func(){} block. Perhaps even that could be avoided by chaining lambdas in stead of structs. But for the rest, the power is amazing. 😃

The potential is limitless, there could even be .If(), .Switch(), so the error handling could get as complex as I would like it to be. But I guess not everyone likes method chaining, so maybe it’s better that this remains an alternate package, much like how we have std lib log and logrus.

What is the rationale for the name? errors/errd doesn’t say anything to me. Is it supposed to be short for error-defer? What about something like errors/deferred instead? Could still be aliased to errd if deferred isn’t an obvious namespace.

I don’t think Apply carries its weight. (It seems helpful here largely to describe how Add and Wrap work.)

The example code isn’t quite right; I think you meant:

defer errd.Apply(&err, func(err error) error {
	if err == io.EOF {
		return io.ErrUnexpectedEOF
	}
	return err
})

But if I were going to do that, I would instead write

defer func() {
	// Don't even need a specific nil check in this case.
	if err == io.EOF {
		err = io.ErrUnexpectedEOF
	}
}()

That is clearer and faster: it only need refer to the single err variable; it doesn’t need the err != nil check at all since it only cares about io.EOF; it only involves a single function call instead of two; it’s lighter on the page; and so on.

If you had a more complicated function to apply, it seems fine to just make it take a *error and do the if *err == nil { return } check itself:

func f() (err error) {
	defer errd.Apply(&err, complexFunc1)
	defer complexFunc2(&err)
	 // ...
}

func complexFunc1(err error) error {
	// lots of stuff
	return err
}

func complexFunc2(errp *error) {
	if *errp == nil {
		return
	}
	// lots of stuff
}

The version without Apply isn’t obviously better here, as above, but it’s not clearly worse either.

Thanks!

I’d like to see this as part of errors. errors.Trace(), errors.Apply(), errors.Add(), & errors.Wrap() read much better than errd.Func()

Whether or not try() is going to be implemented, this errd package is a great idea, because, I have to admit, that after thinking about it, using defer for error handlers makes sense in many cases, as long as performance is not critical.

Unlike @cespare I think the errd.Apply() is a great idea because it allows to define named and resuable error handlers in advance and then just pass them to errd.Apply(), without having to define an inline lambda. A few common use cases could have predefined error handlers in the standard library. For example, to close a file on an error, we could have a method func (f File) CloseErr(err error) error, which closes the file if err is not nil, and then we could do defer errd.Apply(file.CloseErr) . The CloseErr function could then be implemented for every standard library type that has a Close(), to encourage this pattern.

One use case which seems to be missing is logging the error using the log package. I think and errd.Log() function could be very useful.

However, I think there is one way that would make this even more useful and that would be to make it a “fluid” API, a bit like this:

package errd // import "errors/errd"

type errd  * error

func New(err * error) errd {
   return errd(err)
}

func (e errd) Trace() errd 
func (e errd) Log() errd
func (e errd) Logf(fmt string, args...interface{}) errd
func (e errd) Apply( f func(err error) error) errd
func (e errd) Add(errp *error, format string, args ...interface{}) errd
func (e errd) Wrap(errp *error, format string, args ...interface{}) errd

Then we can easily compose error handlers like this: defer errd.New(&err).Log().Add("%s", "some context").Handle(file.CloseErr)

What is the fate of this proposal now that https://github.com/golang/go/issues/32437 has been declined? It is of course still possible to use defer to handle errors even without try, but the value of doing so may need to be re-evaluated.

After all, if the idea of doing error handing using defer() is now to become the preferred way in Go

@beoran as someone who does not have the spare time to ramp up 100% on this new error handling stuff, the current error handling proposals are frankly, terrifying to me from a complexity standpoint if the above statement is accurate. Note that my perspective coming from that of a new user looking at this for the first time, maybe second time. I would be very surprised if other people did not share this opinion under the premise that they did not want to aimlessly re-iterate this concern, especially without concrete feedback and reasoning for their claims.

I thought your experiment was useful, even though I did not agree with it, because it was able to provide a discussion point of what could happen once other users started to adopt this idiom.

Food for thought: my initial reaction to this is simply the question of how this is better than exception handling as a whole. In addition to looking very complex, it also looks unfamiliar.

I see several people dislike my experiment. I have a feeling why this might be so, although I’d prefer to hear why. After all, if the idea of doing error handing using defer() is now to become the preferred way in Go, then people will start using libraries like the one I wrote, probably even more powerful and convoluted, because of the convenience gained, despite the loss of readability. I myself had never considered to do error handling in defer, but now that I know this clever hack, I can see the appeal of it. But I’m not sure myself that this way is better than just an if statement. 😕

I am generally in favor of this proposal, but share concerns about the name. That said, it’s not an easy package to name! I’m wondering about handle or errhandle?

defer handle.Trace(&err)
defer handle.Add(&err, "copy %s %s", src, dst)
defer handle.Wrap(&err, "copy %s %s", src, dst)

@cespare, I agree that Apply doesn’t carry its weight for func literals. It might if people write other (named) helper functions.

It only helps insofar is it allows such helpers have the signature func(error) error rather than func(*error) (and skip the nil check). And the call site is a little more clunky:

defer errd.Apply(&err, handle)
// vs
defer handle(&err)

So even then it seems like a wash. I’d rather establish the convention that custom error helpers have the signature func(*error, ... other args ...), just like Trace/Add/Wrap.

@johanbrandhorst, Yes, errd is short for err-defer. If these are going to be used a lot, it makes sense for them to have a short name (like fmt.Printf not format.Printf). And if the package were errors/deferred then the uses would look like:

defer deferred.Trace(&err)

which is a bit repetitive.

I personally like the idea of try(f(), handler) more, but my only concern with this proposal is the introduction of a new package when we already have the errors helper package.

There is no errors.Wrap in stdlib, only .Unwrap.

Update 2: I had been confused and believed Dave Cheney’s pkg/errors had become part of stdlib, and that the weaknesses of fmt.Errorf before Go 1.13 still applied. But now, having found the right document I see errors.Wrap is not part of the stdlib, and the correct way to achieve wrapping is to use fmt.Errorf with %w and not %v Now I can see! Thanks for your patience. 😄


Update: Maybe the reason RSC used fmt.Errorf() and not errors.Wrap() for the examples in this proposal was because this issue was filed 6/18/19 before Go 1.13 was released, on 9/3/19, where the new errors.Wrap() may have first appeared??? So even the prevailing style when this was filed may have been fmt.Errorf vs errors.Wrap ???


@rsc

I just shared a comment about this issue on another project, as that project has started adopting some of the code-snippets from this issue. I am trying to understand why RSC used fmt.Errorf() vs errors.Wrap() for the examples. For the robust server I am trying to create there seems to be advantages to having a full error chain as created by errors.Wrap() as opposed to the long string created by fmt.Errorf() Thanks for any feedback or insight.


Here is the link to the question I posted mentioning this issue: Why are we using fmt.Errorf vs errors.Wrap?

I’ll chime in with greatly preferring try(f(), handler). Handlers like this are clearly scoped to the single call being tried, a defer ... that manipulates err is not:

func() (err error) {
  defer errd.Apply(&err, func(err error) error {
    // this check will apply to errs from both fn1, and fn2's potentially-applied-err,
    // and any new code that appears below it in the future
    if err == ... {
      err = ...
    }
  })
  _, err = try(fn1())

  defer errd.Apply(&err, func(err error) error {
    // currently only applies to fn2's err
    if err == ... {
      err = ...
    }
  })
  _, err = try(fn2())
}

It seems much safer and easier to me to pass the same fn to try(..., handler) than to require some kind of “only apply this defer-err-thing to the immediate err, not all” logic. TBH I’m not even sure what that kind of defer would look like, especially for it to be a safe pattern for multiple locations / nested errs (e.g. from a try-defer’d func call’s result). It needs to avoid wrapping “itself” while simultaneously wrapping “itself” if the wrapped err came from a fn call rather than just an earlier defer in the same scope. Certainly it’s possible, but likely annoying / strange looking.

I’d prefer to hear why.

I’m actually fond of fluent APIs myself, but I don’t think this one is necessary. In the relatively rare cases where you need multiple error handlers, you can just write multiple defers. That is simpler, and there is no danger of forgetting to call Do.

Another problem is that the only way to add another action is to modify your package.

I read the name as “erred”, as in to err — someone erred, now it’s time to deal with consequences.

The name Trace suggests that it is reasonable to call it as you return errors up the stack, but that would lead to a lot of duplicate stack traces. Like @jba I suspect that Trace ought to report just the current function, and a different function, perhaps Stack, would dump the full stack trace. I don’t know if we need both.

@jba, I would expect everything in errors to be about errors and not about error pointers. Putting the deferrables in their own package lets them have a different name space. I think that solves one of the big naming problems we had with clunkers like fmt.HandleErrorf.

Simple question as I am obviously missing something. Why a pointer to error when error is already an interface?

Also, does errd mean “ERRor Defer?” Being able to read abbreviations as if not abbreviated always helps me reason about code.

errd.Tracef might be natural as well, or having the current errd.Trace also take format string, args ...interface{}.