Skip to content

Commit af098e8

Browse files
committed
add formatting for multi-cause errors
Previously, error formatting logic was based on a single linear chain of causality. Error causes would be printed vertically down the page, and their interpretation was natural. This commit adds multi-cause formatting support with two goals in mind: 1. Preserve output exactly as before for single-cause error chains 2. Format multi-errors in a way that preserves the existing vertical layout style. For non-verbose error display (`.Error()`, `%s`, `%v`) there are no changes implemented here. We rely on the error's own display logic and typically a multi-cause error will have a message within that has been constructed from all of its causes during instantiation. For verbose error display (`%+v`) which relies on object introspection, whenever we encounter a multi-cause error in the chain, we mark that subtree as being displayed with markers for the "depth" of various causes. All child errors of the parent, will be at depth "1", their child errors will be at depth "2", etc. During display, we indent the error by its "depth" and add a `└─` symbol to illustrate the parent/child relationship. Example: Printing the error produced by this construction using the format directive `%+v` ``` fmt.Errorf( "prefix1: %w", fmt.Errorf( "prefix2 %w", goErr.Join( fmt.Errorf("a%w", fmt.Errorf("b%w", fmt.Errorf("c%w", goErr.New("d")))), fmt.Errorf("e%w", fmt.Errorf("f%w", fmt.Errorf("g%w", goErr.New("h")))), ))) ``` Produces the following output: ``` prefix1: prefix2: abcd (1) prefix1 Wraps: (2) prefix2 Wraps: (3) abcd | efgh └─ Wraps: (4) efgh └─ Wraps: (5) fgh └─ Wraps: (6) gh └─ Wraps: (7) h └─ Wraps: (8) abcd └─ Wraps: (9) bcd └─ Wraps: (10) cd └─ Wraps: (11) d Error types: (1) *fmt.wrapError (2) *fmt.wrapError (3) *errors.joinError (4) *fmt.wrapError (5) *fmt.wrapError (6) *fmt.wrapError (7) *errors.errorString (8) *fmt.wrapError (9) *fmt.wrapError (10) *fmt.wrapError (11) *errors.errorString`, ``` Note the following properties of the output: * The top-level single cause chain maintains the left-aligned `Wrap` lines which contain each cause one at a time. * As soon as we hit the multi-cause errors within the `joinError` struct (`(3)`), we add new indentation to show that The child errors of `(3)` are `(4)` and `(8)` * Subsequent causes of errors after `joinError`, are also indented to disambiguate the causal chain * The `Error types` section at the bottom remains the same and the numbering of the types can be matched to the errors above using the integers next to the `Wrap` lines. * No special effort has been made to number the errors in a way that describes the causal chain or tree. This keeps it easy to match up the error to its type descriptor.
1 parent 30a4e82 commit af098e8

13 files changed

+1555
-63
lines changed

errbase/format_error.go

