Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: errors: As with type parameters #51945

Open
slnt opened this issue Mar 25, 2022 · 23 comments
Open

proposal: errors: As with type parameters #51945

slnt opened this issue Mar 25, 2022 · 23 comments
Labels
error-handling Language & library change proposals that are about error handling. Proposal
Milestone

Comments

@slnt
Copy link

slnt commented Mar 25, 2022

Currently in 1.18 and before, when using the errors.As method, an error type you would like to write into must be predeclared before calling the function. For example:

var myErr *MyCustomError
if errors.As(err, &myErr) {
  // handle myErr
}

This can makes control flow around handling errors "unergonomic".

I'd propose that a new, type parameterized method be added to the errors package in 1.19:

func IsA[T error](err error) (T, bool) {
	var isErr T
	if errors.As(err, &isErr) {
		return isErr, true
	}

	var zero T
	return zero, false
}

This enables more "ergonomic" usage as follows:

err := foo()
if err != nil {
	if myErr, ok := errors.IsA[*MyCustomError](err); ok {
		// handle myErr
	} else if otherErr, ok := errors.IsA[*OtherError](err); ok {
		// handle otherErr
	}

	// handle everything else
}

instead of

err := foo()
if err != nil {
	var myErr *MyCustomError
	if errors.As(err, &myErr) {
		// handle customer error
	}

	var otherErr *OtherError
	if errors.As(err, &otherErr) {
		// handle other error
	}

	// handle everything else
}

This change would reduce the overall LOC needed for handling custom errors, imo improves readability of the function, as well as scopes the errors to the if blocks they are needed in.

Naming is hard so IsA might be better replaced with something else.

@slnt slnt added the Proposal label Mar 25, 2022
@gopherbot gopherbot added this to the Proposal milestone Mar 25, 2022
@seankhliao
Copy link
Member

Interesting, but changing the signature of errors.As is not a backwards compatible change.
It will have to be a new function.

@seankhliao seankhliao changed the title proposal: errors: change signature of errors.As to use type parameters in 1.19 proposal: errors: As with type parameters Mar 25, 2022
@slnt
Copy link
Author

slnt commented Mar 25, 2022

Interesting, but changing the signature of errors.As is not a backwards compatible change. It will have to be a new function.

Yeah, unfortunate. I think if customerErr, ok := errors.IsA[*CustomError](err); ok reads quite nicely. It would be implemented pretty simply in terms of errors.As:

func IsA[T error](err error) (T, bool) {
	var isErr T
	if errors.As(err, &isErr) {
		return isErr, true
	}

	var zero T
	return zero, false
}

@seankhliao seankhliao added the error-handling Language & library change proposals that are about error handling. label Mar 25, 2022
@ianlancetaylor
Copy link
Contributor

CC @jba @neild

(My vague recollection is that this was considered and rejected when errors.As was introduced, even beyond the fact that at the time type parameters did not yet exist.)

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Mar 25, 2022
@neild
Copy link
Contributor

neild commented Mar 25, 2022

@rsc made the case in the original proposal issue that a type-parameterized version of errors.As would not be an improvement:
#29934 (comment)

@dsnet also pointed out that the non-type-parameterized version of As can be used in a switch, while the type-parameterized one cannot:
#29934 (comment)

I don't recall if there were any other arguments against a type-parameterized As (aside from, obviously, the fact that type parameters didn't exist at the time).

I personally think that a type-parameterized As seems fairly reasonable, although the practical benefit over the current As seems small. It would need a good name. I don't know what that name would be. IsA does not seem right. Under the current design, "Is" is an enhanced form of equality and "As" is an enhanced form of type assertion; blurring that distinction would add confusion.

An argument against adding a type-parameterized As is that the benefit does not justify the cost in API churn and user confusion. I don't have a strong opinion on whether the benefits do outweigh the costs.

@DeedleFake
Copy link

I personally think that a type-parameterized As seems fairly reasonable, although the practical benefit over the current As seems small. It would need a good name. I don't know what that name would be. IsA does not seem right. Under the current design, "Is" is an enhanced form of equality and "As" is an enhanced form of type assertion; blurring that distinction would add confusion.

AsA() could work, though it's a little oddly close to the existing one. if pe, ok := errors.AsA[*os.PathError](err); ok { ... } reads pretty well to me, though it should technically be 'an *os.PathError', not 'a'.

New proposal: Automatically alias all functions with names that match ([a-z0-9])A$ to ${1}An.

@go101
Copy link

go101 commented Mar 26, 2022

I like this proposal, but I fell the call errors.IsA[*MyCustomError](err) is not very natural.
If would be good if we could pass type arguments to unnamed value parameters (_) of type parameter types.
For example,

func IsA[T error](err error, _ T) (T, bool) {
	var isErr T
	if errors.As(err, &isErr) {
		return isErr, true
	}

	var zero T
	return zero, false
}

err := foo()
if err != nil {
	if myErr, ok := errors.IsA(err, MyCustomError); ok {
		// handle myErr
	} else if otherErr, ok := errors.IsA(err, OtherError); ok {
		// handle otherErr
	}

	// handle everything else
}

@go101
Copy link

go101 commented Mar 26, 2022

Or more generally, it would be good to pass type arguments to any value parameters of type parameter types.
It is equivalent to pass zero values of the type arguments.

[Edit] This could unify built-in generic functions and custom ones to some extent.

func new[T any](T) *T

is much better than the illogical fake declaration:

func new(Type) *Type

@earthboundkid
Copy link
Contributor

