-
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: spec: simplify error handling - error passing with "pass" #37141
Comments
Seems slightly unfortunate that we always have to call the |
For language change proposals, please fill out the template at https://go.googlesource.com/proposal/+/refs/heads/master/go2-language-changes.md . When you are done, please reply to the issue with Thanks! |
@gopherbot please remove label WaitingForInfo |
My concern is that it would make it less explicit and maybe a little more difficult to understand. Also, Go allows separating statements with semicolons so you can technically do it in one line but not sure if it is something that would be preferred.
|
Why not go one step further and do something like:
|
Your version of WriteFile is wrong. While the original function will always close the file, your version will return if there is an error in f.Write or n<len(data) without closing the file. You could use defer, but then you will miss the error returned by f.Close. At the end, I think you would only be able to replace thee first return, so this may not be the most compelling example. |
@yiyus good catch! I updated the example to show something a bit more involved, and when |
It seems that if v != nil {
return ..., v // here ... is the zero value for the other result parameters
} It's unfortunate that error wrapping requires calling the function even when the error is if err == nil {
return nil
} and pull those lines into caller without inlining the whole function. The name Also the name One possibility would be to use return? err
return if err As you know, there has been a lot of discussion about error handling in Go. Is this language change big enough for the problem? It saves about six tokens and two lines. Is that worth the cost of a language change? |
I really like the You've already tried to change the language to fix this problem, so it seems that there is a general agreement that it is worth fixing, and this is just about as minimal, yet impactful, a change as could be made. If it survives the flamethrowers, it would probably build a good feeling in the community that there really is a willingness to fix the few existing sore points in the language.. |
The other possibility we could shorten with Return error (type errors) if not nil: Return true (boolean) if the error match: To return custom error (multiple variables):
|
It should be trivial for a simple tool to fix these usages (maybe
Instead of filling zero values for the result parameters, it is possible to do this:
In this case,
Normal usage:
but I think I like
If developers only had to write This proposal should fix the most common case. If you want to handle the error (not pass it), i like the current method. Errors are values, you can check for it with an if statement and do something. Adding something like This example looks pretty good as well if you prefer
It is much more ergonomic in my opinion than the current way (btw the example above takes only one extra line compared to |
We can certainly do this if we must. But it's a cost. We shouldn't just ignore that cost.
I guess. That doesn't seem like a natural meaning of "pass" to me, though. The first meaning that jumps to mind for me is to pass over something, meaning to ignore it. Any other opinions on this point? I don't dispute that |
I see now that a problem with |
it is returning the error up to the caller, and given that we already have a keyword that means Given that the language already has two forms of
|
What I like about Go is that it often picks the simplest approach to solve a problem. The proposed change is consistent with that. It is a minimal change, but very impactful. It is ergonomic. It reduces boilerplate code and helps you focus on the critical parts of a function. If you choose to go with |
This is a concern, especially for uses of Maybe as an alternative, an optional second argument could be given and, if it is, than the the first argument is only used for the // If err is not nil, execute and return the second argument.
return? err, fmt.Errorf("some extra information: %w", err)
// Alternative: Do a simple boolean check on the first argument. That
// makes this more cleanly just shorthand for an if statement.
return? err != nil, fmt.Errorf("some extra information: %w", err) The downside to this, though, is that with just a Edit: This is assuming that |
@DeedleFake I didn't use |
The more I think about it, the more I like the way errors are currently handled in Go. IMO, Not just that, but ignoring other use cases, for error handling, all this is doing is removing the need for writing Let's say that this is sufficient in term of error handling and we decide to go with To do that, Unlike what @DeedleFake mentioned, I don't think the default behavior should be checking if the first argument is nil, because if This leaves us with the following: something, err := getSomething()
return? err != nil, nil, err
return something, nil I think that this is a downgrade from the existing: something, err := getSomething()
if err != nil {
return nil, err
}
return something, nil Even though there's more LoC, at least there's no confusion, and the number of returned values match with the function signature. Also, the examples used for the func ValidatePassword(password string) (bool, error) {
if len(password) < MinPasswordLength {
return false, fmt.Errorf("password must have at least %d characters", MinPasswordLength)
}
if len(password) > MaxPasswordLength {
return false, fmt.Errorf("password must have less than %d characters", MaxPasswordLength)
}
return true, nil
} With func ValidatePassword(password string) (ok bool, err error) {
return? len(password) < MinPasswordLength, false, fmt.Errorf("password must have at least %d characters", MinPasswordLength)
return? len(password) > MaxPasswordLength, false, fmt.Errorf("password must have less than %d characters", MaxPasswordLength)
return true, nil
} It looks nice, but the lines are quite a bit longer - and there's just two values being returned. Personally, with named parameters, it'd look even better: func ValidatePassword(password string) (ok bool, err error) {
if len(password) < MinPasswordLength {
err = fmt.Errorf("password must have at least %d characters", MinPasswordLength)
} else if len(password) > MaxPasswordLength {
err = fmt.Errorf("password must have less than %d characters", MaxPasswordLength)
} else {
ok = true
}
return
} But I digress. That said, I think the suggestion for As @ianlancetaylor mentioned, I also think that the word |
@TwinProduction if
the conditional return will only check if the error value |
I haven't seen anybody try to address this point:
As far as the keyword goes, what about One of the arguments against the It's a little odd that in an expression like pass errors.Wrap(err, "x") the reader has to understand that |
@ianlancetaylor since it's a keyword, can't we make it lazy evaluate the call? func xyz() (int, err) {
x, err := fn()
pass errors.Wrap(err, "xyz")
if x != 0{
return x * x * x, nil
}
return 1, nil
}
// would be the same as:
func xyz() (int, err) {
x, err := fn()
if err != nil {
return 0, errors.Wrap(err, "xyz")
}
if x != 0{
return x * x * x, nil
}
return 1, nil
} |
Been thinking about this over the weekend and I was initially attracted to the idea but as I have processed the comments the idea of magically inlining parts of a wrapped function or eating the penalty of value creation and throwing it away feel untenable. But there's clearly a problem that needs to be solved otherwise we wouldn't be banging our heads against it ad infinitum. @bserdar's comment doesn't seem too off base in the service of trying to find a solution even if it is not direct feedback on the solution at hand. But the overloading of Looking at xbit's example in the proposal, here is an equivalent with less trade-offs and language impact... before: func newGitRepo(remote string, localOK bool) (Repo, error) {
r := &gitRepo{remote: remote}
if strings.Contains(remote, "://") {
var err error
r.dir, r.mu.Path, err = WorkDir(gitWorkDirType, r.remote)
if err != nil {
return nil, err
}
unlock, err := r.mu.Lock()
if err != nil {
return nil, err
}
defer unlock()
if _, err := os.Stat(filepath.Join(r.dir, "objects")); err != nil {
if _, err := Run(r.dir, "git", "init", "--bare"); err != nil {
os.RemoveAll(r.dir)
return nil, err
}
if _, err := Run(r.dir, "git", "remote", "add", "origin", "--", r.remote); err != nil {
os.RemoveAll(r.dir)
return nil, err
}
}
r.remoteURL = r.remote
r.remote = "origin"
} else {
if strings.Contains(remote, ":") {
return nil, fmt.Errorf("git remote cannot use host:path syntax")
}
if !localOK {
return nil, fmt.Errorf("git remote must not be local directory")
}
r.local = true
info, err := os.Stat(remote)
if err != nil {
return nil, err
}
if !info.IsDir() {
return nil, fmt.Errorf("%s exists but is not a directory", remote)
}
r.dir = remote
r.mu.Path = r.dir + ".lock"
}
return r, nil
} after: func newGitRepo(remote string, localOK bool) (Repo, error) {
r := &gitRepo{remote: remote}
if strings.Contains(remote, "://") {
r.dir, r.mu.Path, ^ = WorkDir(gitWorkDirType, r.remote)
unlock, ^ := r.mu.Lock()
defer unlock()
if _, err := os.Stat(filepath.Join(r.dir, "objects")); err != nil {
if _, err := Run(r.dir, "git", "init", "--bare"); err != nil {
os.RemoveAll(r.dir)
return nil, err
}
if _, err := Run(r.dir, "git", "remote", "add", "origin", "--", r.remote); err != nil {
os.RemoveAll(r.dir)
return nil, err
}
}
r.remoteURL = r.remote
r.remote = "origin"
} else {
if strings.Contains(remote, ":") {
return nil, fmt.Errorf("git remote cannot use host:path syntax")
}
if !localOK {
return nil, fmt.Errorf("git remote must not be local directory")
}
r.local = true
info, ^ := os.Stat(remote)
if !info.IsDir() {
return nil, fmt.Errorf("%s exists but is not a directory", remote)
}
r.dir = remote
r.mu.Path = r.dir + ".lock"
}
return r, nil
} |
The cost of an error expression instead of a boolean expression after
|
instead adding more complexity to the syntax, I'd propose to implement a typical Example: Before: func Parse12To24h(hour12hFormat string) (int, error) {
t, err := time.Parse("15PM", hour12hFormat)
if err != nil {
return -1, err
}
return t.Hour(), nil
} After: func Parse12To24h(hour12hFormat string) (int, error) {
try {
t := time.Parse("15PM", hour12hFormat)
} catch (err error) {
return -1, err
}
return t.Hour(), nil
} This just saves us time and lines of code, whenever we have to write multiple |
I dont like Here a few other ideas: // 1.
result, pass := someFunc()
result, pass fmt.Errorf("adding more context: %v", err) := someFunc()
// 2.
result := someFunc() pass
result := someFunc() pass fmt.Errorf("adding more context: %v", err) However, i think we have to think about 3 things when handling errors:
The pass solution wont handle the 3rd case (and depending of the chosen solution, not even the 2nd), and another solution would have to come in. And maybe go community would not like many different ways to handle errors. That's why i think a catch solution like this would be more appropriate, because it would give us a single, uniform way to handle errors in all those cases. |
I don't know if this is necessary, actually. If any extra code needs to be run, I think that it's fine to just use That being said, I do not like the Honestly, I'm leaning more and more towards some kind of limited macro system to deal with this. If a macro system could be sufficiently powerful to allow you to wrap an arbitrary call that returns func ContrivedExample(s1, s2 string) (int, error) {
macro toInt { ... } // Bikeshedding purposefully omitted.
return toInt(s1) + toInt(s2), nil
} And yes, I am aware that this actually does allow arbitrary extra code, despite my first paragraph up above. |
If you rename
You assume default zero values for all but the error. There are ways to simplify the proposal such that this is always straightforward. For example, removing the following clause
Which allows security consultants to easily view the code and understand that all values will be zero; EXCEPT the |
There is another variation that can be used for explicitness; I can create it once my keyboard is no longer broken. |
Otherwise, this |
This comment was marked as off-topic.
This comment was marked as off-topic.
Building on @DeedleFake 's idea, I think the key is how to limit the macro system. One benefit of going with the Implementation wise, perhaps Rob Pike argued in #19623 that arbitrary precision integers might be the reason for a Go 2.0. |
@fumin, the current mood of Go seems to be against macros. It might be a good solution in this case but the compiler should not become Turing complete, and macros have a high risk of being inadvertently Turing complete. No matter what the keyword, or perhaps, built in function is named, I think the most general use is a kind of return statement that returns only of all it's returned expressions are not nil. So return? or return if, seem like good names for that. As to the question if such a shorthand is worth while, I think it is because as @rcoreilly states, the frequency of use is the key here. My experience in Ruby which has similar expression ifs is that they are often used, very useful, and easy to read. |
I have to assume this has been proposed before but I did not see it. Why not take a lesson from For example; we often write code that passes the error up the stack: if x, err := foo(); err != nil {
return nil, err
}
// note x and err not available here if we allow return nil, err ; err != nil then you have a short form that short circuits and returns from the function. It doesn't change the semantics of the return expression to the left of the condition and defers etc work as before. You can mentally read this form of There is added potential to write expressions such as return x, err := foo() ; err != nil
// note x and err available here though this last form may be a step too far. zero argument returns continue to work, assuming the use of named arguments. return ; err != nil Net introduces no new symbols or keywords and is consistent with |
@mdcfrancis I am afraid that it will make the code much less unreadable. f, err := os.Open(filename)
if err != nil {
return ..., err
}
defer f.Close() would become that f, err := os.Open(filename)
return ..., err; err != nil
defer f.Close() Now imagine you would like to handle it in multiple cases. How are you going to distinguish between a "legit" return or just a simple error return when looking at the left edge of the code? |
To my eyes, scanning for the Under this proposal, the remaining Overall, I really like this latest proposal from @mdcfrancis! |
I created a new proposal that is the same as this one but adds a second parameter for error handling so that error handling can be first class. This proposal has the right foundation, but I believe it is a mistake to not optimize for error handling: it encourage Go programs to not annotate their errors. There is a lot of debate about what exact keyword/syntax to use on this proposal. I would prefer to have that discussion separately and focus on getting a proposal tentatively semantically accepted with the provision that the keyword can still be changed to whatever the community decides is optimal. |
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
What if we used |
Introduction
For the most part, I like the simplicity of error handling in Go, but I would very much like a less verbose way to pass errors.
Passing errors should still be explicit and simple enough to understand. This proposal builds on top of the many existing proposals and suggestions, and attempts to fix the problems in the proposed try built-in see design doc. Please read the original proposal if you haven't already some of the questions may already be answered there.
Would you consider yourself a novice, intermediate, or experienced Go programmer?
What other languages do you have experience with?
Would this change make Go easier or harder to learn, and why?
Who does this proposal help, and why?
Proposal
What is the proposed change?
Adding a new keyword
pass
.Pass statements may only be used inside a function with at least one result parameter where the last result is of type error.
Pass statements take an expression. The expression can be a simple error value. It can also be a function
or a method call that returns an error.
Calling
pass
with any other type or a different context will result in a compile-time error.If the result of the expression is
nil
,pass
will not perform any action, and execution continues.If the result of the expression
!= nil
,pass
will return from the function.pass
assumes their default "zero" values.pass
will return the value they already have.This works similar to the proposed
try
built-in.Is this change backward compatible?
Yes.
Show example code before and after the change.
A simple example:
before:
after:
In the example above, if the value of err is
nil
. it is equivalent to callingpass
in the following way (does not perform any action):Consequently, the following is expected to work with
pass
(passing a new error)Since
pass
accepts an expression that must be evaluated to anerror
. We can create handlers without any additional changes to the language.For example, the following
errors.Wrap()
function works withpass
.Because
Wrap
returnsnil
whenerr == nil
, you can use it to wrap errors:the following does not perform any action (because
Wrap
will returnnil
):You can define any function that takes an error to add logging, context, stack traces ... etc.
To use it
pass
is designed specifically for passing errors and nothing else.Other examples:
(Example updated to better reflect different usages of
pass
)Here's an example in practice. Code from
codehost.newCodeRepo()
(found by searching forerr != nil
- comments removed)This example shows when it's possible to use
pass
, and how it may look like in the real world.before:
after:
What is the cost of this proposal? (Every language change has a cost).
Because this is a new keyword, I'm not sure how much some of these tools would be affected,
but they shouldn't need any significant changes.
Compile time cost may be affected because the compiler may need to
perform additional optimizations to function or method calls used with
pass
.This depends on the implementation. Simple expressions like this:
should have equivalent runtime cost to the current
err != nil
.However, function or method calls will add run time cost and this will largely depend on the implementation.
Can you describe a possible implementation?
For the simple case, the compiler may be able to expand
pass
statementsto
For function or method calls there maybe a better way to do it
possibly inline these calls for common cases?
How would the language spec change?
The new keyword
pass
must be added to the language spec with a more formal definition.Orthogonality: how does this change interact or overlap with existing features?
In some cases,
pass
may overlap with areturn
statement.instead of
It may not be clear which keyword should be used here.
pass
is only useful in these cases when there is multiple result parameters.instead of
(Edited to make example more clear)
Is the goal of this change a performance improvement?
No
Does this affect error handling?
Yes, here is the following advantages of
pass
and how it may differ from other proposals.Advantages to using
pass
pass
is still explicit.handle
... etc.err == nil
witherr != nil
. Sincepass
only returns when error is notnil
in all cases.Is this about generics?
No
Edit: Updated to follow the template
The text was updated successfully, but these errors were encountered: