Repository navigation
proposal: errors: add (stack)trace at error annotation #60873
Description
Activity
Note that you can already call a function that records a stack trace for an error, such as https://pkg.go.dev/github.com/go-errors/errors. Personally I think that if we want to add this kind of functionality into the standard library, we should do it using a different function, not using an annotation in the string passed to
fmt.Errorf.Reacted by Carlana, bokwoon95, Karol Marcjan, Jan, kolobok kelbek, Emil Valeev, Roberto Montagna, Codey Oxley, Denis Timofeev, Trevor N. Suarez and 6 more- changed the title
[-]proposal: Add (stack)trace at error annotation [/-][+]proposal: errors: add (stack)trace at error annotation [/+]on Jun 19, 2023 @ianlancetaylor I'm a aware of ways to add a full traceback and I writted about this, my english is not good.
This as few caveats:
The traceback is full even if you don't need all the stack.
The annotations are all at the end and not near the line.
Returning a new error in the flow loose the trace.
It encourage to just return err every where else.
It mimick a panic. and confuse beginners.There are a number of third party packages with traces. The unique thing the standard library can do is to standardize how the traces are exposed. Maybe an interface in runtime like
type ErrorTrace interface{ error Trace() []Frames }
Reacted by Jan, Roberto Montagna, Mike Schinkel, Trevor N. Suarez, Roman Perekhod, Yannick, Chris Marshall, Sam Wilson and Will Faught@flibustenet Thanks, I think I see what you are saying now. But I don't see why this has to be in the standard library. It can be a function defined in a separate package.
@carlmjohnson my point was to embed only one Frame and not all the stack.
@ianlancetaylor thanks to try to understand my english ! I thought about this because of the way wrapping was resolved with a simple addon to fmt.Errorf with%w, i suggest the same kind of things for trace.
I don't make this proposal because I need it, of course it's nothing more easy to do it alone (and it works !) but I like to imagine for Go something more original than legacy traceback.Reacted by CarlanaI have errorx.Trace which adds a single trace line like this:
func traceExample(ok bool) (err error) { defer errorx.Trace(&err) if !ok { // returns "problem in github.com/carlmjohnson/errorx_test.traceExample (trace_example_test.go:13): oh no!" return errors.New("oh no!") } return nil }
I think it's more interesting to think about what can the standard library do that user code cannot do. Otherwise, I can just use my own code. :-)
@carlmjohnson fine to see similar ideas (I didn't search much).
Reacted by CarlanaThe original set of error proposals for 1.13 included adding stack frames to errors created with
errors.Newandfmt.Errorf. That change was implemented, but ultimately rolled back. As I recall, the concerns were startup overhead (var ErrFoo = errors.New("...")calls at init time got a lot more expensive), andreflect.DeepEqualno longer reporting errors with the same text as identical.This proposal avoids these issues by making stack frames opt-in; you need to ask for one. The downside is that you don't get stack frames for errors unless the creator asks for them. Either this means most errors don't have stack frames, or we need to update the entire Go ecosystem to start asking for stack frames in errors.
Updating the entire Go ecosystem seems infeasible.
I don't know what the right approach to getting stacks into errors is. Clearly this would be useful; we dropped it from 1.13 because we couldn't make it work right, not because the goal was a bad idea. I think any successful proposal here will need to figure out how to get stacks into existing errors without rewriting the world, and without running into the issues the 1.13 implementation did.
Reacted by Carlana, jaloren, Komu Wairagu, Karol Marcjan, Jan, Vojtech Vitek (golang.cz), Jonathan Hall, Max Chechel, chenwei67, Timur Guseynov and 3 more@ianlancetaylor you raise an important point. Why put this in the standard library when this can be written in third party code, especially when there are so many conflicting opinions on how error annotations should be handled?
IMHO, the answer is that in some cases, having a single way to approach a problem in the standard library with a canonical interface is a great help to the go community. The go community has high trust in the standard library in terms of quality and the backwards compatibility guarantee. And for common needs it also removes one less third party dependency i need to pull in or have a discussion about with my team. Finally, it provides a contract that allows third party libraries to opt into so an application can handle cross cutting concerns in a consistent way.
Consider the following examples:
- we are adding structured logging to the std library soon. The proposal process was grueling. I think all told there was well over 600 comments. Structured logging with log levels is something that anyone could write and.....we did. I think at one point there were over 10 popular logging libraries out there.
- the context library
- errors.Join
I've been writing go since 2016. Error annotation comes up constantly on my team both writing go code, reading it, and most importantly debugging it. I am sure everyone has had a story of getting an error printed to standard out that repeats the same error message multiple times (hello improper wrapping), being forced to grep through the go source files, and them making an educated guess which error was the source of the problem.
Reacted by Carlana, Jan, Marko Ćurlin, Trevor N. Suarez and Will FaughtReacted by Marko Ćurlin15 remaining items
@fsaintjacques Thanks, based on that idea I've opened https://go.dev/issue/63358.
Reacted by Simon LehnReacted by Mike Schinkelall usage of fmt.Errorf and errors.New (ignoring statically defined exportable variables) auto-magically attaches a stack trace if none were provided.
Yea, but what about other libraries which are not using standard errors package anymore, because they already want to attach stack traces? It will not enable it for them.
I think the critical piece is that we should standardize what the interface of errors which have recorded a stack trace looks like. I suggest:
type stackTracer interface { StackTrace() []uintptr }Then, we can discuss it adding it to standard errors with a flag or not. But without a standard way to obtain a stack trace, interoperability will suffer.
Reacted by Anton Kaymakchi, Mike Schinkel, Roman Odermatt, Ev Dolzhenko, Trevor N. Suarez and Nikita KoptelovI just wanted to put in a plug for doing nothing on this. Most in this conversation are of the opinion that stacktraces are useful in logging. That could be true for existing codebases that struggle with package organization, but for new code/well organized projects (well organized in my opinion haha, which could be wrong) as simple message is more readable and useful
func AddUser(ctx context.Context, newUser NewUser) (User, error) { // do things user, err := insertUser(ctx, newUser) if err != nil { return User{}, fmt.Errorf("adding user: %w", err) } // do more things }With this style you just search for "adding user" in your editor to find the line. If every layer does this, it's a human readable stacktrace in far fewer characters. It also works great with imported libraries, and there's never any confusion over wether the stacktrace has been added yet, or multiple stacktraces.
If a function is added to the stdlib for this, I think it would be essential that it deals with multiple stack traces. I'd also even prefer a warning in the comments that I can point new engineers to so I don't get railroaded into stack traces in logs because "it's in the standard library" Something I wish the go team should do much more haha
Reacted by Wembley LeachReacted by jaloren, Steve Coffman, cxlblm, Gallyam Biktashev, Takahiro, NAKAMURA (na9amura), Kirill Agalakov, Nikita Koptelov, Sebastian Rühl, Jeff Swenson and Parker Roan@neild What makes a stack trace so expensive/slow? It seems the string data are already in the binary, because they're printed for panic stack traces. Is it copying those string data from the binary into Go string values, and allocating *runtime.Func's? If so, what about deferring all that work behind getter methods instead of precomputing it for struct fields like in runtime.Func?
@willfaught Stack trace record does not need any strings. Stings are obtained only when printing. Stack trace is fully described with
[]uintptr. Obtaining that has to walk the stack frames. Not too expensive, but also not free.I think the main reason why stack traces are not included is that idiomatic Go error handling generally prepends some string to error message at every point human decided it is an important point to annotate error further. So generally you can reconstruct the error path by tracing those prepends.
My issue with that is that sometimes developer does not do that because they think the error cannot happen really. And having automatic stack trace helps in that case.
I think the main reason why stack traces are not included is that idiomatic Go error handling generally prepends some string to error message at every point human decided it is an important point to annotate error further. So generally you can reconstruct the error path by tracing those prepends.
My issue with that is that sometimes developer does not do that because they think the error cannot happen really. And having automatic stack trace helps in that case.
I do not think it is an ideal practice to write code that wraps all the places where an error might occur. It just increases the amount of manual, error-prone, and hard-to-test code. Wrapping errors should only be done when context information is really needed, and manually wrapping errors just to add the caller function name is a bad habit.
To avoid that bad practice, I think we need two things: a mechanism to create a new error with a stack trace as easily as fmt.Errorf(), and a mechanism to attach the current stack trace when wrapping an error if that error does not already have a stack trace.
Stack traces are slow because 1) the code has to literally walk the stack; 2) the stack trace has to be stored somewhere which typically means a memory allocation. It's not obviously a good choice to pay that cost every time an error value is created. Of course it makes sense to have a way to do it; there are various packages mentioned on this issue that do it.
See also #63358.
Reacted by Pierre DurandThere are two issues with wrapping the stack on the error.
- It add memory allocation like you said.
- The other one is that we loose everything if we return the error with
%von the road. If not it means that we need to embed the stack even with '%v' which is overkill most of the time and we come back to 1.
3 years after using my simple proposal I can confirm that it's enough to return the small stack in the string itself explicitly when we are sure that it's an error and that the trace will be useful. It's fully compatible with Go error handling.
I just wanted to put in a plug for doing nothing on this. Most in this conversation are of the opinion that stacktraces are useful in logging. That could be true for existing codebases that struggle with package organization, but for new code/well organized projects (well organized in my opinion haha, which could be wrong) as simple message is more readable and useful
func AddUser(ctx context.Context, newUser NewUser) (User, error) { // do things user, err := insertUser(ctx, newUser) if err != nil { return User{}, fmt.Errorf("adding user: %w", err) } // do more things }With this style you just search for "adding user" in your editor to find the line. If every layer does this, it's a human readable stacktrace in far fewer characters. It also works great with imported libraries, and there's never any confusion over wether the stacktrace has been added yet, or multiple stacktraces.
If a function is added to the stdlib for this, I think it would be essential that it deals with multiple stack traces. I'd also even prefer a warning in the comments that I can point new engineers to so I don't get railroaded into stack traces in logs because "it's in the standard library" Something I wish the go team should do much more haha
😟 When your function is called in more than >=5 places, it becomes difficult to pinpoint the exact cause of the error message, which diminishes the value of the error string. For example, ReadXXX() returns an error.
When your function is called in more than >=5 places, it becomes difficult to pinpoint the exact cause of the error message
I think the general Go approach is that you prefix the message with some unique caller-based prefix for each of those places, which help you then identify which of the callers was, e.g., if error is "read error" then prefix could be so that final error message is "user: read error", "comment: read error" and so on.
Reacted by CarlanaWhen your function is called in more than >=5 places, it becomes difficult to pinpoint the exact cause of the error message
I think the general Go approach is that you prefix the message with some unique caller-based prefix for each of those places, which help you then identify which of the callers was, e.g., if error is "read error" then prefix could be so that final error message is "user: read error", "comment: read error" and so on.
Aren't you worried that your caller has multiple callers themselves? It is evident that it will exponentially amplify, causing prefix wraps to be written everywhere... For example, if I need to write a prefix or more at 5x5x5=125, I will go crazy
For example, if I need to write a prefix or more at 5x5x5=125, I will go crazy
You write prefix on 5 + 5 + 5 = 15 places. Not 125. But yea, it is a lot of work you have to do and anticipate in which places such prefixes are beneficial. Also, approach is tricky with recursion.
So this is why I personally do think stack traces are useful and made a package for it. Then you get both: stack traces for places developer didn't think/bother to write a prefix messages, and a prefix message where developer decided to add one.
On the stdlib the convention is to prefix the string with the name of the operation, often it's the name of the function. For example in os package the function Rename return "rename: ...".
It's why my proposal is to make this easier to include with a format specifier of Errorf the name of the package/function in the string. I believe I'm not able to explain it !The guideline I like is the prefix should include information that the caller has which the callee does not. So in that case, it would be "adding user John Smith: database locked." AddUser knows it was John Smith that you're trying to add and insertUser knows that the database is locked or unreachable or whatever.
But this is just a style guide. It's nice to have stack traces as an option as well for teams that want to do it differently.
The guideline I like is the prefix should include information that the caller has which the callee does not. So in that case, it would be "adding user John Smith: database locked." AddUser knows it was John Smith that you're trying to add and insertUser knows that the database is locked or unreachable or whatever.
Yes of course. But about stack trace what we need is not only what's append but also where.
This proposal try to address the lack of traceback in errors but keep the way we handle errors as value annotated where the error occur.
Errors are values, a string or a custom error.
To retrieve the stack of errors we need to follow the chain of strings, each annotation should be written carefully to retrieve easily from where the error comes. If we just
return errone time we loose the path.Because of this we can embed all the stack in a custom error at the higher lever and print it at the end with the hope that we didn't loose the path with a
return fmt.Errorf("... %v"). Like that we can justreturn erreverywhere, we'll just use the full traceback to see what's happen like in most of the others languages.Often we'll have both, a huge traceback and a chain of annotations in the end and full of
return erreverywhere. This is not fun and not idiomatic in Go.Sometimes we just need an annotation, in a lib for example, and sometimes it's interesting to have the method and the exact line in an application. In a web server we generaly need only the stack after the handler and before the lib but not in the server and middlewares.
My proposal is to can add easily the trace and just this trace when we annotate the error. Something like
fmt.Errorf("@trace I call foo: %v", err)The result is a minimal traceback like that:
https://go.dev/play/p/VJSXkbOX7J_c
I believe it follow the Go way of handling errors.
The error is still a value, a value with a trace as annotation (not to do for an API).
We decide at each error if we need the trace or not and not for the whole chain.
We encourage to annotate.
Explicit.
Easy to implement.
It's easy to experiment, just replace fmt.Errorf with a function like this:
Hope it give at least inspiration.