I don't see much benefit to duplicating the existing API. This also implies a similar change to eg json.Marshal etc. We would end up with duplicate functions throughout the standard library. And as noted this doesn't add any type safety; it is just more convenient, although even that is debatable. I think generics should be reserved for areas where they either add real type safety or big convenience, and cases like this of minor convenience can stay as they are.

@neild
Copy link
Contributor

neild commented Mar 28, 2022

And as noted this doesn't add any type safety

That's not quite true: This does add type safety at the language level, rather than leaving it to a go vet check. Under this proposal, this code would not be valid:

type MyError struct{}
func (*MyError) Error() string { return "MyError" }

func main() {
	var err error
	m, ok := errors.AsA[MyError](err) // MyError does not implement error (Error method has pointer receiver)
	fmt.Println(m, ok)
}

The equivalent errors.As call is a run-time panic or go vet failure:

second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type

@slnt
Copy link
Author

slnt commented Apr 6, 2022

A sidenote is the original error inspection draft design includes this blurb:

Here we are assuming the use of the contracts draft design to make errors.As explicitly polymorphic:
func As(type E)(err error) (e E, ok bool)

so its not I guess a new idea

@GeorgeMac
Copy link

Love this, I stumbled across this pattern, made a quick blog post about it. Then, of course, that is when I find all the proposals 😂.

I think you can get it down to this terse definition:

func AsA[E error](err error) (e E, _ bool) {
    return e, errors.As(err, &e)
}

@phenpessoa
Copy link

phenpessoa commented Nov 27, 2022

There are a few benefits of having a generic version of As, that I don't think have been brought up here.
I mentioned them in: #56949

User @Jorropo wrote the function this way:

func AsOf[E error](err error) (E, bool) {
	var ptrToE *E
	for err != nil {
		if e, ok := err.(E); ok {
			return e, true
		}
		if x, ok := err.(interface{ As(any) bool }); ok {
			if ptrToE == nil {
				ptrToE = new(E)
			}
			if x.As(ptrToE) {
				return *ptrToE, true
			}
		}
		err = Unwrap(err)
	}
	var zero E
	return zero, false
}]

Note that it does not use the current As implementation.
The benefits of this implementation are:

  • No usage of reflection
  • No runtime panic possibility
  • An allocation free path
  • Compile time type safety
  • Faster

@joncalhoun
Copy link

joncalhoun commented Jun 6, 2023

All of these proposals are looking for a way to return the error as a specific type, which I agree is nice if possible, but have there been any discussions around simply improving errors.As with generics? Specifically:

func As[T error](err error, target *T) bool {
	// This is used to show the functionality works the same
	return errors.As(err, target)
}

At first glance this appears to be a breaking change, but passing anything that doesn't meet this criteria into the As function would result in a panic. This change would alert people of the issue sooner (during compile time) rather than at runtime.

It is also very possible I am missing an edge case.

I have looked, but haven't found an issue that discusses this approach. Please let me know if one exists.

@neild
Copy link
Contributor

neild commented Jun 6, 2023

At first glance this appears to be a breaking change

This would break the following:

var _ = errors.As

@joncalhoun
Copy link

joncalhoun commented Jun 6, 2023

This would break the following:

var _ = errors.As

Darn, you are right. I missed that case. Kinda sucks though, because the change would absolutely help with bugs that aren't discovered until testing or some other runtime occurrence.

@mkielar
Copy link

mkielar commented Dec 9, 2023

#64629 was closed as duplicate, so let me advertise my idea here. Perhaps, instead of adding new IsA function, it would be easier to add something that actually produces that double pointer? Like this:

func AsTarget[T error](err T) *T {
	p1 := &err
	return p1
}

Then we could:

if errors.As(err, AsTarget(&MyErr{})) {
    ...
}

@earthboundkid
Copy link
Contributor

You couldn’t use the value, so it seems like it wouldn’t be useful most of the time.

@mkielar
Copy link

mkielar commented Dec 10, 2023

Oooh, okay, now I get it. I'm new to go, and I didn't RTFM, so that's on me. I missed the fact that error.As actually sets the target to the value of the error it finds (if it finds it). My idea still holds when one doesn't need that value (which is rare, probably) and makes little sense in a wider context.

Learned something today, thanks @carlmjohnson.

@mitar
Copy link
Contributor

mitar commented Dec 10, 2023

@mkielar: If you do not need value, you use errors.Is.

@neilotoole
Copy link

Per (closed) duplicate proposal #64771, I advocate for the name errors.Has. Implementation would look like:

// Has returns true if err, or an error in its error tree, matches error type E.
// An error is considered a match by the rules of [errors.As].
func Has[E error](err error) bool {
  return errors.As(err, new(E))
}

@earthboundkid
Copy link
Contributor

A) It's an Is check, not really an As check, which is less useful. B) That would be very prone to accidental misuse in which the type system would infer error instead of a concrete error type.

@earthboundkid
Copy link
Contributor

  err = container.RunExec(ctx, s.dockerCli, target.ID, exec)
  if errors.Has[*cli.StatusError](err) {
    return sterr.StatusCode, nil
  }
  return 0, err

sterr.StatusCode won't work because sterr is never declared.

@imax9000
Copy link

imax9000 commented Jan 4, 2024

Until this gets into the stdlib, I'm using a simple wrapper around the whole errors package: https://pkg.go.dev/github.com/imax9000/errors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error-handling Language & library change proposals that are about error handling. Proposal
Projects
Status: Incoming
Development

No branches or pull requests