proposal: errors: unwarap joinError to avoid deeply-nested joins

[sethvargo]

Proposal Details

The current implementation of joinError appends the given errors onto an internal []error. However, it does not take into account itself, leading to unexpected internal structures. This is perhaps best illustrated by an example:

var merrA error
merrA = errors.Join(merrA, fmt.Errorf("one"))
merrA = errors.Join(merrA, fmt.Errorf("two"))
merrA = errors.Join(merrA, fmt.Errorf("three"))

var merrB error
merrB = errors.Join(merrB, fmt.Errorf("one"), fmt.Errorf("two"), fmt.Errorf("three"))

One would likely expect merrA and merrB to be structurally equal. While they "print out" the same error, their internal representation differs:

merrA = &joinError{
  errs: []error{
    fmt.Errorf("one"),
    &joinError{
      errs: []error{
        fmt.Errorf("two"),
        &joinError{
          errs: []error{
            fmt.Errorf("three"),
          },
        },
      },
    },
  },
}

merrB = &joinError{
  errs: []error{
    fmt.Errorf("one"),
    fmt.Errorf("two"),
    fmt.Errorf("three"),
  }
}

My proposal is that errors.Join special-case the internal joinError type to "unwrap" the inner errors and flatten the slice.

I believe this change is fully backwards-compatible with any documented APIs and public interfaces. There are no test cases that cover this expected nesting as shown in #65360.

I also believe this will lead to more performant error printing, since it reduces the need to recurse multierrors, but I do not have time to gather data and benchmark my gut feeling.

I could not find any corresponding issues, but "errors" and "unwrap" are not particularly unique keywords, so apologies if I missed them.

• GitHub: errors: unwrap joinError to avoid deeply-nested joins #65360
• Gerrit: https://go-review.googlesource.com/c/go/+/559355

[seankhliao]

It was an explicit design choice not to flatten errors #53435 (comment)

[neild]

One would likely expect merrA and merrB to be structurally equal.

Why?

The documentation for Join says that it "wraps the given errors". If it flattened its inputs, that statement would not be accurate, because Join(err) could return an error that does not wrap err.

I do not believe changing this would be backwards compatible.

In addition, if Join flattened errors produced by Join, but not other errors implementing Unwrap() []error, this would be a point of confusion in the API. We currently don't preference errors produced by the errors package over errors produced by other packages in any way, and I don't think we should change that. This was a significant motivation in choosing the current behavior of Join--it is by design simple and without special cases.