errors icon indicating copy to clipboard operation
errors copied to clipboard

PROPOSAL: Allow hiding part of the stack trace

Open decibel opened this issue 8 years ago • 9 comments

Taking inspiration from how pkg/errors is meant as a drop-in replacement for the standard errors package, I have created an errors package that's specific to a larger project I'm involved in. That makes it easy to customize error behavior in the future. This package is mostly thin wrappers around pkg/errors.

The one downside to this is the stacks end up with an extra layer. Obviously I could fix that, but as far as I can tell it would require a fair amount of code duplication with pkg/errors.

I think there is a fairly easy way around this though; add an exposedStack *stack to withStack{}, and allow for callers to modify the boundaries of exposedStack. By default, exposedStack would be stack[:] so everything would work as today.

I think this would also make it easier for someone to satisfy #111, since they wouldn't need to duplicate any of the printing or other interfaces.

decibel avatar Aug 04 '17 17:08 decibel

Thanks you for your proposal. I can see what you want to do, but not how to do itm can you show some code to demonstrate what happens today, and what could be changed to address that in the future.

Thanks

Dave

On Sat, 5 Aug 2017, 03:05 Jim Nasby [email protected] wrote:

Taking inspiration from how pkg/errors is meant as a drop-in replacement for the standard errors package, I have created an errors package that's specific to a larger project I'm involved in. That makes it easy to customize error behavior in the future. This package is mostly thin wrappers around pkg/errors.

The one downside to this is the stacks end up with an extra layer. Obviously I could fix that, but as far as I can tell it would require a fair amount of code duplication with pkg/errors.

I think there is a fairly easy way around this though; add an exposedStack *stack to withStack{}, and allow for callers to modify the boundaries of exposedStack. By default, exposedStack would be stack[:] so everything would work as today.

I think this would also make it easier for someone to satisfy #111 https://github.com/pkg/errors/issues/111, since they wouldn't need to duplicate any of the printing or other interfaces.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/pkg/errors/issues/129, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAcA6MB-hhnwXb2TZRHAOnODjqf-FrHks5sU09TgaJpZM4Ot9ev .

davecheney avatar Aug 04 '17 20:08 davecheney

I think there would need to be a func SetStackBounds(start,end int) that found the first error that had a stack trace and adjusted it's stack trace accordingly. For withStack{}, this might look like....

type withStack struct {
    error
    *stack
    originalStack *stack
}

func (w *withStack) SetStackBounds(start,end int) {
    *w.stack=*w.originalStack[start,end]
}

decibel avatar Feb 06 '18 01:02 decibel

I am in a similar boat as @decibel and would love to see this addressed. Another implementation of this approach, https://github.com/go-errors/errors/blob/master/error.go#L93, has a parameter to accomplish saving only a subset of the stack. There are at least two other implementations of this need that I have seen and I am hoping that we can all standardize on a single one (ideally, this one) :)

thebitguru avatar May 22 '18 19:05 thebitguru

To be clear, the reason this is not done is I do not know an algorithm for gluing together two chains of program counters. This is the roadblock for this issue.

On 23 May 2018 at 05:12, Farhan Ahmad [email protected] wrote:

I am in a similar boat as @decibel https://github.com/decibel and would love to see this addressed. Another implementation of this approach, https://github.com/go-errors/errors/blob/master/error.go#L93, has a parameter to accomplish saving only a subset of the stack. There are at least two other implementations of this need that I have seen and I am hoping that we can all standardize on a single one (ideally, this one) :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pkg/errors/issues/129#issuecomment-391107834, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAcAxOOrjyb1EUq86_1BZIH8CJl0hn6ks5t1GMjgaJpZM4Ot9ev .

davecheney avatar May 23 '18 06:05 davecheney

That's helpful. Thanks for the response and more context, @davecheney.

thebitguru avatar May 23 '18 12:05 thebitguru

A simple option is to add functions like WrapSkip(err error, message string, skip int) error and pass the skip value down to callers() and add it to the hard-coded 3. It seems the Go 2 errors proposal was reduced to only contain the point the error was created rather than a stack trace, so this package probably still has a place after it, and might even want to support its interfaces to integrate with it.

segevfiner avatar Jun 26 '19 10:06 segevfiner

Here's an untested implementation of this: https://github.com/segevfiner/pkg-errors/tree/stack-skip

An alternative is to have stuff like WrapOptions(err error, message string, opts Options) error so we can add other stuff in the future.

Should this look acceptable I can send it in as a PR.

segevfiner avatar Jun 30 '19 07:06 segevfiner

Here's an untested implementation of this: https://github.com/segevfiner/pkg-errors/tree/stack-skip

An alternative is to have functions like WrapOptions(err error, message string, opts Options) error so we can add other stuff in the future.

Should this look acceptable I can send it in as a PR.

segevfiner avatar Jun 30 '19 11:06 segevfiner

Here's an untested implementation of this: https://github.com/segevfiner/pkg-errors/tree/stack-skip

An alternative is to have functions like WrapOptions(err error, message string, opts Options) error so we can add other stuff in the future.

Should this look acceptable I can send it in as a PR.

Really need this, It's easy to implement some common error-processing operations.

dingyaguang117 avatar Aug 23 '21 12:08 dingyaguang117