rotisserie/eris

Is there a way to indicate a skip level for stack in eris ?

magicalo opened this issue · 6 comments

Is your feature request related to a problem? Please describe.
I would like to call eris.Wrap inside a utility Logging function but have it attribute the stack to where my function is called vs where .Wrap is called. (e.g. a typical skip level arg)

Describe the solution you'd like
I would prefer to call eris.Wrap inside mypackage.LogError instead of passing it in. But have eris skip a stack level and report one level above instead of inside my utility function.

So imagine I have mypackage.LogError(eris.Wrap(err, "blah"));
But instead I want to hide the eris.Wrap call inside mypackage.LogError(err, "blah") so that I don't have to litter my code with eris.Wrap everywhere.
I want to tell eris.Wrap to skip the immediate stack frame and record the one above as the frame.

Describe alternatives you've considered
Putting mypackage.LogError(eris.Wrap()) all over my code.

Additional context
Add any other context or screenshots about the feature request here.

Thanks for the feature request @magicalo! I can’t think of an automated way of doing this at the moment but I suppose we could add another method for this use case. I need to think about it a bit more but I’ll get back to you soon.

Sounds good.
Maybe SkipWrap method?

func SkipWrapf(skiplevel int, err error, format string, args ...interface{}) error {
	return wrap(skiplevel, err, fmt.Sprintf(format, args...))
}

func wrap(skiplevel int, err error, msg string) error {
	if err == nil {
		return nil
	}

	// callers(4+skiplevel) skips runtime.Callers, stack.callers, this method, SkipWrap(f) and skiplevel methods above
        //use this if you are wrapping inside a utility logging function, for example.
	stack := callers(4+skiplevel)
...

Yeah, I was thinking of something like that as well. Still trying to think of any alternatives but if I can't think of anything better, I'd be willing to add it.

Hey @magicalo, just letting you know I think I'm going to support this and should have something for you in a few days. Thanks again for the feature request!

After looking into this a bit more, I'm not comfortable with this implementation. Exposing that skip arg leaves the possibility of a panic. For example, if someone were to call WrapSkip(5, err, "blah"), it would panic if there are only 4 callers. As far as I know, there's no good way to guard against it or detect how many callers there are.

Generally, error wrapping is meant to be done at the source anyway. I could possibly consider adding a WrapSkip method that simply skips one extra frame, but I'm a little wary of littering the public interface with things like that. Still need to think about it more.

Closing for now. Willing to reopen if there is more interest in this or if someone suggests a safer implementation.