Our changes to the fork force printing of stack for all errors, can we undo this?

[coldfgirl]

The errors.fundamental type format function is:

func (f *fundamental) Format(s fmt.State, verb rune) {
	io.WriteString(s, f.msg)
	f.stack.Format(s, verb)
}

_{https://github.com/kohofinancial/errors/blob/master/errors.go#L147}

And pkg/errors has always (since before we forked) implemented this as:

func (f *fundamental) Format(s fmt.State, verb rune) {
	switch verb {
	case 'v':
		if s.Flag('+') {
			io.WriteString(s, f.msg)
			f.stack.Format(s, verb)
			return
		}
		fallthrough
	case 's':
		io.WriteString(s, f.msg)
	case 'q':
		fmt.Fprintf(s, "%q", f.msg)
	}
}

_{https://github.com/pkg/errors/blob/master/errors.go#L127}

The result of our version is that it is not possible to print the error message without the stack trace. I want to improve our DataDog error integration by providing error metadata but this requires being able to read the message separate from the stack trace.

The commit creating this difference is here indicating that this is intentional: https://github.com/kohofinancial/errors/commit/e2946ae3a7736210a5fea4f873cdba6be7d5ed46

What are @kohofinancial/nomads current thoughts on undoing our diversion?

[bewuethr]

I am in favour of this, and to go further, I think the stack trace is noise 95% of the time and I wish I had a carefully wrapped error instead. I'm really not interested in half a dozen middleware stack frames.

Since Go 1.13, the stdlib error package provides the %w formatting directive, and with Is, As, and Unwrap, we have all we need. This would require re-thinking (or starting to think) about things like when to wrap, be more careful about logging only after bubbling up etc., but I think it would potentially make errors much more useful.

If we switch to stlib errors, the migration is also quite easy: we can just start using it, and migrate existing services over slowly, as it's a different import path.

The Go blog article about error handling for Go 1.13+ is my go-to reference about when to wrap, and errors in general.

[coldfgirl]

I personally still find a lot of value in the stack trace pointing to specific files and lines that I can paste into my search bar to navigate to, but I also agree that using standard errors has value. The pkg/errors repository has itself moved to maintenance mode ahead of the Go 2 error handling proposal.

At least for now though do you think I'd be ok to introduce a PR to undo our changes (but keep the error types introduced)? I don't think there will be effects to our dependencies but do you think more work should be done to prove no impact?

[bewuethr]

It was, I think, briefly un-archived to add Is and As, but other than that it's not been getting updates.

If we can change it such that %+v is "with stack trace" and %v is "without stack trace" (as it always should have been), we get more control, and don't change behaviour, except where today we accidentally have %v instead of %+v, so I don't see a downside.

I've poked @kohofinancial/nomads to chime in, too, but I'm all for it.

[bewuethr]

I'm actually just now looking at our implementation (e2946ae). Bit of a blunt approach, to put it mildly...

[coldfgirl]

Yeah, I think it's just a flat reduction in functionality from what you're describing, with some work to limit stack traces to our own code which may be the only impact?

[coldfgirl]

Will open a PR to revert and I think the discussion or concerns can be raised there in the context of the revert.