Proposal: Ergonomic error returns

[TWRaven]

A common pattern I've seen in code is this:

something, err := fetchSomething()

if err != nil {
	return nil, err
}

return something, nil

and this

something, err := fetchSomething()

if err != nil {
	return nil, errors.wrap(err, "extra info")
}

return something, nil

Would something like this:

func Return[T any](x T, err error) (T, error) {
	if err != nil {
		return nil, err
	}

	return x, nil
}

and one with wrapping

func ReturnWrapped[T any](x T, err error, info string) (T, error) {
	if err != nil {
		return nil, errors.wrap(err,  info)
	}

	return x, nil
}

be a good addition?

That woud simplify the previous cases to:

lo.Return(fetchSomething())

and

lo.ReturnWrapped(fetchSomething(), "extra info")

I understand that using lo.Ternary would already make this a one liner, but that would still introduce a lot of repeated behaviour (swapping the parameters, wrapping the error).

[ghostsquad]

this really needs "lazy" syntax. There's a huge thread about this here: golang/go#37739

basically, the problem you'll run into when developing something like this, is that you essentially want to wait to evaluate something (like the error string, especially in the case of using errors.Wrapf) until the last moment.

[ghostsquad]

but, there's a bit of a work around today (though without caching behavior) simply be func() T, so instead of

func ReturnWrapped[T any](x T, err error, info string) (T, error) {

it would be:

func ReturnWrapped[T any](x T, err error, infoFn func() string) (T, error) {

The function signature could be more flexible as well, to work both with Wrap and Wrapf:

func ReturnWrapped[T any](x T, err error, msgAndArgs ...func() any) (T, error) {

in this way:

switch l := len(msgAndArgs); {
case l == 0:
  return errors.WithStack(err)
case l == 1:
  return errors.Wrap(err, string(msgAndArgs[0]()))
default:
  msg := msgAndArgs[0]
  args := msgAndArgs[1:]
  argsAsAny := lo.Map(args, // more here)
  return errors.Wrapf(err, string(msg()), argsAsAny...)
}

or something to this effect

inspiration from stretchr/testify: https://pkg.go.dev/github.com/stretchr/testify/assert#Contains

func Contains(t TestingT, s, contains interface{}, msgAndArgs ...interface{}) bool

[eplusminus]

I think there's a little bit of confusion here about Go typing. Assuming that a generic helper like Return is desirable (which I'm not sure it is, but I'll get to that in a minute), your proposed implementation of it won't compile— because if T can be literally any type, then there's no guarantee that nil is an allowable value for that type.

There are a few ways to work around that kind of thing with generics, depending on what kinds of use cases you want to support. If you only ever want this to work with pointer types, then that's easy: just change the signature to func Return[T any](x *T, err error) (*T, error). Now the first parameter is guaranteed to be a pointer to some type, and so is the return value, so either of those can be nil. But then it won't work with, for instance, an interface type. The more general way to go would be this:

func Return[T any](x T, err error) (T, error) {
	if err != nil {
		var zeroValue T
		return zeroValue, err
	}
	return x, nil
}

The part with zeroValue (which is equivalent to lo.Empty[T]()) simply uses an uninitialized variable to provide whatever Go considers to be the zero value for that type. For a pointer or an interface, that would be nil; for an int, it's 0; etc.

Now— is this function useful? It depends on what you're trying to accomplish. The only difference between your original example and simply writing return fetchSomething() is if fetchSomething() might ever return a non-nil first value and a non-nil error. That's an uncommon pattern in Go, but it's not against the law or anything; if that is really a meaningful condition, for whatever fetchSomething() is trying to do, then you wouldn't necessarily want to throw away that first value and substitute nil for it.

But if there's a reason why you need to do so, and you do it often enough that a generic helper would be useful, I would definitely recommend naming it something more specific. The only thing that this helper is doing that is different from a plain return is that it's discarding the first value if there is an error. Naming it Return not only doesn't clarify what's happening— it's actively misleading, because Return(fetchSomething()) looks almost identical to return fetchSomething() but has different behavior. A better name might be DiscardValueIfError, or just about anything other than Return.

[ghostsquad]

I actually just realized something. If you are using github.com/pkg/errors you don't need to do nil checking. As wrap, withStack, etc return nil if the error input is nil.

[ghostsquad]

From a memory management standpoint, discardOnError does seem useful. I don't think that's what the complaint was though.