Skip to content

Commit

Permalink
add formatting for multi-cause errors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dhartunian committed Aug 16, 2023
1 parent fe9cc0f commit 6cb12a7
Show file tree
Hide file tree
Showing 13 changed files with 1,555 additions and 63 deletions.
131 changes: 106 additions & 25 deletions errbase/format_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,13 @@ func formatErrorInternal(err error, s fmt.State, verb rune, redactableOutput boo
// to enable stack trace de-duplication. This requires a
// post-order traversal. Since we have a linked list, the best we
// can do is a recursion.
p.formatRecursive(err, true /* isOutermost */, true /* withDetail */)
p.formatRecursive(
err,
true, /* isOutermost */
true, /* withDetail */
false, /* withDepth */
0, /* depth */
)

// We now have all the data, we can render the result.
p.formatEntries(err)
Expand Down Expand Up @@ -146,7 +152,13 @@ func formatErrorInternal(err error, s fmt.State, verb rune, redactableOutput boo
// by calling FormatError(), in which case we'd get an infinite
// recursion. So we have no choice but to peel the data
// and then assemble the pieces ourselves.
p.formatRecursive(err, true /* isOutermost */, false /* withDetail */)
p.formatRecursive(
err,
true, /* isOutermost */
false, /* withDetail */
false, /* withDepth */
0, /* depth */
)
p.formatSingleLineOutput()
p.finishDisplay(verb)

Expand Down Expand Up @@ -195,7 +207,19 @@ func (s *state) formatEntries(err error) {
// Wraps: (N) <details>
//
for i, j := len(s.entries)-2, 2; i >= 0; i, j = i-1, j+1 {
fmt.Fprintf(&s.finalBuf, "\nWraps: (%d)", j)
s.finalBuf.WriteByte('\n')
// Extra indentation starts at depth==2 because the direct
// children of the root error area already printed on separate
// newlines.
for m := 0; m < s.entries[i].depth-1; m += 1 {
if m == s.entries[i].depth-2 {
s.finalBuf.WriteString("└─ ")
} else {
s.finalBuf.WriteByte(' ')
s.finalBuf.WriteByte(' ')
}
}
fmt.Fprintf(&s.finalBuf, "Wraps: (%d)", j)
entry := s.entries[i]
s.printEntry(entry)
}
Expand Down Expand Up @@ -330,12 +354,34 @@ func (s *state) formatSingleLineOutput() {
// s.finalBuf is untouched. The conversion of s.entries
// to s.finalBuf is done by formatSingleLineOutput() and/or
// formatEntries().
func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
//
// `withDepth` and `depth` are used to tag subtrees of multi-cause
// errors for added indentation during printing. Once a multi-cause
// error is encountered, all subsequent calls with set `withDepth` to
// true, and increment `depth` during recursion. This information is
// persisted into the generated entries and used later to display the
// error with increased indentation based in the depth.
func (s *state) formatRecursive(err error, isOutermost, withDetail, withDepth bool, depth int) int {
cause := UnwrapOnce(err)
numChildren := 0
if cause != nil {
// Recurse first.
s.formatRecursive(cause, false /*isOutermost*/, withDetail)
// Recurse first, which populates entries list starting from innermost
// entry. If we've previously seen a multi-cause wrapper, `withDepth`
// will be true, and we'll record the depth below ensuring that extra
// indentation is applied to this inner cause during printing.
// Otherwise, we maintain "straight" vertical formatting by keeping the
// parent callers `withDepth` value of `false` by default.
numChildren += s.formatRecursive(cause, false, withDetail, withDepth, depth+1)
}

causes := UnwrapMulti(err)
for _, c := range causes {
// Override `withDepth` to true for all child entries ensuring they have
// indentation applied during formatting to distinguish them from
// parents.
numChildren += s.formatRecursive(c, false, withDetail, true, depth+1)
}
// inserted := len(s.entries) - 1 - startChildren

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

case Formatter:
desiredShortening := v.FormatError((*printer)(s))
if desiredShortening == nil {
// The error wants to elide the short messages from inner
// causes. Do it.
s.elideFurtherCauseMsgs()
s.elideShortChildren(numChildren)
}

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

Expand All @@ -412,7 +460,7 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
if desiredShortening == nil {
// The error wants to elide the short messages from inner
// causes. Do it.
s.elideFurtherCauseMsgs()
s.elideShortChildren(numChildren)
}
break
}
Expand All @@ -421,16 +469,21 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail bool) {
// If the error did not implement errors.Formatter nor
// fmt.Formatter, but it is a wrapper, still attempt best effort:
// print what we can at this level.
if elideCauseMsg := s.formatSimple(err, cause); elideCauseMsg {
elideChildren := s.formatSimple(err, cause)
// always elideChildren when dealing with multi-cause errors.
if len(causes) > 0 {
elideChildren = true
}
if elideChildren {
// The error wants to elide the short messages from inner
// causes. Do it.
s.elideFurtherCauseMsgs()
s.elideShortChildren(numChildren)
}
}
}

// Collect the result.
entry := s.collectEntry(err, bufIsRedactable)
entry := s.collectEntry(err, bufIsRedactable, withDepth, depth)

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

return numChildren + 1
}

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

func (s *state) collectEntry(err error, bufIsRedactable bool) formatEntry {
func (s *state) collectEntry(err error, bufIsRedactable bool, withDepth bool, depth int) formatEntry {
entry := formatEntry{err: err}
if s.wantDetail {
// The buffer has been populated as a result of formatting with
Expand Down Expand Up @@ -495,6 +553,10 @@ func (s *state) collectEntry(err error, bufIsRedactable bool) formatEntry {
}
}

if withDepth {
entry.depth = depth
}

return entry
}

Expand Down Expand Up @@ -712,6 +774,11 @@ type formatEntry struct {
// truncated to avoid duplication of entries. This is used to
// display a truncation indicator during verbose rendering.
elidedStackTrace bool

// depth, if positive, represents a nesting depth of this error as
// a causer of others. This is used with verbose printing to
// illustrate the nesting depth for multi-cause error wrappers.
depth int
}

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

for i, c := range b {
if c == '\n' {
//if s.needNewline > 0 {
// for i := 0; i < s.needNewline-1; i++ {
// s.buf.Write(detailSep[:len(sep)-1])
// }
// s.needNewline = 0
//}
// Flush all the bytes seen so far.
s.buf.Write(b[k:i])
// Don't print the newline itself; instead, prepare the state so
Expand Down Expand Up @@ -762,6 +835,11 @@ func (s *state) Write(b []byte) (n int, err error) {
s.notEmpty = true
}
}
//if s.needNewline > 0 {
// for i := 0; i < s.needNewline-1; i++ {
// s.buf.Write(detailSep[:len(sep)-1])
// }
//}
s.buf.Write(b[k:])
return len(b), nil
}
Expand All @@ -788,6 +866,9 @@ func (p *state) switchOver() {
p.buf = bytes.Buffer{}
p.notEmpty = false
p.hasDetail = true

// One of the newlines is accounted for in the switch over.
// p.needNewline -= 1
}

func (s *printer) Detail() bool {
Expand Down
Loading

0 comments on commit 6cb12a7

Please sign in to comment.