palantir / stacktrace Goto Github PK

There is a common pattern in domain logic, where the creation of errors has been standardised with helper functions.
For example in a http handler one might want to easily create a validation error, with a custom errorCode, without having to type the complete stacktrace boilerplate.

if err := domain.DoTheThing(ctx, args); err != nil {
   // call to stacktrace.Propagate has been deferred to an opaque package
   // containing all of the domain semantic conventions
   return nil, errors.InvalidThingDone(err, args)
}
// stacktrace will always point to its caller, which is in this case the helper function

The problem is that stacktrace will use the helper function callsite instead of the line of invocation of said helper function.
It would not be a problem if go allowed explicit inlining, but that does not seem possible currently.

The idea would be to add an optional parameter in order to skip n runtime call frames, allowing forward declaration of error helpers, without impacting the stacktrace.
Is it something that could benefit this library (as a new public function), or is it something totally out of scope for the community ?

// example implementation of the `errors` package with proposed functionnalities
// API is given as an example

// builder created at runtime
var invalidThingDoneErr = stacktrace.NewErrorBuilder()

// invokes the error builder which skips this frame, being the actual helper call site
func InvalidThingDone(err error, args any) error {
   return invalidThingDoneErr.
      WithCode(0xdeadbeef).
      Propagate(err)
} 

Currently it trims everything before and including "/src/". Nothing prevents a project from having a package called "src" so this heuristic can be too aggressive. Instead, it should loop through directories in the $GOPATH environment variable and remove the longest that matches.

I really love the concept of stacktrace and I think it is an amazing idea.
The only thing is that I don't understand why it is designed to return the "stacktraced" error when the Error() method is called.
As you write it yourself, Note that stack traces are not designed to be user-visible. So why are the decorations shown in the error ?
The logical thing should not be to never touch the original error message and send the decorations of the stacktrace to a logger ?
That way, one's app way of working would not change at all, but stacktraces would be shown to admins in the logs.

The new slog library provides a slog.LogValuer interface for formatting attributes.

Documentation here: https://pkg.go.dev/golang.org/x/exp/slog#hdr-Customizing_a_type_s_logging_behavior

I think palantir/stacktrace should implement it.

type customError struct {
    ...
}

func (e customError) Error() string {
    ...
}

// implements slog.LogValuer
func (e customError) LogValue() slog.Value {
	return slog.GroupValue(
	        slog.Int("code", e.code),
	        slog.String("message", e.msg),
	        slog.String("stacktrace", e.stacktrace),
        )
}

slog will be released in go 1.21. But is already available and golang.org/x/exp/slog

I realize it is probably too late to change this behavior, but I still think it is worthwhile flagging a bug I found recently. if you have already declared err in your scope (happens frequently), the following code can return nil on an error condition

err := doSomething()
if err != nil {
    return stacktrace.Propagate(err, "")
}
...
for _, whatever := range things {
    if whatever {
        return nil, stacktrace.Propagate(err,"this is supposed to be an error condition")
    }
}
...

the loss is not being able to do compact single-line returns, which is arguably poor go style anyways

Go 1.13 introduced error wrapping. It would be beneficial if palantir/stacktrace supported it so that errors could still be compared after propagation.