6 ms·

I think this is the way to bubble up error messages that I like the most. Simple, not needing any additional tools, and very practical (sometimes even better th

by j1elo 1y ago

I think this is the way to bubble up error messages that I like the most. Simple, not needing any additional tools, and very practical (sometimes even better than a stack trace). The idea is to only add information that the caller isn't already aware of. Error messages shouldn't include the function name or any of its arguments, because the caller will include those in its own wrapping of that error. This is done with fmt.Errorf(): userId := "A0101" err := database.Store(userId); if err != nil { return fmt.Errorf("database.Store({userId: %q}): %w", userId, err) } If this is done consistently across all layers, and finally logged in the outermost layer, the end result will be nice error messages with all the context needed to understand the exact call chain that failed: fmt.Printf("ERROR %v\n", err) Output: ERROR app.run(): room.start({name: "participant5"}): UseStorage({type: "sqlite"}): Store({userId: "A0101"}): the transaction was interrupted This message shows at a quick glance which participant, which database selection, and which integer value where used when the call failed. Much more useful than Stack Traces, which don't show argument values. Of course, longer error messages could be written, but it seems optimal to just convey a minimal expression of what function call and argument was being called when the error happened. Adding to this, the Go code linter forbids writing error messages that start with Upper Case, precisely because it assumes that all this will be done and error messages are just parts of a longer sentence: https://staticcheck.dev/docs/checks/#ST1005 https://staticcheck.dev/docs/checks/#ST1005

masklinn 1y ago
> This message shows at a quick glance which participant, which database selection, and which integer value where used when the call failed. And it's completely useless for looking up the errors linked to a participant in an aggregator, which is pretty much the first issue the article talks about, unless you add an entire parsing and extraction layer overtop. > Much more useful than Stack Traces, which don't show argument values. No idea how universal it is, but there are at least some languages where you can get full stackframes out of the stacktrace. That's how pytest can show the locals of the leaf function out of the box in case of traceback: def test_a(): > a(0) test.py:11: test.py:2: in a b(f) test.py:5: in b c(f, 5/g) f = 0, x = 5.0 def c(f, x): > assert f E assert 0 test.py:8: AssertionError and can do so in every function of the trace if requested: def test_a(): > a(0) test.py:11: f = 0 def a(f): > b(f) test.py:2: f = 0, g = 1 def b(f, g=1): > c(f, 5/g) test.py:5: f = 0, x = 5.0 def c(f, x): > assert f E assert 0 test.py:8: AssertionError So this is just a matter of formatting.
- deleted 1y ago
  [deleted]
atombender 1y ago
I agree except for the part about using strings, as you lose structure they way. You should instead return structured errors: return CouldNotStoreUser{ UserID: userId, } and now this struct is available to anyone looking up the chain of wrapped errors.
- masklinn 1y ago
  This is covered by the “ergonomics” section of TFA: > With custom error structs however, it's a lot of writing to create your own error type and thus it becomes more of a burden to encourage your team members to do this. Because you need a type per layer, and that type needs to implement both error and unwrap.
  - atombender 1y ago
    In practice, I have found the benefit of explicit typing to outweigh the downsides of needing to declare the types. As a concrete example, it means you can target types with precision in the API layer: switch e := err.(type) { case UserNotFound: writeJSONResponse(w, 404, "User not found") case interface { Timeout() bool }: if e.Timeout() { writeJSONResponse(w, 503, "Timeout") } } I skimmed the article and didn't see the author proposing a way to do that with their arbitrary key/value map. Of course, you could use something else like error codes to translate groups of errors. But then why not just use types? But as I suggested in my other comment, you could also generalize it. For example: return meta.Wrap(err, "storing user", "userID", userID) Here, Wrap() is something like: func Wrap(err error, msg string, kvs ...any) { return &KV{ KV: kvs, cause: err, msg: msg, } } This is the inverse of the context solution. The point is to provide data at the point of error, not at every call site. You can always merge these later into a single map and pay the allocation cost there: var fields map[string]any for err != nil { if e, ok := err.(*KV); ok { for i := 0; i < len(KV.KV); i += 2 { fields[e.KV[i].(string)] = e.KV[i+1] } } err = errors.Unwrap(err) }
    - radlad 1y ago
candiddevmike 1y ago
You have to add all of that detail manually which sucks. You can get the function name from the runtime package and generate that metadata easily with a helper function. Otherwise when you rename the function, you have to rename all of the error messages.

by j1elo 1y ago

masklinn 1y ago

> This message shows at a quick glance which participant, which database selection, and which integer value where used when the call failed. And it's completely useless for looking up the errors linked to a participant in an aggregator, which is pretty much the first issue the article talks about, unless you add an entire parsing and extraction layer overtop. > Much more useful than Stack Traces, which don't show argument values. No idea how universal it is, but there are at least some languages where you can get full stackframes out of the stacktrace. That's how pytest can show the locals of the leaf function out of the box in case of traceback: def test_a(): > a(0) test.py:11: test.py:2: in a b(f) test.py:5: in b c(f, 5/g) f = 0, x = 5.0 def c(f, x): > assert f E assert 0 test.py:8: AssertionError and can do so in every function of the trace if requested: def test_a(): > a(0) test.py:11: f = 0 def a(f): > b(f) test.py:2: f = 0, g = 1 def b(f, g=1): > c(f, 5/g) test.py:5: f = 0, x = 5.0 def c(f, x): > assert f E assert 0 test.py:8: AssertionError So this is just a matter of formatting.

deleted 1y ago
[deleted]

atombender 1y ago

I agree except for the part about using strings, as you lose structure they way. You should instead return structured errors: return CouldNotStoreUser{ UserID: userId, } and now this struct is available to anyone looking up the chain of wrapped errors.

masklinn 1y ago
This is covered by the “ergonomics” section of TFA: > With custom error structs however, it's a lot of writing to create your own error type and thus it becomes more of a burden to encourage your team members to do this. Because you need a type per layer, and that type needs to implement both error and unwrap.
- atombender 1y ago
  In practice, I have found the benefit of explicit typing to outweigh the downsides of needing to declare the types. As a concrete example, it means you can target types with precision in the API layer: switch e := err.(type) { case UserNotFound: writeJSONResponse(w, 404, "User not found") case interface { Timeout() bool }: if e.Timeout() { writeJSONResponse(w, 503, "Timeout") } } I skimmed the article and didn't see the author proposing a way to do that with their arbitrary key/value map. Of course, you could use something else like error codes to translate groups of errors. But then why not just use types? But as I suggested in my other comment, you could also generalize it. For example: return meta.Wrap(err, "storing user", "userID", userID) Here, Wrap() is something like: func Wrap(err error, msg string, kvs ...any) { return &KV{ KV: kvs, cause: err, msg: msg, } } This is the inverse of the context solution. The point is to provide data at the point of error, not at every call site. You can always merge these later into a single map and pay the allocation cost there: var fields map[string]any for err != nil { if e, ok := err.(*KV); ok { for i := 0; i < len(KV.KV); i += 2 { fields[e.KV[i].(string)] = e.KV[i+1] } } err = errors.Unwrap(err) }
  - radlad 1y ago

candiddevmike 1y ago

You have to add all of that detail manually which sucks. You can get the function name from the runtime package and generate that metadata easily with a helper function. Otherwise when you rename the function, you have to rename all of the error messages.