Version of Append that does not flatten

[bits01]

I realize that Append() was fixed at some point to flatten, but I believe it's valuable to have a version of it that does not flatten MultiErrors. Use case: execute a bunch of tasks in parallel, wait for them to finish, collect the errors from the failed tasks and return them to the caller as a single MultiError. I would like to be able to preserve any MutliError generated by each task rather than ending up with a flat list of errors that lost their context, the demarcation points between tasks if you will.

[mentalisttraceur]

@bits01 Yo check it:

Code with a solution to an example of your problem:

package main

import (
	"fmt"

	"github.com/hashicorp/errwrap"
	"github.com/hashicorp/go-multierror"
)

func subTask1(errorChannel chan error) {
	var err error
	err = multierror.Append(err, fmt.Errorf("error foo"))
	err = multierror.Append(err, fmt.Errorf("error bar"))
	errorChannel <- err
}

func subTask2(errorChannel chan error) {
	var err error
	err = multierror.Append(err, fmt.Errorf("error foo"))
	errorChannel <- err
}

func subTask3(errorChannel chan error) {
	var err error
	err = multierror.Append(err, fmt.Errorf("error foo"))
	err = multierror.Append(err, fmt.Errorf("error bar"))
	errorChannel <- err
}

func main() {
	errorChannel1 := make(chan error, 1)
	errorChannel2 := make(chan error, 1)
	errorChannel3 := make(chan error, 1)

	go subTask1(errorChannel1)
	go subTask2(errorChannel2)
	go subTask3(errorChannel3)

	err1 := <- errorChannel1
	err2 := <- errorChannel2
	err3 := <- errorChannel3

	var err error

	// Prefix each error in the multierror with a nested tab+asterisk
	err1 = multierror.Prefix(err1, "\t*")
	err2 = multierror.Prefix(err2, "\t*")
	err3 = multierror.Prefix(err3, "\t*")

	// Wrap each multierror to stop multierror flattening
	err1 = errwrap.Wrap(err1, nil)
	err2 = errwrap.Wrap(err1, nil)
	err3 = errwrap.Wrap(err3, nil)

	err = multierror.Append(err, err1, err2, err3)
	fmt.Println(err)
}

Output:

3 errors occurred:
	* 2 errors occurred:
	*	* error foo
	*	* error bar


	* 1 error occurred:
	*	* error foo


	* 2 errors occurred:
	*	* error foo
	*	* error bar

multierror.Prefix by itself just prefixes each individual error but does not prevent flattening in the data structure itself. If that's good enough, you can just use that if you make each prefix start with a number or other unique identifier instead of "\t*".

errwrap.Wrap preserves the nested structure itself, by wrapping the multierror.Error in a different type, but does nothing for the error string formatting, so the nested errors would end up at the same level, just separated by the empty lines like above.

errwrap is already used internally by at least one file in multierror, so you're not really adding a dependency, but if you didn't want to use it, you could easily make your own simple wrapper error interface type.

[mentalisttraceur]

@mitchellh and the rest of the Hashicorp folks, how do you feel about adding something like this into multierror to solve this usecase for anyone who has it?

func DontFlatten(err error) {
	if _, ok := err.(*Error); !ok {
		return err
	}
	err = Prefix(err, "\t*")
	return errwrap.Wrap(err, nil)
}

Usage would look like this

mErrs := new(multierror.Error)

// ... got an `err` that may be a `multierror.Error` that we don't want to flatten:

err = multierror.Append(mErrs, multierror.DontFlatten(err))

If the answer is yes, I'll happily do a pull request and sign your CLA or whatever. In the meantime I license it under the Zero-Clause BSD license (SPDX short identifier: 0BSD) and to my knowledge it doesn't (and in most jurisdictions can't, because it's so obvious) fall under any patents, so I'm pretty sure legally you're allowed to use it as if it was public domain anyway (but this does not consistute legal advice, I'm not a lawyer, etc).

[mentalisttraceur]

Some potential downsides I can see to the above proposed multierror.DontFlatten function:

1. The gradually widening whitespace gaps with each layer of nesting are kinda ugly.

2. It's not immediately intuitively obvious why it works out "correctly" - a person has to sorta step through the interaction between each nesting level's multierror.Error's formatting function.

3. The injection of a prefix into every sub-error might not be very convenient for people who want to prevent nesting but who are also customizing their multierror formatter functions.

4. I haven't thought through how jamming a nil into the back of errwrap.Wrap interacts with all the various uses of errwrap out there - maybe it's bad somehow?

5. In general I'm sorta abusing errwrap.Wrap here - a separate, dedicated wrapper type might be better.

6. A separate multierror.DontFlatten function that munges arguments intended for multierror.Append might not be the best interface - maybe a multierror.AppendWithoutFlattening function is more human-friendly?

Despite all this, it's probably still good enough for the most common and the most simple usecases as-is, but that's not for me to decide so I wanted to make sure I drew attention to all these.

[mentalisttraceur]

Update re: above CLA comment: I've now signed your CLA.

[mentalisttraceur]

Question: are people like me just missing some wisdom in favor of not nesting errors?

It seems that both Hashicorp's multierror and Uber's multierr exclusively expose a way that flattens the errors.

Past experience suggests that when I find myself limited by something like this, it could be just because I'm wrong in wanting to do what I'm trying to do.

It seems that my own current use-cases could be equally solved by:

1. Making sure to just pre-stringify any error or multierror that I have which I find myself wanting to nest, before nesting it.

   Importantly, I would have to do this anyway because I want to ensure unambiguous quoting/escaping/representation of the errors.

2. Always making sure my errors at those logical boundaries where I do this nesting are multierrors (this imposes a logical regularity across the board and avoids the need for special cases at all levels that touch the logic).

Okay, I think I have some clarity now.

My desire for this feature was naive.

That doesn't mean it's wrong - there may be a right reason for wanting it that I just haven't seen yet.

But it seem that every place I found myself wanting it can and should be solved another way.

[dolmen]

errors.Join (go1.20) doesn't flatten.

(and I'm not using mulierror.Group because it flattens error trees)