errors icon indicating copy to clipboard operation
errors copied to clipboard

Consider StackTrace() []uintptr

Open cep21 opened this issue 9 years ago • 7 comments

Hi,

I am trying to write a library to interoperate with wrapped errors.

There are many error wrapping libraries, but I can only get the StackTrace() of this one by importing this one. This creates problems trying to write library agnostic code.

With Cause() error, I can simulate an error exposing this interface inside my library, meaning I can interoperate with pkg/errors without needing to import it, as well as define an API that other libraries can implement without forcing them into pkg/errors API.

With StackTrace() StackTrace I am forced to tell people wanting to work with my library to import errors, just for the StackTrace object, just to wrap their own stack trace with it.

This is a general problem with APIs that don't use Go's core types as parameters. Since a stack trace is just an array of pointers, if the implementation was StackTrace() []uintptr, then other error wrapping libraries could standardize to this and we could all interoperate.

I would also say that pretty printing a stack trace is behavior reasonably independent of wrapping errors itself. By returning the stack trace raw, you allow libraries to choose how it should be pretty printed: with maybe errors package providing a default implementation like it does now.

cep21 avatar Aug 04 '16 17:08 cep21

Id love to do that, that was how it worked previously, but if the type is unnamed then it cannot host a Formatter method.

On Fri, 5 Aug 2016, 03:44 Jack Lindamood [email protected] wrote:

Hi,

I am trying to write a library to interoperate with wrapped errors.

There are many error wrapping libraries, but I can only get the StackTrace() of this one by importing this one. This creates problems trying to write library agnostic code.

With Cause() error, I can simulate an error exposing this interface inside my library, meaning I can interoperate with pkg/errors without needing to import it, as well as define an API that other libraries can implement without forcing them into pkg/errors API.

With StackTrace() StackTrace I am forced to tell people wanting to work with my library to import errors, just for the StackTrace object, just to wrap their own stack trace with it.

This is a general problem with APIs that don't use Go's core types as parameters. Since a stack trace is just an array of pointers, if the implementation was StackTrace() []uintptr, then other error wrapping libraries could standardize to this and we could all interoperate.

I would also say that pretty printing a stack trace is behavior reasonably independent of wrapping errors itself. By returning the stack trace raw, you allow libraries to choose how it should be pretty printed: with maybe errors package providing a default implementation like it does now.

— 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/79, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAcA0RhUzhUTAoiIG08AMZ1FSjaHZL9ks5qciT_gaJpZM4Jc8o4 .

davecheney avatar Aug 04 '16 22:08 davecheney

Why can't the error type have a Stack() []uintptr function, error type have a Formatter method, and there be helpers to format []uintptr stacks that error uses in its Formatter method? There's even already precedent in the stdlib for Stack() []uintptr in the runtime package.

cep21 avatar Aug 05 '16 03:08 cep21

Maybe it can change, I haven't looked closely. It depends if people want to do st := err.Stack(); fmt.Printf("%+v", st); or if they want to fmt.Printf("%+v", err)

On Fri, Aug 5, 2016 at 1:37 PM, Jack Lindamood [email protected] wrote:

Why can't the error type have a Stack() []uintptr function, error type have a Formatter method, and there be helpers to format []uintptr stacks that error uses in its Formatter method? There's even already precedent in the stdlib for Stack() []uintptr in the runtime package.

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

davecheney avatar Aug 05 '16 03:08 davecheney

Can't you make fmt.Printf("%+v", err) work like it does today by having func (e _error) Format format the []uintptr returned by e.Stack() ? Maybe i'm missing something:

func (e _error) Format(s fmt.State, verb rune) {
    switch verb {
    case 'v':
        if s.Flag('+') {
            io.WriteString(s, e.msg) 
            fmt.Fprintf(s, builtInFormatting(e.Stack())
            return
        }
        fallthrough
    case 's':
        io.WriteString(s, e.msg)
    }
}

cep21 avatar Aug 05 '16 03:08 cep21

I think I see what you mean now. I don't think it's too much to, if someone wants to format the stack only, to get it out as an []uintptr and call some string formatter on that. There are probably many ways someone may want to format a []uintptr. Maybe errors.Stack() is one way.

cep21 avatar Aug 05 '16 03:08 cep21

Maybe I'm missing something but... why not both? Is there reason why the package couldn't support both StackTrace() StackTrace and Stack() []uintptr?

I'd like to add support to raven-go, Sentry's library for capturing and reporting errors in Go programs, but accessing the stack trace from pkg/errors is also a hang-up for me.

@davecheney If you're open to this, I can easily submit a PR.

mark-adams avatar Jun 29 '17 01:06 mark-adams

Was there ever a decision made about this? I'm in the same camp as cep21 and mark-adams, I think it would be valuable to have a universal API, and if you wanna keep the formatting methods, keep the current API as well (what mark-adams suggested).

fgblomqvist avatar Jan 10 '19 17:01 fgblomqvist