Skip to content

Commit c03a0c7

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 fe9cc0f commit c03a0c7

13 files changed

+1549
-63
lines changed

errbase/format_error.go

Lines changed: 100 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,7 @@ 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(err, true, false, false, 0)
150156
p.formatSingleLineOutput()
151157
p.finishDisplay(verb)
152158

@@ -195,7 +201,19 @@ func (s *state) formatEntries(err error) {
195201
// Wraps: (N) <details>
196202
//
197203
for i, j := len(s.entries)-2, 2; i >= 0; i, j = i-1, j+1 {
198-
fmt.Fprintf(&s.finalBuf, "\nWraps: (%d)", j)
204+
s.finalBuf.WriteByte('\n')
205+
// Extra indentation starts at depth==2 because the direct
206+
// children of the root error area already printed on separate
207+
// newlines.
208+
for m := 0; m < s.entries[i].depth-1; m += 1 {
209+
if m == s.entries[i].depth-2 {
210+
s.finalBuf.WriteString("└─ ")
211+
} else {
212+
s.finalBuf.WriteByte(' ')
213+
s.finalBuf.WriteByte(' ')
214+
}
215+
}
216+
fmt.Fprintf(&s.finalBuf, "Wraps: (%d)", j)
199217
entry := s.entries[i]
200218
s.printEntry(entry)
201219
}
@@ -330,12 +348,34 @@ func (s *state) formatSingleLineOutput() {
330348
// s.finalBuf is untouched. The conversion of s.entries
331349
// to s.finalBuf is done by formatSingleLineOutput() and/or
332350
// formatEntries().
333-
func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
351+
//
352+
// `withDepth` and `depth` are used to tag subtrees of multi-cause
353+
// errors for added indentation during printing. Once a multi-cause
354+
// error is encountered, all subsequent calls with set `withDepth` to
355+
// true, and increment `depth` during recursion. This information is
356+
// persisted into the generated entries and used later to display the
357+
// error with increased indentation based in the depth.
358+
func (s *state) formatRecursive(err error, isOutermost, withDetail, withDepth bool, depth int) int {
334359
cause := UnwrapOnce(err)
360+
numChildren := 0
335361
if cause != nil {
336-
// Recurse first.
337-
s.formatRecursive(cause, false /*isOutermost*/, withDetail)
362+
// Recurse first, which populates entries list starting from innermost
363+
// entry. If we've previously seen a multi-cause wrapper, `withDepth`
364+
// will be true, and we'll record the depth below ensuring that extra
365+
// indentation is applied to this inner cause during printing.
366+
// Otherwise, we maintain "straight" vertical formatting by keeping the
367+
// parent callers `withDepth` value of `false` by default.
368+
numChildren += s.formatRecursive(cause, false, withDetail, withDepth, depth+1)
369+
}
370+
371+
causes := UnwrapMulti(err)
372+
for _, c := range causes {
373+
// Override `withDepth` to true for all child entries ensuring they have
374+
// indentation applied during formatting to distinguish them from
375+
// parents.
376+
numChildren += s.formatRecursive(c, false, withDetail, true, depth+1)
338377
}
378+
// inserted := len(s.entries) - 1 - startChildren
339379

340380
// Reinitialize the state for this stage of wrapping.
341381
s.wantDetail = withDetail
@@ -355,17 +395,19 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
355395
bufIsRedactable = true
356396
desiredShortening := v.SafeFormatError((*safePrinter)(s))
357397
if desiredShortening == nil {
358-
// The error wants to elide the short messages from inner
359-
// causes. Do it.
360-
s.elideFurtherCauseMsgs()
398+
// The error wants to elide the short messages from inner causes.
399+
// Read backwards through list of entries up to the number of new
400+
// entries created "under" this one amount and mark `elideShort`
401+
// true.
402+
s.elideShortChildren(numChildren)
361403
}
362404

363405
case Formatter:
364406
desiredShortening := v.FormatError((*printer)(s))
365407
if desiredShortening == nil {
366408
// The error wants to elide the short messages from inner
367409
// causes. Do it.
368-
s.elideFurtherCauseMsgs()
410+
s.elideShortChildren(numChildren)
369411
}
370412

371413
case fmt.Formatter:
@@ -389,7 +431,7 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
389431
if elideCauseMsg := s.formatSimple(err, cause); elideCauseMsg {
390432
// The error wants to elide the short messages from inner
391433
// causes. Do it.
392-
s.elideFurtherCauseMsgs()
434+
s.elideShortChildren(numChildren)
393435
}
394436
}
395437

@@ -412,7 +454,7 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
412454
if desiredShortening == nil {
413455
// The error wants to elide the short messages from inner
414456
// causes. Do it.
415-
s.elideFurtherCauseMsgs()
457+
s.elideShortChildren(numChildren)
416458
}
417459
break
418460
}
@@ -421,16 +463,21 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
421463
// If the error did not implement errors.Formatter nor
422464
// fmt.Formatter, but it is a wrapper, still attempt best effort:
423465
// print what we can at this level.
424-
if elideCauseMsg := s.formatSimple(err, cause); elideCauseMsg {
466+
elideChildren := s.formatSimple(err, cause)
467+
// always elideChildren when dealing with multi-cause errors.
468+
if len(causes) > 0 {
469+
elideChildren = true
470+
}
471+
if elideChildren {
425472
// The error wants to elide the short messages from inner
426473
// causes. Do it.
427-
s.elideFurtherCauseMsgs()
474+
s.elideShortChildren(numChildren)
428475
}
429476
}
430477
}
431478

