-
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: Use Context for the proposed built-in Go error check function, "try" #32853
Comments
What's a |
There are various implementations, but this gives a good sense of the concept: https://godoc.org/context You can also check out https://godoc.org/github.com/gin-gonic/gin#Context for an alternative implementation. |
@mibes Sorry, I wasn't clear (I know what a In your proposal you use |
Fair enough. The key difference with the original proposal is that although the Error in Context can be nil, the Context itself can not; it is always set and can be passed down to the handlers in the chain. I'll update the proposal based on your feedback. Let me know if it is still unclear. Thanks! |
Not sure why this should be an interface, the proposal does not state if and whether it would have different implementations, and what roles they would play. It seems much easier to just have a handler like this:
in order to support your abort/dismiss ideas. The equivalent of your dismiss would just be returning nil as the error. The second, boolean return argument, would represent an abort, if that is really needed. |
The concept of wrapping the error in a Context is that it does not need an explicit action from the developer to return the error. If the error is not explicitly updated, the function just "falls through" with the original error still in place. As mentioned, the framework could implement the interface in such a way that the error can never be |
This seems like a great deal of special purpose machinery to add just for error handling. While doing something for error handling may well be worthwhile, we still don't want to add too much additional complexity. If this could be in a separate package it would be more acceptable, but as far as I can see it must be built into the language itself. |
I think it's useful to give the function name. That will make it's easy to be used by all try. type Context interface {
Error() error // original error
FuncName() string // the function name we handled
}
func foo() error {
handler := func(c Context) error {
return fmt.Errorf("%s: %v", c.FuncName(), c.Error()) // wrap something
//return nil // or dismiss
}
a, b := try(bar1(), handler)
c, d := try(bar2(), handler)
} or just remove Context func foo() error {
handler := func(err error, funcname string) error {
return fmt.Errorf("%s: %v", funcname, err) // wrap something
//return nil // or dismiss
}
a, b := try(bar1(), handler)
c, d := try(bar2(), handler)
} |
This wouldn't remove boilerplate for me because I like wrapping errors. I would need a new handler function for each error. Each line would become something similar to:
I like that the error is contained to the scope of the handler function, but since it is a function the ability to handle in other ways like |
I want to read the code fluently. The |
Thanks for all your comments. I've closed this issue, because the referred proposal #32437 has been closed. |
updated based on the feedback from @DisposaBoy
This proposal is related to #32437. I opened a new issue for it, to not side-track the discussion that is ongoing on the main thread.
TL;DR: keep the current proposal largely intact, but use explicit error handling with a Context.
Background
In the design iterations discussion, @griesemer commented on the concept of user-defined error handlers. That concept was dismissed largely due to these concerns: What should happen if the handler is provided but is nil? Should try panic or treat it as an absent error handler? What if the handler is invoked with a non-nil error and then returns a nil result?
Suggestion
I'd like to suggest to revisit this concept with a few adjustments.
Rather than accepting a handler that takes and returns an error (as suggested initially), consider to use a void function that takes a Context. The Context would be an interface like this:
And used like this:
When the handler is defined the c.err is set by
try
and the handler is called. This internal value is available through c.Err() and can be replaced with c.Error(error).When no handler is defined, or the handler is nil,
try
will directly return the error. So these are equivalents:The use of a Context would also allow developers to chain together handlers, similar to the way Gin Gonic handles routes:
The handler would implicitly fall through without the need for c.Next(). If
try
hits a nil handler it will return with the value of c.err.Explicitly aborting the Context would stop the handler chain and have
try
set the values to whatever the original function returned:This allows for the backward compatibility where a function would return a non-critical error and valid values.
Calling c.Abort() is the only way to break the handler chain and have
try
not return with an error.If developers explicitly set c.err to
nil
using the Dimiss() function,try
would handle this as any other situation and passnil
down the chain. At the end of the chaintry
returns with the exact value of c.err.If this behaviour is undesired, c.Error() could reject a
nil
value and c.Dismiss() could be removed from the Context interface altogether.With this proposal this short-hand code would still be possible:
I hope this concept addresses some of the original concerns of the explicit error handling with
try()
, while retaining the benefits of the current design, and allowing for alternative scenarios that don't obfuscate the code clarity.The text was updated successfully, but these errors were encountered: