-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
Comments
Interesting, but changing the signature of |
Yeah, unfortunate. I think 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
} |
@rsc made the case in the original proposal issue that a type-parameterized version of @dsnet also pointed out that the non-type-parameterized version of I don't recall if there were any other arguments against a type-parameterized I personally think that a type-parameterized An argument against adding a type-parameterized |
New proposal: Automatically alias all functions with names that match |
I like this proposal, but I fell the call 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
} |
Or more generally, it would be good to pass type arguments to any value parameters of type parameter types. [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 |
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. |
That's not quite true: This does add type safety at the language level, rather than leaving it to a
The equivalent
|
A sidenote is the original error inspection draft design includes this blurb:
so its not I guess a new idea |
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)
} |
There are a few benefits of having a generic version of 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
|
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 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 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. |
This would break the following:
|
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. |
#64629 was closed as duplicate, so let me advertise my idea here. Perhaps, instead of adding new func AsTarget[T error](err T) *T {
p1 := &err
return p1
} Then we could:
|
You couldn’t use the value, so it seems like it wouldn’t be useful most of the time. |
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 Learned something today, thanks @carlmjohnson. |
@mkielar: If you do not need value, you use |
Per (closed) duplicate proposal #64771, I advocate for the name // 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))
} |
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 |
err = container.RunExec(ctx, s.dockerCli, target.ID, exec)
if errors.Has[*cli.StatusError](err) {
return sterr.StatusCode, nil
}
return 0, err
|
Until this gets into the stdlib, I'm using a simple wrapper around the whole |
In our codebase, we've noticed the need for two different methods:
I doubt both methods would get included in the standard library, but besides added type safety and convenience, I've found that both methods make for much cleaner and simpler error-checking code. |
@slnt FWIW, I've just released jub0bs/errutil, a small utility package that exports the following function: func Find[T error](err error) (T, bool)
|
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: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:This enables more "ergonomic" usage as follows:
instead of
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.The text was updated successfully, but these errors were encountered: