Add WithMessagef function

[chemicL]

WithMessagef utility function to accompany WithMessage, similar to
exiting Wrapf function accompanying Wrap.

[jonreyna]

After reviewing the code diffs, and running the unit tests on my OSX Sierra box with go 1.8.3, I'm submitting this code review approval.
I just starting using this library, recognized the need for this new functionality, and was happy to find you beat me to this pull request.
Thanks @chemicL!!!

[chemicL]

Cheers, @JReyLBC. Hopefully @davecheney accepts this :-)

[davecheney]

Hello,

To give you some visibility here. You didn't follow the contribution process, you just threw some code over the wall. That's not cool.

The reason I ask people to submit an issue before writing code is so we discuss it first, then if necessary a change is made.

I understand why you are suggesting adding this feature, but my strong desire for this package is to keep it small. What I am doing is surveying the importers of this package, http://godoc.org/github.com/pkg/errors?importers. As you can see, there are a lot, thousands, and while I won't survey every single use, I do want to see how many are combining fmt.Sprintf with errors.WithMessage. If the number is small, then I will no accept this change. If it is clear that there is a pattern of people having to reach for fmt.Sprintf to use WithMessage, then I'll merge this change.

In future, please follow the contribution guidelines and discuss your change before starting to code.

Thank you.

[dragonsinth]

We had to write a helper to do this when we went through and audited our existing uses of errors.Wrapf(). We had too much stack trace explosion, and it would have been nice to trivially convert this to errors.WithMessagef(). We wrote our own helper because it was too much manual effort to jam a fmt.Sprint() into the middle of each call-- it's not amenable to simple search/replace.

[vbezhenar]

I've found that for my code I would only use WithMessage with parameters. Here's my reasoning:

1. I don't want to add another meaningless stacktrace, it means that err comes from my code with already meaningfull stacktrace.
2. I don't want to just add string message: stacktrace already contains function names and static string won't add much.
3. What I actually do want to add is some variable values to improve context. Stacktrace doesn't include variable values, so they might add important information about error.

In my code I always using WithMessage and fmt.Sprintf together. Not a big deal, but collapsing them into one function is only logical and currently it's inconsistent because every other function has analog with formatting.

[dragonsinth]

Well put @vbezhenar! That captures my feelings exactly.

[natefinch]

I get lazy and use Wrapf when what I really want is WithMessagef. I'm sure I'm not the only one.

[aviau]

I don't want to add another meaningless stacktrace, it means that err comes from my code with already meaningfull stacktrace.

I have the same feeling.

However, how can I know if there is a stack trace already? I don't even bother thinking about it and I use my custom WithMessage/WithMessagef implementations:

(see my errors package here)

package errors

import (
	"fmt"
	pkgerrors "github.com/pkg/errors"
)

//WithMessage annotates an error with a message. Unlike github.com/pkg/errors,
//this implementation also annotates the error with a stack trace if there
//is not already one.
func WithMessage(err error, message string) error {
	if err == nil {
		return nil
	}

	type stackTracer interface {
		StackTrace() pkgerrors.StackTrace
	}

	// Try to find a StackTracer down the error cause chain.
	current := err
	for current != nil {
		if _, ok := current.(stackTracer); ok {
			break
		}
		current = nextCause(current)
	}

	// The error is not yet annotated with a stack trace, Wrap it.
	if current == nil {
		err = pkgerrors.Wrap(err, message)
	}

	return pkgerrors.WithMessage(err, message)
}

//WithMessagef annonates an error with a formatted message
func WithMessagef(err error, format string, args ...interface{}) error {
	if err == nil {
		return nil
	}
	message := fmt.Sprintf(format, args...)
	return WithMessage(err, message)
}

//nextCause returns the direct cause of the error
func nextCause(err error) error {
	type causer interface {
		Cause() error
	}

	if causer, ok := err.(causer); ok {
		return causer.Cause()
	}

	return nil
}


// Unmodified pkgerrors methods
var New = pkgerrors.New
var Errorf = pkgerrors.Errorf

[ondrej-fabry]

I do want to see how many are combining fmt.Sprintf with errors.WithMessage. If the number is small, then I will no accept this change. If it is clear that there is a pattern of people having to reach for fmt.Sprintf to use WithMessage, then I'll merge this change.

@davecheney It won't be that simple to know if people would use it by searching such use. As other already mentioned, people often tend to look for simpler solutions instead of repeating the same thing all over their code.

if err != nil {
    return errors.WithMessage(err, fmt.Sprintf("failed to add %v", x))
}

vs.

if err != nil {
    return errors.WithMessagef(err, "failed to add %v", x)
}

And I believe you wouldn't use the first variant either if it occurred quite handful of times in your code, and you would most likely also implement some helper function for it. Or even wrap the entire errors package with custom implementation like @aviau did.

Please consider accepting this.

[davecheney]

Thank you for this PR.

[dragonsinth]

Yay!