AppendInvoke silently drops errors subtly

[thomasdesr]

Thanks for putting together this library!

The behavior I found surprising was that if you do not use the less-common pattern where you define variables in the function return signature, that using defer multierr.AppendInvoke(&err, someInvoker) will not have the error returned by someInvoker included in the error that is returned by the function.

Example: https://go.dev/play/p/JoSVtFwapQL

Note how outer20 is missing foo from its list of errors.

I guess Go's runtime is returning (or copying the return value) before the defer is running when the return value is defined in the function body vs in the function signature.

[rabbbit]

This seems to be expected given how defer works in Go.

https://go.dev/play/p/eF8TmyEnTxt

func t1() int {
	var i int
	defer func() {
		i++
	}()
	return i
}

func t2() (i int) {
	defer func() {
		i++
	}()
	return i
}

func main() {
	fmt.Println(t1())
	fmt.Println(t2())
}

Results in:

0
1
Program exited.

Even playing with pointers does not work around this:

func t3() int {
	var i int
	ii := &i
	defer func() {
		*ii++
	}()
	return *ii
}
// returns 0

My brief reading of the spec here https://go.dev/ref/spec#Defer_statements calls out that you need to be using named returns for defers to modify the return value:

For instance, if the deferred function is a function literal and the surrounding function has named result parameters that are in scope within the literal, the deferred function may access and modify the result parameters before they are returned.

[abhinav]

This is a good point, but yeah @rabbbit is right, because of how defers work, the library can't address this. We can do two things here:

• Update the documentation of AppendInvoke to loudly state that if you're using it inside a defer, the variable must be a named return.
• In the future, we can look into building a go/analysis-based linter to catch these cases since this is pretty easily lintable.

[abhinav]

Closing because documentation was updated in https://github.com/uber-go/multierr/pull/63, and there's nothing we can do at the library level besides that.