432479
// Collect the result.
433-
entry := s.collectEntry(err, bufIsRedactable)
480+
entry := s.collectEntry(err, bufIsRedactable, withDepth, depth)
434481

435482
// If there's an embedded stack trace, also collect it.
436483
// This will get either a stack from pkg/errors, or ours.
@@ -444,21 +491,26 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
444491
// Remember the entry for later rendering.
445492
s.entries = append(s.entries, entry)
446493
s.buf = bytes.Buffer{}
494+
495+
return numChildren + 1
447496
}
448497

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
498+
// elideShortChildren takes a number of entries to set `elideShort` to
499+
// false. The reason a number of entries is needed is because
500+
//
501+
// TODO(davidh): I have a hypothesis that the `newEntries` arg is
502+
// unnecessary and it's correct to elide all children whenever this is
503+
// called, even with multi-cause scenarios. This is because it's
504+
// impossible for a multi-cause tree to later "switch" to a
505+
// single-cause chain, so any time a multi-cause error is recursing
506+
// you're guaranteed to only have its children within.
507+
func (s *state) elideShortChildren(newEntries int) {
508+
for i := 0; i < newEntries; i++ {
509+
s.entries[len(s.entries)-1-i].elideShort = true
458510
}
459511
}
460512

461-
func (s *state) collectEntry(err error, bufIsRedactable bool) formatEntry {
513+
func (s *state) collectEntry(err error, bufIsRedactable bool, withDepth bool, depth int) formatEntry {
462514
entry := formatEntry{err: err}
463515
if s.wantDetail {
464516
// The buffer has been populated as a result of formatting with
@@ -495,6 +547,10 @@ func (s *state) collectEntry(err error, bufIsRedactable bool) formatEntry {
495547
}
496548
}
497549

550+
if withDepth {
551+
entry.depth = depth
552+
}
553+
498554
return entry
499555
}
500556

@@ -712,6 +768,11 @@ type formatEntry struct {
712768
// truncated to avoid duplication of entries. This is used to
713769
// display a truncation indicator during verbose rendering.
714770
elidedStackTrace bool
771+
772+
// depth, if positive, represents a nesting depth of this error as
773+
// a causer of others. This is used with verbose printing to
774+
// illustrate the nesting depth for multi-cause error wrappers.
775+
depth int
715776
}
716777

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

734795
for i, c := range b {
735796
if c == '\n' {
797+
//if s.needNewline > 0 {
798+
// for i := 0; i < s.needNewline-1; i++ {
799+
// s.buf.Write(detailSep[:len(sep)-1])
800+
// }
801+
// s.needNewline = 0
802+
//}
736803
// Flush all the bytes seen so far.
737804
s.buf.Write(b[k:i])
738805
// Don't print the newline itself; instead, prepare the state so
@@ -762,6 +829,11 @@ func (s *state) Write(b []byte) (n int, err error) {
762829
s.notEmpty = true
763830
}
764831
}
832+
//if s.needNewline > 0 {
833+
// for i := 0; i < s.needNewline-1; i++ {
834+
// s.buf.Write(detailSep[:len(sep)-1])
835+
// }
836+
//}
765837
s.buf.Write(b[k:])
766838
return len(b), nil
767839
}
@@ -788,6 +860,9 @@ func (p *state) switchOver() {
788860
p.buf = bytes.Buffer{}
789861
p.notEmpty = false
790862
p.hasDetail = true
863+
864+
// One of the newlines is accounted for in the switch over.
865+
// p.needNewline -= 1
791866
}
792867

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

0 commit comments

Comments
 (0)