Lines changed: 106 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,13 @@ func formatErrorInternal(err error, s fmt.State, verb rune, redactableOutput boo
102102
// to enable stack trace de-duplication. This requires a
103103
// post-order traversal. Since we have a linked list, the best we
104104
// can do is a recursion.
105-
p.formatRecursive(err, true /* isOutermost */, true /* withDetail */)
105+
p.formatRecursive(
106+
err,
107+
true, /* isOutermost */
108+
true, /* withDetail */
109+
false, /* withDepth */
110+
0, /* depth */
111+
)
106112

107113
// We now have all the data, we can render the result.
108114
p.formatEntries(err)
@@ -146,7 +152,13 @@ func formatErrorInternal(err error, s fmt.State, verb rune, redactableOutput boo
146152
// by calling FormatError(), in which case we'd get an infinite
147153
// recursion. So we have no choice but to peel the data
148154
// and then assemble the pieces ourselves.
149-
p.formatRecursive(err, true /* isOutermost */, false /* withDetail */)
155+
p.formatRecursive(
156+
err,
157+
true, /* isOutermost */
158+
false, /* withDetail */
159+
false, /* withDepth */
160+
0, /* depth */
161+
)
150162
p.formatSingleLineOutput()
151163
p.finishDisplay(verb)
152164

@@ -195,7 +207,19 @@ func (s *state) formatEntries(err error) {
195207
// Wraps: (N) <details>
196208
//
197209
for i, j := len(s.entries)-2, 2; i >= 0; i, j = i-1, j+1 {
198-
fmt.Fprintf(&s.finalBuf, "\nWraps: (%d)", j)
210+
s.finalBuf.WriteByte('\n')
211+
// Extra indentation starts at depth==2 because the direct
212+
// children of the root error area already printed on separate
213+
// newlines.
214+
for m := 0; m < s.entries[i].depth-1; m += 1 {
215+
if m == s.entries[i].depth-2 {
216+
s.finalBuf.WriteString("└─ ")
217+
} else {
218+
s.finalBuf.WriteByte(' ')
219+
s.finalBuf.WriteByte(' ')
220+
}
221+
}
222+
fmt.Fprintf(&s.finalBuf, "Wraps: (%d)", j)
199223
entry := s.entries[i]
200224
s.printEntry(entry)
201225
}
@@ -330,12 +354,34 @@ func (s *state) formatSingleLineOutput() {
330354
// s.finalBuf is untouched. The conversion of s.entries
331355
// to s.finalBuf is done by formatSingleLineOutput() and/or
332356
// formatEntries().
333-
func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
357+
//
358+
// `withDepth` and `depth` are used to tag subtrees of multi-cause
359+
// errors for added indentation during printing. Once a multi-cause
360+
// error is encountered, all subsequent calls with set `withDepth` to
361+
// true, and increment `depth` during recursion. This information is
362+
// persisted into the generated entries and used later to display the
363+
// error with increased indentation based in the depth.
364+
func (s *state) formatRecursive(err error, isOutermost, withDetail, withDepth bool, depth int) int {
334365
cause := UnwrapOnce(err)
366+
numChildren := 0
335367
if cause != nil {
336-
// Recurse first.
337-
s.formatRecursive(cause, false /*isOutermost*/, withDetail)
368+
// Recurse first, which populates entries list starting from innermost
369+
// entry. If we've previously seen a multi-cause wrapper, `withDepth`
370+
// will be true, and we'll record the depth below ensuring that extra
371+
// indentation is applied to this inner cause during printing.
372+
// Otherwise, we maintain "straight" vertical formatting by keeping the
373+
// parent callers `withDepth` value of `false` by default.
374+
numChildren += s.formatRecursive(cause, false, withDetail, withDepth, depth+1)
375+
}
376+
377+
causes := UnwrapMulti(err)
378+
for _, c := range causes {
379+
// Override `withDepth` to true for all child entries ensuring they have
380+
// indentation applied during formatting to distinguish them from
381+
// parents.
382+
numChildren += s.formatRecursive(c, false, withDetail, true, depth+1)
338383
}
384+
// inserted := len(s.entries) - 1 - startChildren
339385

340386
// Reinitialize the state for this stage of wrapping.
341387
s.wantDetail = withDetail
@@ -355,17 +401,19 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
355401
bufIsRedactable = true
356402
desiredShortening := v.SafeFormatError((*safePrinter)(s))
357403
if desiredShortening == nil {
358-
// The error wants to elide the short messages from inner
359-
// causes. Do it.
360-
s.elideFurtherCauseMsgs()
404+
// The error wants to elide the short messages from inner causes.
405+
// Read backwards through list of entries up to the number of new
406+
// entries created "under" this one amount and mark `elideShort`
407+
// true.
408+
s.elideShortChildren(numChildren)
361409
}
362410

363411
case Formatter:
364412
desiredShortening := v.FormatError((*printer)(s))
365413
if desiredShortening == nil {
366414
// The error wants to elide the short messages from inner
367415
// causes. Do it.
368-
s.elideFurtherCauseMsgs()
416+
s.elideShortChildren(numChildren)
369417
}
370418

371419
case fmt.Formatter:
@@ -389,7 +437,7 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
389437
if elideCauseMsg := s.formatSimple(err, cause); elideCauseMsg {
390438
// The error wants to elide the short messages from inner
391439
// causes. Do it.
392-
s.elideFurtherCauseMsgs()
440+
s.elideShortChildren(numChildren)
393441
}
394442
}
395443

@@ -412,7 +460,7 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
412460
if desiredShortening == nil {
413461
// The error wants to elide the short messages from inner
414462
// causes. Do it.
415-
s.elideFurtherCauseMsgs()
463+
s.elideShortChildren(numChildren)
416464
}
417465
break
418466
}
@@ -421,16 +469,21 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
421469
// If the error did not implement errors.Formatter nor
422470
// fmt.Formatter, but it is a wrapper, still attempt best effort:
423471
// print what we can at this level.
424-
if elideCauseMsg := s.formatSimple(err, cause); elideCauseMsg {
472+
elideChildren := s.formatSimple(err, cause)
473+
// always elideChildren when dealing with multi-cause errors.
474+
if len(causes) > 0 {
475+
elideChildren = true
476+
}
477+
if elideChildren {
425478
// The error wants to elide the short messages from inner
426479
// causes. Do it.
427-
s.elideFurtherCauseMsgs()
480+
s.elideShortChildren(numChildren)
428481
}
429482
}
430483
}
431484

432485
// Collect the result.
433-
entry := s.collectEntry(err, bufIsRedactable)
486+
entry := s.collectEntry(err, bufIsRedactable, withDepth, depth)
434487

435488
// If there's an embedded stack trace, also collect it.
436489
// This will get either a stack from pkg/errors, or ours.
@@ -444,21 +497,26 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
444497
// Remember the entry for later rendering.
445498
s.entries = append(s.entries, entry)
446499
s.buf = bytes.Buffer{}
500+
501+
return numChildren + 1
447502
}
448503

449-
// elideFurtherCauseMsgs sets the `elideShort` field
450-
// on all entries added so far to `true`. Because these
451-
// entries are added recursively from the innermost
452-
// cause outward, we can iterate through all entries
453-
// without bound because the caller is guaranteed not
454-
// to see entries that it is the causer of.
455-
func (s *state) elideFurtherCauseMsgs() {
456-
for i := range s.entries {
457-
s.entries[i].elideShort = true
504+
// elideShortChildren takes a number of entries to set `elideShort` to
505+
// false. The reason a number of entries is needed is because
506+
//
507+
// TODO(davidh): I have a hypothesis that the `newEntries` arg is
508+
// unnecessary and it's correct to elide all children whenever this is
509+
// called, even with multi-cause scenarios. This is because it's
510+
// impossible for a multi-cause tree to later "switch" to a
511+
// single-cause chain, so any time a multi-cause error is recursing
512+
// you're guaranteed to only have its children within.
513+
func (s *state) elideShortChildren(newEntries int) {
514+
for i := 0; i < newEntries; i++ {
515+
s.entries[len(s.entries)-1-i].elideShort = true
458516
}
459517
}
460518

461-
func (s *state) collectEntry(err error, bufIsRedactable bool) formatEntry {
519+
func (s *state) collectEntry(err error, bufIsRedactable bool, withDepth bool, depth int) formatEntry {
462520
entry := formatEntry{err: err}
463521
if s.wantDetail {
464522
// The buffer has been populated as a result of formatting with
@@ -495,6 +553,10 @@ func (s *state) collectEntry(err error, bufIsRedactable bool) formatEntry {
495553
}
496554
}
497555

556+
if withDepth {
557+
entry.depth = depth
558+
}
559+
498560
return entry
499561
}
500562

@@ -712,6 +774,11 @@ type formatEntry struct {
712774
// truncated to avoid duplication of entries. This is used to
713775
// display a truncation indicator during verbose rendering.
714776
elidedStackTrace bool
777+
778+
// depth, if positive, represents a nesting depth of this error as
779+
// a causer of others. This is used with verbose printing to
780+
// illustrate the nesting depth for multi-cause error wrappers.
781+
depth int
715782
}
716783

717784
// String is used for debugging only.
@@ -733,6 +800,12 @@ func (s *state) Write(b []byte) (n int, err error) {
733800

734801
for i, c := range b {
735802
if c == '\n' {
803+
//if s.needNewline > 0 {
804+
// for i := 0; i < s.needNewline-1; i++ {
805+
// s.buf.Write(detailSep[:len(sep)-1])
806+
// }
807+
// s.needNewline = 0
808+
//}
736809
// Flush all the bytes seen so far.
737810
s.buf.Write(b[k:i])
738811
// Don't print the newline itself; instead, prepare the state so
@@ -762,6 +835,11 @@ func (s *state) Write(b []byte) (n int, err error) {
762835
s.notEmpty = true
763836
}
764837
}
838+
//if s.needNewline > 0 {
839+
// for i := 0; i < s.needNewline-1; i++ {
840+
// s.buf.Write(detailSep[:len(sep)-1])
841+
// }
842+
//}
765843
s.buf.Write(b[k:])
766844
return len(b), nil
767845
}
@@ -788,6 +866,9 @@ func (p *state) switchOver() {
788866
p.buf = bytes.Buffer{}
789867
p.notEmpty = false
790868
p.hasDetail = true
869+
870+
// One of the newlines is accounted for in the switch over.
871+
// p.needNewline -= 1
791872
}
792873

793874
func (s *printer) Detail() bool {

0 commit comments

Comments
 (0)