Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upproposal: Go 2: simplify error handling with || err suffix #21161
Comments
ianlancetaylor
added
Go2
LanguageChange
Proposal
labels
Jul 25, 2017
ianlancetaylor
added this to the Proposal milestone
Jul 25, 2017
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bradfitz
Jul 25, 2017
Member
syscall.Chdir(dir) || &PathError{"chdir", dir, e}
Where does e come from there? Typo?
Or did you mean:
func Chdir(dir string) (e error) {
syscall.Chdir(dir) || &PathError{"chdir", dir, e}
return nil
}
(That is, does the implicit err != nil check first assign error to the result parameter, which can be named to modify it again before the implicit return?)
Where does Or did you mean:
(That is, does the implicit err != nil check first assign error to the result parameter, which can be named to modify it again before the implicit return?) |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ianlancetaylor
Jul 25, 2017
Contributor
Sigh, messed up my own example. Now fixed: the e should be err. The proposal puts err in scope to hold the error value of the function call when not in an assignment statement.
|
Sigh, messed up my own example. Now fixed: the |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
mvdan
Jul 25, 2017
Member
While I'm not sure if I agree with the idea or the syntax, I have to give you credit for giving attention to adding context to errors before returning them.
This might be of interest to @davecheney, who wrote https://github.com/pkg/errors.
|
While I'm not sure if I agree with the idea or the syntax, I have to give you credit for giving attention to adding context to errors before returning them. This might be of interest to @davecheney, who wrote https://github.com/pkg/errors. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
object88
Jul 25, 2017
What happens in this code:
if foo, err := thing.Nope() || &PathError{"chdir", dir, err}; err == nil || ignoreError {
}
(My apologies if this isn't even possible w/o the || &PathError{"chdir", dir, e} part; I'm trying to express that this feels like a confusing override of existing behavior, and implicit returns are... sneaky?)
object88
commented
Jul 25, 2017
•
|
What happens in this code:
(My apologies if this isn't even possible w/o the |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ianlancetaylor
Jul 25, 2017
Contributor
@object88 I would be fine with not permitting this new case in a SimpleStmt as used in if and for and switch statements. That would probably be best though it would complicate the grammar slightly.
But if we don't do that, then what happens is that if thing.Nope() returns a non-nil error, then the calling function returns with &PathError{"chdir", dir, err} (where err is the variable set by the call to thing.Nope()). If thing.Nope() returns a nil error, then we know for sure that err == nil is true in the condition of the if statement, and so the body of the if statement is executed. The ignoreError variable is never read. There is no ambiguity or override of existing behavior here; the handling of || introduced here is only accepted when the expression after || is not a boolean value, which means that it would not compile currently.
I do agree that implicit returns are sneaky.
|
@object88 I would be fine with not permitting this new case in a SimpleStmt as used in But if we don't do that, then what happens is that if I do agree that implicit returns are sneaky. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
object88
Jul 25, 2017
Yeah, my example is pretty poor. But not permitting the operation inside an if, for, or switch would resolve a lot of potential confusion.
object88
commented
Jul 25, 2017
|
Yeah, my example is pretty poor. But not permitting the operation inside an |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jimmyfrasche
Jul 25, 2017
Member
Since the bar for consideration is generally something that is hard to do in the language as is, I decided to see how hard this variant was to encode in the language. Not much harder than the others: https://play.golang.org/p/9B3Sr7kj39
I really dislike all of these proposals for making one type of value and one position in the return arguments special. This one is in some ways actually worse because it also makes err a special name in this specific context.
Though I certainly agree that people (including me!) should be wearier of returning errors without extra context.
When there are other return values, like
if err != nil {
return 0, nil, "", Struct{}, wrap(err)
}
it can definitely get tiresome to read. I somewhat liked @nigeltao's suggestion for return ..., err in #19642 (comment)
|
Since the bar for consideration is generally something that is hard to do in the language as is, I decided to see how hard this variant was to encode in the language. Not much harder than the others: https://play.golang.org/p/9B3Sr7kj39 I really dislike all of these proposals for making one type of value and one position in the return arguments special. This one is in some ways actually worse because it also makes Though I certainly agree that people (including me!) should be wearier of returning errors without extra context. When there are other return values, like
it can definitely get tiresome to read. I somewhat liked @nigeltao's suggestion for |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
opennota
Jul 26, 2017
If I understand correctly, to build the syntax tree the parser would need to know the types of the variables to distinguish between
boolean := BoolFunc() || BoolExpr
and
err := FuncReturningError() || Expr
It doesn't look good.
opennota
commented
Jul 26, 2017
|
If I understand correctly, to build the syntax tree the parser would need to know the types of the variables to distinguish between
and
It doesn't look good. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
dup2X
commented
Jul 26, 2017
|
less is more... |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
mattn
Jul 26, 2017
Member
When the return ExpressionList contains two ore more elements, how it works?
BTW, I want panicIf instead.
err := doSomeThing()
panicIf(err)
err = doAnotherThing()
panicIf(err)|
When the return ExpressionList contains two ore more elements, how it works? BTW, I want panicIf instead. err := doSomeThing()
panicIf(err)
err = doAnotherThing()
panicIf(err) |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
tandr
Jul 26, 2017
@ianlancetaylor In your proposal's example err is still not declared explicitly, and pulled in as 'magical' (language predefined), right?
Or will it be something like
func Chdir(dir string) error {
return (err := syscall.Chdir(dir)) || &PathError{"chdir", dir, err}
}?
On the other hand (since it is marked as a "language change" already...)
Introduce a new operator (!! or ??) that does the shortcut on error != nil (or any nullable?)
func DirCh(dir string) (string, error) {
return dir, (err := syscall.Chdir(dir)) !! &PathError{"chdir", dir, err}
}Sorry if this is too far off :)
tandr
commented
Jul 26, 2017
|
@ianlancetaylor In your proposal's example Or will it be something like func Chdir(dir string) error {
return (err := syscall.Chdir(dir)) || &PathError{"chdir", dir, err}
}? On the other hand (since it is marked as a "language change" already...) func DirCh(dir string) (string, error) {
return dir, (err := syscall.Chdir(dir)) !! &PathError{"chdir", dir, err}
}Sorry if this is too far off :) |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
henryas
Jul 26, 2017
I agree that error handling in Go can be repetitive. I don't mind the repetition, but too many of them affects readability. There is a reason why "Cyclomatic Complexity" (whether you believe in it or not) uses control flows as a measure for complexity. The "if" statement adds extra noise.
However, the proposed syntax "||" is not very intuitive to read, especially since the symbol is commonly known as an OR operator. In addition, how do you deal with functions that return multiple values and error?
I am just tossing some ideas here. How about instead of using error as output, we use error as input? Example: https://play.golang.org/p/rtfoCIMGAb
henryas
commented
Jul 26, 2017
|
I agree that error handling in Go can be repetitive. I don't mind the repetition, but too many of them affects readability. There is a reason why "Cyclomatic Complexity" (whether you believe in it or not) uses control flows as a measure for complexity. The "if" statement adds extra noise. However, the proposed syntax "||" is not very intuitive to read, especially since the symbol is commonly known as an OR operator. In addition, how do you deal with functions that return multiple values and error? I am just tossing some ideas here. How about instead of using error as output, we use error as input? Example: https://play.golang.org/p/rtfoCIMGAb |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ianlancetaylor
Jul 26, 2017
Contributor
Thanks for all the comments.
@opennota Good point. It could still work but I agree that aspect is awkward.
@mattn I don't think there is a return ExpressionList, so I'm not sure what you are asking. If the calling function has multiple results, all but the last are returned as the zero value of the type.
@mattn panicif does not address one the key elements of this proposal, which is an easy way to return an error with additional context. And, of course, one can write panicif today easily enough.
@tandr Yes, err is defined magically, which is pretty awful. Another possibility would be to permit the error-expression to use error to refer to the error, which is awful in a different way.
@tandr We could use a different operator but I don't see any big advantage. It doesn't seem to make the result any more readable.
@henryas I think the proposal explains how it handles multiple results.
@henryas Thanks for the example. The thing I dislike about that kind of approach is that it makes the error handling the most prominent aspect of the code. I want the error handling to be present and visible but I don't want it to be the first thing on the line. That is true today, with the if err != nil idiom and the indentation of the error handling code, and it should remain true if any new features are added for error handling.
Thanks again.
|
Thanks for all the comments. @opennota Good point. It could still work but I agree that aspect is awkward. @mattn I don't think there is a return ExpressionList, so I'm not sure what you are asking. If the calling function has multiple results, all but the last are returned as the zero value of the type. @mattn @tandr Yes, @tandr We could use a different operator but I don't see any big advantage. It doesn't seem to make the result any more readable. @henryas I think the proposal explains how it handles multiple results. @henryas Thanks for the example. The thing I dislike about that kind of approach is that it makes the error handling the most prominent aspect of the code. I want the error handling to be present and visible but I don't want it to be the first thing on the line. That is true today, with the Thanks again. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jimmyfrasche
Jul 26, 2017
Member
@ianlancetaylor I don't know if you checked out my playground link but it had a "panicIf" that let you add extra context.
I'll reproduce a somewhat simplified version here:
func panicIf(err error, transforms ...func(error) error) {
if err == nil {
return
}
for _, transform := range transforms {
err = transform(err)
}
panic(err)
}
|
@ianlancetaylor I don't know if you checked out my playground link but it had a "panicIf" that let you add extra context. I'll reproduce a somewhat simplified version here:
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jba
Jul 26, 2017
Coincidentally, I just gave a lightning talk at GopherCon where I used (but didn't seriously propose) a bit of syntax to aid in visualizing error-handling code. The idea is to put that code to the side to get it out of the way of the main flow, without resorting to any magic tricks to shorten the code. The result looks like
func DirCh(dir string) (string, error) {
dir := syscall.Chdir(dir) =: err; if err != nil { return "", err }
}
where =: is the new bit of syntax, a mirror of := that assigns in the other direction. Obviously we'd also need something for = as well, which is admittedly problematic. But the general idea is to make it easier for the reader to understand the happy path, without losing information.
jba
commented
Jul 26, 2017
|
Coincidentally, I just gave a lightning talk at GopherCon where I used (but didn't seriously propose) a bit of syntax to aid in visualizing error-handling code. The idea is to put that code to the side to get it out of the way of the main flow, without resorting to any magic tricks to shorten the code. The result looks like
where |
jimmyfrasche
referenced this issue
Jul 26, 2017
Closed
proposal: Go 2: reduce noise in return statements that contain mostly zero values #21182
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
henryas
Jul 27, 2017
On the other hand, the current way of error handling does have some merits in that it serves as a glaring reminder that you may be doing too many things in a single function, and some refactoring may be overdue.
henryas
commented
Jul 27, 2017
|
On the other hand, the current way of error handling does have some merits in that it serves as a glaring reminder that you may be doing too many things in a single function, and some refactoring may be overdue. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
rodcorsi
Jul 27, 2017
I really like the the syntax proposed by @billyh here
func Chdir(dir string) error {
e := syscall.Chdir(dir) catch: &PathError{"chdir", dir, e}
return nil
}or more complex example using https://github.com/pkg/errors
func parse(input io.Reader) (*point, error) {
var p point
err := read(&p.Longitude) catch: nil, errors.Wrap(err, "Failed to read longitude")
err = read(&p.Latitude) catch: nil, errors.Wrap(err, "Failed to read Latitude")
err = read(&p.Distance) catch: nil, errors.Wrap(err, "Failed to read Distance")
err = read(&p.ElevationGain) catch: nil, errors.Wrap(err, "Failed to read ElevationGain")
err = read(&p.ElevationLoss) catch: nil, errors.Wrap(err, "Failed to read ElevationLoss")
return &p, nil
}
rodcorsi
commented
Jul 27, 2017
|
I really like the the syntax proposed by @billyh here func Chdir(dir string) error {
e := syscall.Chdir(dir) catch: &PathError{"chdir", dir, e}
return nil
}or more complex example using https://github.com/pkg/errors func parse(input io.Reader) (*point, error) {
var p point
err := read(&p.Longitude) catch: nil, errors.Wrap(err, "Failed to read longitude")
err = read(&p.Latitude) catch: nil, errors.Wrap(err, "Failed to read Latitude")
err = read(&p.Distance) catch: nil, errors.Wrap(err, "Failed to read Distance")
err = read(&p.ElevationGain) catch: nil, errors.Wrap(err, "Failed to read ElevationGain")
err = read(&p.ElevationLoss) catch: nil, errors.Wrap(err, "Failed to read ElevationLoss")
return &p, nil
} |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ALTree
Jul 27, 2017
Member
A plain idea, with support for error decoration, but requiring a more drastic language change (obviously not for go1.10) is the introduction of a new check keyword.
It would have two forms: check A and check A, B.
Both A and B need to be error. The second form would only be used when error-decorating; people that do not need or wish to decorate their errors will use the simpler form.
1st form (check A)
check A evaluates A. If nil, it does nothing. If not nil, check acts like a return {<zero>}*, A.
Examples
- If a function just returns an error, it can be used inline with
check, so
err := UpdateDB() // signature: func UpdateDb() error
if err != nil {
return err
}
becomes
check UpdateDB()
- For a function with multiple return values, you'll need to assign, like we do now.
a, b, err := Foo() // signature: func Foo() (string, string, error)
if err != nil {
return "", "", err
}
// use a and b
becomes
a, b, err := Foo()
check err
// use a and b
2nd form (check A, B)
check A, B evaluates A. If nil, it does nothing. If not nil, check acts like a return {<zero>}*, B.
This is for error-decorating needs. We still check on A, but is B that is used in the implicit return.
Example
a, err := Bar() // signature: func Bar() (string, error)
if err != nil {
return "", &BarError{"Bar", err}
}
becomes
a, err := Foo()
check err, &BarError{"Bar", err}
Notes
It's a compilation error to
- use the
checkstatement on things that do not evaluate toerror - use
checkin a function with return values not in the form{ type }*, error
The two-expr form check A, B is short-circuited. B is not evaluated if A is nil.
Notes on practicality
There's support for decorating errors, but you pay for the clunkier check A, B syntax only when you actually need to decorate errors.
For the if err != nil { return nil, nil, err } boilerplate (which is very common) check err is as brief as it could be without sacrificing clarity (see note on the syntax below).
Notes on syntax
I'd argue that this kind of syntax (check .., at the beginning of the line, similar to a return) is a good way to eliminate error checking boilerplate without hiding the control flow disruption that implicit returns introduce.
A downside of ideas like the <do-stuff> || <handle-err> and <do-stuff> catch <handle-err> above, or the a, b = foo()? proposed in another thread, is that they hide the control flow modification in a way that makes the flow harder to follow; the former with || <handle-err> machinery appended at the end of an otherwise plain-looking line, the latter with a little symbol that can appear everywhere, including in the middle and at the end of a plain-looking line of code, possibly multiple times.
A check statement will always be top-level in the current block, having the same prominence of other statements that modify the control flow (for example, an early return).
|
A plain idea, with support for error decoration, but requiring a more drastic language change (obviously not for go1.10) is the introduction of a new It would have two forms: Both 1st form (check A)
Examples
becomes
becomes
2nd form (check A, B)
This is for error-decorating needs. We still check on Example
becomes
NotesIt's a compilation error to
The two-expr form Notes on practicalityThere's support for decorating errors, but you pay for the clunkier For the Notes on syntaxI'd argue that this kind of syntax ( A downside of ideas like the A |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
billyh
Jul 27, 2017
@ALTree, I didn't understand how your example:
a, b, err := Foo()
check err
Achieves the three valued return from the original:
return "", "", err
Is it just returning zero values for all the declared returns except the final error? What about cases where you'd like to return a valid value along with an error, e.g. number of bytes written when a Write() fails?
Whatever solution we go with should minimally restrict the generality of error handling.
Regarding the value of having check at the start of the line, my personal preference is to see the primary control flow at the start of each line and have the error handling interfere with the readability of that primary control flow as little as possible. Also, if the error handling is set apart by a reserved word like check or catch, then pretty much any modern editor is going to highlight the reserved word syntax in some way and make it noticeable even if it's on the right hand side.
billyh
commented
Jul 27, 2017
•
|
@ALTree, I didn't understand how your example:
Achieves the three valued return from the original:
Is it just returning zero values for all the declared returns except the final error? What about cases where you'd like to return a valid value along with an error, e.g. number of bytes written when a Write() fails? Whatever solution we go with should minimally restrict the generality of error handling. Regarding the value of having |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ALTree
Jul 27, 2017
Member
@billyh this is explained above, on the line that says:
If not nil, check acts like a
return {<zero>}*, A
check will return the zero value of any return value, except the error (in last position).
What about cases where you'd like to return a valid value along with an error
Then you'll use the if err != nil { idiom.
There are many cases where you'll need a more sophisticate error-recovering procedure. For example you may need, after catching an error, to roll back something, or to write something on a log file. In all these cases, you'll still have the usual if err idiom in your toolbox, and you can use it to start a new block, where any kind of error-handling related operation, no matter how articulated, can be carried out.
Whatever solution we go with should minimally restrict the generality of error handling.
See my answer above. You'll still have if and anything else the language gives you now.
pretty much any modern editor is going to highlight the reserved word
Maybe. But introducing opaque syntax, that requires syntax highlighting to be readable, is not ideal.
|
@billyh this is explained above, on the line that says:
Then you'll use the There are many cases where you'll need a more sophisticate error-recovering procedure. For example you may need, after catching an error, to roll back something, or to write something on a log file. In all these cases, you'll still have the usual
See my answer above. You'll still have
Maybe. But introducing opaque syntax, that requires syntax highlighting to be readable, is not ideal. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
go-li
Jul 28, 2017
this particular bug can be fixed by introducing a double return facility to the language.
in this case the function a() returns 123:
func a() int {
b()
return 456
}
func b() {
return return int(123)
}
This facility can be used to simplify error handling as follows:
func handle(var *foo, err error )(var *foo, err error ) {
if err != nil {
return return nil, err
}
return var, nil
}func client_code() (*client_object, error) {
var obj, err =handle(something_that_can_fail())
// this is only reached if something not failed
// otherwise the client_code function would propagate the error up the stack
assert(err == nil)
}
This allows people to write an error handler functions that can propagate the errors up the stack
such error handling functions can be separate from main code
go-li
commented
Jul 28, 2017
|
this particular bug can be fixed by introducing a double return facility to the language.
This facility can be used to simplify error handling as follows:
This allows people to write an error handler functions that can propagate the errors up the stack |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
rodcorsi
Jul 28, 2017
Sorry if I got it wrong, but I want to clarify a point, the function below will produce an error, vet warning or will it be accepted?
func Chdir(dir string) (err error) {
syscall.Chdir(dir) || err
return nil
}
rodcorsi
commented
Jul 28, 2017
•
|
Sorry if I got it wrong, but I want to clarify a point, the function below will produce an error, func Chdir(dir string) (err error) {
syscall.Chdir(dir) || err
return nil
} |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ianlancetaylor
Jul 29, 2017
Contributor
@rodcorsi Under this proposal your example would be accepted with no vet warning. It would be equivalent to
if err := syscall.Chdir(dir); err != nil {
return err
}
|
@rodcorsi Under this proposal your example would be accepted with no vet warning. It would be equivalent to
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
henryas
Jul 29, 2017
How about expanding the use of Context to handle error? For instance, given the following definition:
type ErrorContext interface {
HasError() bool
SetError(msg string)
Error() string
}
Now in the error-prone function ...
func MyFunction(number int, ctx ErrorContext) int {
if ctx.HasError() {
return 0
}
return number + 1
}
In the intermediate function ...
func MyIntermediateFunction(ctx ErrorContext) int {
if ctx.HasError() {
return 0
}
number := 0
number = MyFunction(number, ctx)
number = MyFunction(number, ctx)
number = MyFunction(number, ctx)
return number
}
And in the upper level function
func main() {
ctx := context.New()
no := MyIntermediateFunction(ctx)
if ctx.HasError() {
log.Fatalf("Error: %s", ctx.Error())
return
}
fmt.Printf("%d\n", no)
}
There are several benefits using this approach. First, it doesn't distract the reader from the main execution path. There is minimal "if" statements to indicate deviation from the main execution path.
Second, it doesn't hide error. It is clear from the method signature that if it accepts ErrorContext, then the function may have errors. Inside the function, it uses the normal branching statements (eg. "if") which shows how error is handled using normal Go code.
Third, error is automatically bubbled up to the interested party, which in this case is the context owner. Should there be an additional error processing, it will be clearly shown. For instance, let's make some changes to the intermediate function to wrap any existing error:
func MyIntermediateFunction(ctx ErrorContext) int {
if ctx.HasError() {
return 0
}
number := 0
number = MyFunction(number, ctx)
number = MyFunction(number, ctx)
number = MyFunction(number, ctx)
if ctx.HasError() {
ctx.SetError(fmt.Sprintf("wrap msg: %s", ctx.Error())
return
}
number *= 20
number = MyFunction(number, ctx)
return number
}
Basically, you just write the error-handling code as needed. You don't need to manually bubble them up.
Lastly, you as the function writer have a say whether the error should be handled. Using the current Go approach, it is easy to do this ...
//given the following definition
func MyFunction(number int) error
//then do this
MyFunction(8) //without checking the error
With the ErrorContext, you as the function owner can make the error checking optional with this:
func MyFunction(ctx ErrorContext) {
if ctx != nil && ctx.HasError() {
return
}
//...
}
Or make it compulsory with this:
func MyFunction(ctx ErrorContext) {
if ctx.HasError() { //will panic if ctx is nil
return
}
//...
}
If you make error handling compulsory and yet the user insists on ignoring error, they can still do that. However, they have to be very explicit about it (to prevent accidental ignore). For instance:
func UpperFunction(ctx ErrorContext) {
ignored := context.New()
MyFunction(ignored) //this one is ignored
MyFunction(ctx) //this one is handled
}
This approach changes nothing to the existing language.
henryas
commented
Jul 29, 2017
|
How about expanding the use of Context to handle error? For instance, given the following definition:
Now in the error-prone function ...
In the intermediate function ...
And in the upper level function
There are several benefits using this approach. First, it doesn't distract the reader from the main execution path. There is minimal "if" statements to indicate deviation from the main execution path. Second, it doesn't hide error. It is clear from the method signature that if it accepts ErrorContext, then the function may have errors. Inside the function, it uses the normal branching statements (eg. "if") which shows how error is handled using normal Go code. Third, error is automatically bubbled up to the interested party, which in this case is the context owner. Should there be an additional error processing, it will be clearly shown. For instance, let's make some changes to the intermediate function to wrap any existing error:
Basically, you just write the error-handling code as needed. You don't need to manually bubble them up. Lastly, you as the function writer have a say whether the error should be handled. Using the current Go approach, it is easy to do this ...
With the ErrorContext, you as the function owner can make the error checking optional with this:
Or make it compulsory with this:
If you make error handling compulsory and yet the user insists on ignoring error, they can still do that. However, they have to be very explicit about it (to prevent accidental ignore). For instance:
This approach changes nothing to the existing language. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
tandr
Jul 30, 2017
@ALTree Alberto, what about mixing your check and what @ianlancetaylor proposed?
so
func F() (int, string, error) {
i, s, err := OhNo()
if err != nil {
return i, s, &BadStuffHappened(err, "oopsie-daisy")
}
// all is good
return i+1, s+" ok", nil
}
becomes
func F() (int, string, error) {
i, s, err := OhNo()
check i, s, err || &BadStuffHappened(err, "oopsie-daisy")
// all is good
return i+1, s+" ok", nil
}
Also, we can limit check to only deal with error types, so if you need multiple return values, they need to be named and assigned, so it assigns "in-place" somehow and behaves like simple "return"
func F() (a int, s string, err error) {
i, s, err = OhNo()
check err |= &BadStuffHappened(err, "oopsy-daisy") // assigns in place and behaves like simple "return"
// all is good
return i+1, s+" ok", nil
}
If return would become acceptable in expression one day, then check is not needed, or becomes a standard function
func check(e error) bool {
return e != nil
}
func F() (a int, s string, err error) {
i, s, err = OhNo()
check(err) || return &BadStuffHappened(err, "oopsy-daisy")
// all is good
return i+1, s+" ok", nil
}
the last solution feels like Perl though
tandr
commented
Jul 30, 2017
|
@ALTree Alberto, what about mixing your so
becomes
Also, we can limit
If
the last solution feels like Perl though |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Azareal
Jul 5, 2018
That example would probably be easier to follow with a deferifnotnil or something.
But that kinda goes back to the whole one line if thing which many of these suggestions are about.
Azareal
commented
Jul 5, 2018
|
That example would probably be easier to follow with a |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
carlmjohnson
Jul 5, 2018
Contributor
Having played with the example code a little, I'm now against the try(label) name variant. I think if you have multiple fancy things to do, just use the current system of if err != nil { ... }. If you're doing basically the same thing, such as setting a custom error message, you could do this:
func WriteFooBar(w io.Writer) (err error) {
msg := "thing1 went wrong"
_, try err = io.WriteString(w, "foo")
msg = "thing2 went wrong"
_, try err = w.Write([]byte{','})
msg = "thing3 went wrong"
_, try err = io.WriteString(w, "bar")
return nil
catch:
return &CustomError{msg, err}
}If anyone has used Ruby, this looks a lot like their rescue syntax, which I think reads reasonably well.
|
Having played with the example code a little, I'm now against the func WriteFooBar(w io.Writer) (err error) {
msg := "thing1 went wrong"
_, try err = io.WriteString(w, "foo")
msg = "thing2 went wrong"
_, try err = w.Write([]byte{','})
msg = "thing3 went wrong"
_, try err = io.WriteString(w, "bar")
return nil
catch:
return &CustomError{msg, err}
}If anyone has used Ruby, this looks a lot like their |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Azareal
Jul 5, 2018
One thing which could be done is to make nil a falsy value and for other values to evaluate to true, so you wind up with:
err := doSomething()
if err { return err }But I'm not sure that would really work and it only shaves off a few characters.
I've typoed many things, but I don't think I've ever typoed != nil.
Azareal
commented
Jul 5, 2018
•
|
One thing which could be done is to make err := doSomething()
if err { return err }But I'm not sure that would really work and it only shaves off a few characters. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
carlmjohnson
Jul 5, 2018
Contributor
Making interfaces truthy/falsey has been mentioned before, and I said that it would make bugs with flags more common:
verbose := flag.Bool("v", false, "verbose logging")
flag.Parse()
if verbose { ... } // should be *verbose!|
Making interfaces truthy/falsey has been mentioned before, and I said that it would make bugs with flags more common: verbose := flag.Bool("v", false, "verbose logging")
flag.Parse()
if verbose { ... } // should be *verbose! |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
object88
Jul 5, 2018
@carlmjohnson , in the example you provided just above, there are error messages interspersed with happy-path code, which is a little strange to me. If you need to format those strings, then you're doing a lot of extra work regardless of whether anything goes wrong, or not:
func (f *foo) WriteFooBar(w io.Writer) (err error) {
msg := fmt.Sprintf("While writing %s, thing1 went wrong", f.foo)
_, try err = io.WriteString(w, f.foo)
msg = fmt.Sprintf("While writing %s, thing2 went wrong", f.separator)
_, try err = w.Write(f.separator)
msg = fmt.Sprintf("While writing %s, thing3 went wrong", f.bar)
_, try err = io.WriteString(w, f.bar)
return nil
catch:
return &CustomError{msg, err}
}
object88
commented
Jul 5, 2018
|
@carlmjohnson , in the example you provided just above, there are error messages interspersed with happy-path code, which is a little strange to me. If you need to format those strings, then you're doing a lot of extra work regardless of whether anything goes wrong, or not: func (f *foo) WriteFooBar(w io.Writer) (err error) {
msg := fmt.Sprintf("While writing %s, thing1 went wrong", f.foo)
_, try err = io.WriteString(w, f.foo)
msg = fmt.Sprintf("While writing %s, thing2 went wrong", f.separator)
_, try err = w.Write(f.separator)
msg = fmt.Sprintf("While writing %s, thing3 went wrong", f.bar)
_, try err = io.WriteString(w, f.bar)
return nil
catch:
return &CustomError{msg, err}
} |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
carlmjohnson
Jul 5, 2018
Contributor
@object88 , I think that the SSA analysis should be able to figure out if certain assignments aren't used and rearrange them to not happen if not needed (too optimistic?). If that's true, this should be efficient:
func (f *foo) WriteFooBar(w io.Writer) (err error) {
var format string, args []interface{}
msg = "While writing %s, thing1 went wrong",
args = []interface{f.foo}
_, try err = io.WriteString(w, f.foo)
format = "While writing %s, thing2 went wrong"
args = []interface{f.separator}
_, try err = w.Write(f.separator)
format = "While writing %s, thing3 went wrong"
args = []interface{f.bar}
_, try err = io.WriteString(w, f.bar)
return nil
catch:
msg := fmt.Sprintf(format, args...)
return &CustomError{msg, err}
}|
@object88 , I think that the SSA analysis should be able to figure out if certain assignments aren't used and rearrange them to not happen if not needed (too optimistic?). If that's true, this should be efficient: func (f *foo) WriteFooBar(w io.Writer) (err error) {
var format string, args []interface{}
msg = "While writing %s, thing1 went wrong",
args = []interface{f.foo}
_, try err = io.WriteString(w, f.foo)
format = "While writing %s, thing2 went wrong"
args = []interface{f.separator}
_, try err = w.Write(f.separator)
format = "While writing %s, thing3 went wrong"
args = []interface{f.bar}
_, try err = io.WriteString(w, f.bar)
return nil
catch:
msg := fmt.Sprintf(format, args...)
return &CustomError{msg, err}
} |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
carlmjohnson
Jul 5, 2018
Contributor
Would this be legal?
func Foo() error {
catch:
try _ = doThing()
return nil
}I think that should loop until doThing() returns nil, but I could be convinced otherwise.
|
Would this be legal? func Foo() error {
catch:
try _ = doThing()
return nil
}I think that should loop until |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
lukescott
Jul 5, 2018
Having played with the example code a little, I'm now against the try(label) name variant.
Yeah, I wasn't sure about the syntax. I don't like it because it makes try look like a function call. But I could see the value of specifying a different label.
Would this be legal?
I would say yes because try should be forward-only. If you wanted to do that, I would say you need to do it like this:
func Foo() error {
tryAgain:
if err := doThing(); err != nil {
goto tryAgain
}
return nil
}Or like this:
func Foo() error {
for doThing() != nil {}
return nil
}
lukescott
commented
Jul 5, 2018
Yeah, I wasn't sure about the syntax. I don't like it because it makes try look like a function call. But I could see the value of specifying a different label.
I would say yes because try should be forward-only. If you wanted to do that, I would say you need to do it like this: func Foo() error {
tryAgain:
if err := doThing(); err != nil {
goto tryAgain
}
return nil
}Or like this: func Foo() error {
for doThing() != nil {}
return nil
} |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
lukescott
Jul 5, 2018
One thing which could be done is to make nil a falsy value and for other values to evaluate to true, so you wind up with:
err := doSomething() if err { return err }
I think there is value in shortening it. However, I don't think it should apply to nil in all situations. Perhaps there could be a new interface like this:
interface Truthy {
True() bool
}Then any value that implements this interface can be used as you proposed.
This would work as long as the error implemented the interface:
err := doSomething()
if err { return err }But this would not work:
err := doSomething()
if err == true { return err } // err is not true
lukescott
commented
Jul 5, 2018
•
I think there is value in shortening it. However, I don't think it should apply to nil in all situations. Perhaps there could be a new interface like this: interface Truthy {
True() bool
}Then any value that implements this interface can be used as you proposed. This would work as long as the error implemented the interface: err := doSomething()
if err { return err }But this would not work: err := doSomething()
if err == true { return err } // err is not true |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
msr1k
Jul 23, 2018
I'm really new to golang but how do you think about introducing conditional delegator like below?
func someFunc() error {
errorHandler := delegator(arg1 Arg1, err error) error if err != nil {
// ...
return err // or modifiedErr
}
ret, err := doSomething()
delegate errorHandler(ret, err)
ret, err := doAnotherThing()
delegate errorHandler(ret, err)
return nil
}delegator is function like stuff but
- Its
returnmeansreturn from its caller context. (return type must be same as caller) - It takes optionally
ifbefore{, in above example it isif err != nil. - It must be delegated by the caller with
delegatekeyword
It might be able to omit delegate to delegate, but I think it makes hard to read flow of the function.
And perhaps it is useful not only for error handling, I'm not sure now though.
msr1k
commented
Jul 23, 2018
•
|
I'm really new to golang but how do you think about introducing conditional delegator like below? func someFunc() error {
errorHandler := delegator(arg1 Arg1, err error) error if err != nil {
// ...
return err // or modifiedErr
}
ret, err := doSomething()
delegate errorHandler(ret, err)
ret, err := doAnotherThing()
delegate errorHandler(ret, err)
return nil
}delegator is function like stuff but
It might be able to omit And perhaps it is useful not only for error handling, I'm not sure now though. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
vcaesar
Aug 14, 2018
It's beautiful to add check, but you can do further before returning:
result, err := openFile(f);
if err != nil {
log.Println(..., err)
return 0, err
}becomes
result, err := openFile(f);
check errresult, err := openFile(f);
check err {
log.Println(..., err)
}reslt, _ := check openFile(f)
// If err is not nil direct return, does not execute the next step.result, err := check openFile(f) {
log.Println(..., err)
}It also attempts simplifying the error handling (#26712):
result, err := openFile(f);
check !err {
// err is an interface with value nil or holds a nil pointer
// it is unusable
result.C...()
}It also attempts simplifying the (by some considered tedious) error handling (#21161). It would become:
result, err := openFile(f);
check err {
// handle error and return
log.Println(..., err)
}Of course, you can use a try and other keywords instead of the check , if it's clearer.
reslt, _ := try openFile(f)
// If err is not nil direct return, does not execute the next step.result, err := openFile(f);
try err {
// handle error and return
log.Println(..., err)
}Reference:
A plain idea, with support for error decoration, but requiring a more drastic language change (obviously not for go1.10) is the introduction of a new check keyword.
It would have two forms: check A and check A, B.
Both A and B need to be error. The second form would only be used when error-decorating; people that do not need or wish to decorate their errors will use the simpler form.
1st form (check A)
check A evaluates A. If nil, it does nothing. If not nil, check acts like a return {}*, A.
Examples
If a function just returns an error, it can be used inline with check, so
err := UpdateDB() // signature: func UpdateDb() error
if err != nil {
return err
}becomes
check UpdateDB()For a function with multiple return values, you'll need to assign, like we do now.
a, b, err := Foo() // signature: func Foo() (string, string, error)
if err != nil {
return "", "", err
}
// use a and bbecomes
a, b, err := Foo()
check err
// use a and b2nd form (check A, B)
check A, B evaluates A. If nil, it does nothing. If not nil, check acts like a return {}*, B.
This is for error-decorating needs. We still check on A, but is B that is used in the implicit return.
Example
a, err := Bar() // signature: func Bar() (string, error)
if err != nil {
return "", &BarError{"Bar", err}
}becomes
a, err := Foo()
check err, &BarError{"Bar", err}Notes
It's a compilation error to
use the check statement on things that do not evaluate to error
use check in a function with return values not in the form { type }*, error
The two-expr form check A, B is short-circuited. B is not evaluated if A is nil.
Notes on practicality
There's support for decorating errors, but you pay for the clunkier check A, B syntax only when you actually need to decorate errors.
For the if err != nil { return nil, nil, err } boilerplate (which is very common) check err is as brief as it could be without sacrificing clarity (see note on the syntax below).
Notes on syntax
I'd argue that this kind of syntax (check .., at the beginning of the line, similar to a return) is a good way to eliminate error checking boilerplate without hiding the control flow disruption that implicit returns introduce.
A downside of ideas like the || and catch above, or the a, b = foo()? proposed in another thread, is that they hide the control flow modification in a way that makes the flow harder to follow; the former with || machinery appended at the end of an otherwise plain-looking line, the latter with a little symbol that can appear everywhere, including in the middle and at the end of a plain-looking line of code, possibly multiple times.
A check statement will always be top-level in the current block, having the same prominence of other statements that modify the control flow (for example, an early return).
vcaesar
commented
Aug 14, 2018
•
|
It's beautiful to add check, but you can do further before returning: result, err := openFile(f);
if err != nil {
log.Println(..., err)
return 0, err
}becomes result, err := openFile(f);
check errresult, err := openFile(f);
check err {
log.Println(..., err)
}reslt, _ := check openFile(f)
// If err is not nil direct return, does not execute the next step.result, err := check openFile(f) {
log.Println(..., err)
}It also attempts simplifying the error handling (#26712): result, err := openFile(f);
check !err {
// err is an interface with value nil or holds a nil pointer
// it is unusable
result.C...()
}It also attempts simplifying the (by some considered tedious) error handling (#21161). It would become: result, err := openFile(f);
check err {
// handle error and return
log.Println(..., err)
}Of course, you can use a reslt, _ := try openFile(f)
// If err is not nil direct return, does not execute the next step.result, err := openFile(f);
try err {
// handle error and return
log.Println(..., err)
}Reference: A plain idea, with support for error decoration, but requiring a more drastic language change (obviously not for go1.10) is the introduction of a new check keyword. It would have two forms: check A and check A, B. Both A and B need to be error. The second form would only be used when error-decorating; people that do not need or wish to decorate their errors will use the simpler form. 1st form (check A) Examples If a function just returns an error, it can be used inline with check, so err := UpdateDB() // signature: func UpdateDb() error
if err != nil {
return err
}becomes check UpdateDB()For a function with multiple return values, you'll need to assign, like we do now. a, b, err := Foo() // signature: func Foo() (string, string, error)
if err != nil {
return "", "", err
}
// use a and bbecomes a, b, err := Foo()
check err
// use a and b2nd form (check A, B) This is for error-decorating needs. We still check on A, but is B that is used in the implicit return. Example a, err := Bar() // signature: func Bar() (string, error)
if err != nil {
return "", &BarError{"Bar", err}
}becomes a, err := Foo()
check err, &BarError{"Bar", err}Notes use the check statement on things that do not evaluate to error Notes on practicality For the Notes on syntax A downside of ideas like the || and catch above, or the a, b = foo()? proposed in another thread, is that they hide the control flow modification in a way that makes the flow harder to follow; the former with || machinery appended at the end of an otherwise plain-looking line, the latter with a little symbol that can appear everywhere, including in the middle and at the end of a plain-looking line of code, possibly multiple times. A check statement will always be top-level in the current block, having the same prominence of other statements that modify the control flow (for example, an early return). |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
rogpeppe
Aug 14, 2018
Contributor
Here's another thought.
Imagine an again statement which defines a macro with a label. The statement statement it labels can be expanded again by textual substitution later in the function (reminiscent of const/iota, with shades of goto :-] ).
For example:
func(foo int) (int, error) {
err := f(foo)
again check:
if err != nil {
return 0, errors.Wrap(err)
}
err = g(foo)
check
x, err := h()
check
return x, nil
}
would be exactly equivalent to:
func(foo int) (int, error) {
err := f(foo)
if err != nil {
return 0, errors.Wrap(err)
}
err = g(foo)
if err != nil {
return 0, errors.Wrap(err)
}
x, err := h()
if err != nil {
return 0, errors.Wrap(err)
}
return x, nil
}
Note that the macro expansion has no arguments - this means that there should be less confusion about the fact that it is a macro, because the compiler doesn't like symbols on their own.
Like the goto statement, the scope of the label is within the current function.
|
Here's another thought. Imagine an For example:
would be exactly equivalent to:
Note that the macro expansion has no arguments - this means that there should be less confusion about the fact that it is a macro, because the compiler doesn't like symbols on their own. Like the goto statement, the scope of the label is within the current function. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
carlmjohnson
Aug 15, 2018
Contributor
Interesting idea. I liked the catch label idea, but I don't think it was a good fit with Go scopes (with current Go, you can't goto a label with new variables defined in its scope). The again idea fixes that problem because the label comes before new scopes are introduced.
|
Interesting idea. I liked the catch label idea, but I don't think it was a good fit with Go scopes (with current Go, you can't |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
carlmjohnson
Aug 15, 2018
Contributor
Here is the mega-example again:
func NewClient(...) (*Client, error) {
var (
err error
listener net.Listener
conn net.Conn
)
catch {
if conn != nil {
conn.Close()
}
if listener != nil {
listener.Close()
}
return nil, err
}
listener, try err = net.Listen("tcp4", listenAddr)
conn, try err = ConnectionManager{}.connect(server, tlsConfig)
if forwardPort == 0 {
env, err := environment.GetRuntimeEnvironment()
if err != nil {
log.Printf("not forwarding because: %v", err)
} else {
forwardPort, err = env.PortToForward()
if err != nil {
log.Printf("env couldn't provide forward port: %v", err)
}
}
}
var forwardOut *forwarding.ForwardOut
if forwardPort != 0 {
u, _ := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", forwardPort))
forwardOut = forwarding.NewOut(u)
}
client := &Client{...}
toServer := communicationProtocol.Wrap(conn)
try err = toServer.Send(&client.serverConfig)
try err = toServer.Send(&stprotocol.ClientProtocolAck{ClientVersion: Version})
session, try err := communicationProtocol.FinalProtocol(conn)
client.session = session
return client, nil
}|
Here is the mega-example again: func NewClient(...) (*Client, error) {
var (
err error
listener net.Listener
conn net.Conn
)
catch {
if conn != nil {
conn.Close()
}
if listener != nil {
listener.Close()
}
return nil, err
}
listener, try err = net.Listen("tcp4", listenAddr)
conn, try err = ConnectionManager{}.connect(server, tlsConfig)
if forwardPort == 0 {
env, err := environment.GetRuntimeEnvironment()
if err != nil {
log.Printf("not forwarding because: %v", err)
} else {
forwardPort, err = env.PortToForward()
if err != nil {
log.Printf("env couldn't provide forward port: %v", err)
}
}
}
var forwardOut *forwarding.ForwardOut
if forwardPort != 0 {
u, _ := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", forwardPort))
forwardOut = forwarding.NewOut(u)
}
client := &Client{...}
toServer := communicationProtocol.Wrap(conn)
try err = toServer.Send(&client.serverConfig)
try err = toServer.Send(&stprotocol.ClientProtocolAck{ClientVersion: Version})
session, try err := communicationProtocol.FinalProtocol(conn)
client.session = session
return client, nil
} |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
carlmjohnson
Aug 15, 2018
Contributor
Here is a version closer to Rog's proposal (I don't like it as much):
func NewClient(...) (*Client, error) {
var (
err error
listener net.Listener
conn net.Conn
)
again:
if err != nil {
if conn != nil {
conn.Close()
}
if listener != nil {
listener.Close()
}
return nil, err
}
listener, err = net.Listen("tcp4", listenAddr)
check
conn, err = ConnectionManager{}.connect(server, tlsConfig)
check
if forwardPort == 0 {
env, err := environment.GetRuntimeEnvironment()
if err != nil {
log.Printf("not forwarding because: %v", err)
} else {
forwardPort, err = env.PortToForward()
if err != nil {
log.Printf("env couldn't provide forward port: %v", err)
}
}
}
var forwardOut *forwarding.ForwardOut
if forwardPort != 0 {
u, _ := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", forwardPort))
forwardOut = forwarding.NewOut(u)
}
client := &Client{...}
toServer := communicationProtocol.Wrap(conn)
err = toServer.Send(&client.serverConfig)
check
err = toServer.Send(&stprotocol.ClientProtocolAck{ClientVersion: Version})
check
session, err := communicationProtocol.FinalProtocol(conn)
check
client.session = session
return client, nil
}|
Here is a version closer to Rog's proposal (I don't like it as much): func NewClient(...) (*Client, error) {
var (
err error
listener net.Listener
conn net.Conn
)
again:
if err != nil {
if conn != nil {
conn.Close()
}
if listener != nil {
listener.Close()
}
return nil, err
}
listener, err = net.Listen("tcp4", listenAddr)
check
conn, err = ConnectionManager{}.connect(server, tlsConfig)
check
if forwardPort == 0 {
env, err := environment.GetRuntimeEnvironment()
if err != nil {
log.Printf("not forwarding because: %v", err)
} else {
forwardPort, err = env.PortToForward()
if err != nil {
log.Printf("env couldn't provide forward port: %v", err)
}
}
}
var forwardOut *forwarding.ForwardOut
if forwardPort != 0 {
u, _ := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", forwardPort))
forwardOut = forwarding.NewOut(u)
}
client := &Client{...}
toServer := communicationProtocol.Wrap(conn)
err = toServer.Send(&client.serverConfig)
check
err = toServer.Send(&stprotocol.ClientProtocolAck{ClientVersion: Version})
check
session, err := communicationProtocol.FinalProtocol(conn)
check
client.session = session
return client, nil
} |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
rogpeppe
Aug 16, 2018
Contributor
@carlmjohnson For the record, that's not quite what I'm suggesting. The "check" identifier isn't special - it needs to be declared by putting it after the "again" keyword.
Also, I'd suggest that the above example doesn't illustrate its use very well - there's nothing in the again-labelled statement above that couldn't just as well be done in a defer statement. In the try/catch example, that code cannot (for example) wrap the error with information about the source location of the error return. It also won't work AFAICS if you add a "try" inside one of those if statements (for example to check the error returned by GetRuntimeEnvironment), because the "err" referred to by the catch statement is in a different scope from that declared inside the block.
|
@carlmjohnson For the record, that's not quite what I'm suggesting. The "check" identifier isn't special - it needs to be declared by putting it after the "again" keyword. Also, I'd suggest that the above example doesn't illustrate its use very well - there's nothing in the again-labelled statement above that couldn't just as well be done in a defer statement. In the try/catch example, that code cannot (for example) wrap the error with information about the source location of the error return. It also won't work AFAICS if you add a "try" inside one of those if statements (for example to check the error returned by GetRuntimeEnvironment), because the "err" referred to by the catch statement is in a different scope from that declared inside the block. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
deanveloper
Aug 23, 2018
I think my only issue with a check keyword is that all exits to a function should be a return (or at least have some kind of "I'm gonna leave the function" connotation). We might get become (for TCO), at least become has some kind of "We're becoming a different function"... but the word "check" really doesn't sound like it's going to be an exit for the function.
The exit point of a function is extremely important, and I'm not sure if check really has that "exit point" feel. Other than that, I really like the idea of what check does, it allows for much more compact error handling, but still allows to handle each error differently, or to wrap the error how you wish.
deanveloper
commented
Aug 23, 2018
|
I think my only issue with a The exit point of a function is extremely important, and I'm not sure if |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
cthackers
Aug 27, 2018
Can I add a suggestion also?
What about something like this:
func Open(filename string) os.File onerror (string, error) {
f, e := os.Open(filename)
if e != nil {
fail "some reason", e // instead of return keyword to go on the onerror
}
return f
}
f := Open(somefile) onerror reason, e {
log.Prinln(reason)
// try to recover from error and reasign 'f' on success
nf = os.Create(somefile) onerror err {
panic(err)
}
return nf // return here must return whatever Open returns
}The error assignment can have any form, even be something stupid like
f := Open(name) =: e
Or return a different set of values in case of error not just errors, And also a try catch block would be nice.
try {
f := Open("file1") // can fail here
defer f.Close()
f1 := Open("file2") // can fail here
defer f1.Close()
// do something with the files
} onerror err {
log.Println(err)
}
cthackers
commented
Aug 27, 2018
|
Can I add a suggestion also? func Open(filename string) os.File onerror (string, error) {
f, e := os.Open(filename)
if e != nil {
fail "some reason", e // instead of return keyword to go on the onerror
}
return f
}
f := Open(somefile) onerror reason, e {
log.Prinln(reason)
// try to recover from error and reasign 'f' on success
nf = os.Create(somefile) onerror err {
panic(err)
}
return nf // return here must return whatever Open returns
}The error assignment can have any form, even be something stupid like
Or return a different set of values in case of error not just errors, And also a try catch block would be nice. try {
f := Open("file1") // can fail here
defer f.Close()
f1 := Open("file2") // can fail here
defer f1.Close()
// do something with the files
} onerror err {
log.Println(err)
} |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
deanveloper
Aug 27, 2018
@cthackers I personally believe that it's very nice for errors in Go to not have special treatment. They are simply values, and I think they should stay that way.
Also, try-catch (and similar constructs) is just all around a bad construct that encourages bad practices. Every error should be handled separately, not handled by some "catch all" error handler.
deanveloper
commented
Aug 27, 2018
|
@cthackers I personally believe that it's very nice for errors in Go to not have special treatment. They are simply values, and I think they should stay that way. Also, try-catch (and similar constructs) is just all around a bad construct that encourages bad practices. Every error should be handled separately, not handled by some "catch all" error handler. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
chen56
Aug 29, 2018
https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling-overview.md
this is too complicated.
my idea: |err| means check error : if err!=nil {}
// common util func
func parseInt(s string) (i int64, err error){
return strconv.ParseInt(s, 10, 64)
}
// expression check err 1 : check and use err variable
func parseAndSum(a string ,b string) (int64,error) {
sum := parseInt(a) + parseInt(b) |err| return 0,err
return sum,nil
}
// expression check err 2 : unuse variable
func parseAndSum(a string , b string) (int64,error) {
a,err := parseInt(a) |_| return 0, fmt.Errorf("parseInt error: %s", a)
b,err := parseInt(b) |_| { println(b); return 0,fmt.Errorf("parseInt error: %s", b);}
return a+b,nil
}
// block check err
func parseAndSum(a string , b string) ( int64, error) {
{
a := parseInt(a)
b := parseInt(b)
return a+b,nil
}|err| return 0,err
}
chen56
commented
Aug 29, 2018
•
my idea: // common util func
func parseInt(s string) (i int64, err error){
return strconv.ParseInt(s, 10, 64)
}
// expression check err 1 : check and use err variable
func parseAndSum(a string ,b string) (int64,error) {
sum := parseInt(a) + parseInt(b) |err| return 0,err
return sum,nil
}
// expression check err 2 : unuse variable
func parseAndSum(a string , b string) (int64,error) {
a,err := parseInt(a) |_| return 0, fmt.Errorf("parseInt error: %s", a)
b,err := parseInt(b) |_| { println(b); return 0,fmt.Errorf("parseInt error: %s", b);}
return a+b,nil
}
// block check err
func parseAndSum(a string , b string) ( int64, error) {
{
a := parseInt(a)
b := parseInt(b)
return a+b,nil
}|err| return 0,err
}
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
thejerf
Aug 29, 2018
@chen56 and all future commenters: See https://go.googlesource.com/proposal/+/master/design/go2draft.md .
I suspect this obsoletes this thread now and there is little point in continuing on here. The Wiki feedback page is where things should probably go in the future.
thejerf
commented
Aug 29, 2018
|
@chen56 and all future commenters: See https://go.googlesource.com/proposal/+/master/design/go2draft.md . I suspect this obsoletes this thread now and there is little point in continuing on here. The Wiki feedback page is where things should probably go in the future. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
carlmjohnson
Aug 29, 2018
Contributor
The mega example using the Go 2 proposal:
func NewClient(...) (*Client, error) {
var (
listener net.Listener
conn net.Conn
)
handle err {
if conn != nil {
conn.Close()
}
if listener != nil {
listener.Close()
}
return nil, err
}
listener = check net.Listen("tcp4", listenAddr)
conn = check ConnectionManager{}.connect(server, tlsConfig)
if forwardPort == 0 {
env, err := environment.GetRuntimeEnvironment()
if err != nil {
log.Printf("not forwarding because: %v", err)
} else {
forwardPort, err = env.PortToForward()
if err != nil {
log.Printf("env couldn't provide forward port: %v", err)
}
}
}
var forwardOut *forwarding.ForwardOut
if forwardPort != 0 {
u, _ := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", forwardPort))
forwardOut = forwarding.NewOut(u)
}
client := &Client{...}
toServer := communicationProtocol.Wrap(conn)
check toServer.Send(&client.serverConfig)
check toServer.Send(&stprotocol.ClientProtocolAck{ClientVersion: Version})
session := check communicationProtocol.FinalProtocol(conn)
client.session = session
return client, nil
}I think this is about as clean as we can hope for. The handle block has the good qualities of the again label or Ruby's rescue keyword. The only questions left in my mind are whether to use punctuation or a keyword (I think keyword) and whether to allow for getting the error without returning it.
|
The mega example using the Go 2 proposal: func NewClient(...) (*Client, error) {
var (
listener net.Listener
conn net.Conn
)
handle err {
if conn != nil {
conn.Close()
}
if listener != nil {
listener.Close()
}
return nil, err
}
listener = check net.Listen("tcp4", listenAddr)
conn = check ConnectionManager{}.connect(server, tlsConfig)
if forwardPort == 0 {
env, err := environment.GetRuntimeEnvironment()
if err != nil {
log.Printf("not forwarding because: %v", err)
} else {
forwardPort, err = env.PortToForward()
if err != nil {
log.Printf("env couldn't provide forward port: %v", err)
}
}
}
var forwardOut *forwarding.ForwardOut
if forwardPort != 0 {
u, _ := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", forwardPort))
forwardOut = forwarding.NewOut(u)
}
client := &Client{...}
toServer := communicationProtocol.Wrap(conn)
check toServer.Send(&client.serverConfig)
check toServer.Send(&stprotocol.ClientProtocolAck{ClientVersion: Version})
session := check communicationProtocol.FinalProtocol(conn)
client.session = session
return client, nil
}I think this is about as clean as we can hope for. The |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
sdwarwick
Aug 29, 2018
I'm trying to understand the proposal - it appears that there is only one handle block per function, rather than the ability to create different responses to different errors throughout the function execution processes. This seems like a real weakness.
I'm also wondering if we are overlooking the critical need for developing testing harnesses in our systems as well. Considering how we are going to exercise error paths during tests should be part of the discussion, but I don't see that either,
sdwarwick
commented
Aug 29, 2018
•
|
I'm trying to understand the proposal - it appears that there is only one handle block per function, rather than the ability to create different responses to different errors throughout the function execution processes. This seems like a real weakness. I'm also wondering if we are overlooking the critical need for developing testing harnesses in our systems as well. Considering how we are going to exercise error paths during tests should be part of the discussion, but I don't see that either, |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ianlancetaylor
Aug 30, 2018
Contributor
@sdwarwick I don't think this is the best place to discuss the design draft described at https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling.md . A better approach is to add a link to a writeup at the wiki page at https://github.com/golang/go/wiki/Go2ErrorHandlingFeedback .
That said, that design draft does permit multiple handle blocks in a function.
|
@sdwarwick I don't think this is the best place to discuss the design draft described at https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling.md . A better approach is to add a link to a writeup at the wiki page at https://github.com/golang/go/wiki/Go2ErrorHandlingFeedback . That said, that design draft does permit multiple handle blocks in a function. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ianlancetaylor
Aug 30, 2018
Contributor
This issue started out as a specific proposal. We aren't going to adopt that proposal. There has been a lot of great discussion on this issue, and I hope that people will pull the good ideas out into separate proposals and into discussion on the recent design draft. I'm going to close out this issue. Thanks for all the discussion.
|
This issue started out as a specific proposal. We aren't going to adopt that proposal. There has been a lot of great discussion on this issue, and I hope that people will pull the good ideas out into separate proposals and into discussion on the recent design draft. I'm going to close out this issue. Thanks for all the discussion. |
ianlancetaylor
closed this
Aug 30, 2018
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
claygod
Sep 3, 2018
If to speak in the set of these examples:
r, err := os.Open(src)
if err != nil {
return err
}That I would like to write in one line approximately so:
r, err := os.Open(src) try ("blah-blah: %v", err)Instead of "try", put any beautiful and suitable word.
With such a syntax, the error would return and the rest would be some default values depending on the type. If I need to return along with an error and something else specific, rather than default, then no one cancels the classic more multi-line option.
Even more shortly (without adding some kind of error handling):
r, err := os.Open(src) try)
P.S. Excuse me for my English))
claygod
commented
Sep 3, 2018
|
If to speak in the set of these examples: r, err := os.Open(src)
if err != nil {
return err
}That I would like to write in one line approximately so: r, err := os.Open(src) try ("blah-blah: %v", err)Instead of "try", put any beautiful and suitable word. With such a syntax, the error would return and the rest would be some default values depending on the type. If I need to return along with an error and something else specific, rather than default, then no one cancels the classic more multi-line option. Even more shortly (without adding some kind of error handling): r, err := os.Open(src) try) |
ianlancetaylor
referenced this issue
Sep 7, 2018
Open
proposal: Go 2: The #id/catch error model, a rethink of check/handle #27519
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
prigornitskiy
Sep 21, 2018
My variant:
func CopyFile(src, dst string) string, error {
r := check os.Open(src) // return nil, error
defer r.Close()
// if error: run 1 defer and retun error message
w := check os.Create(dst) // return nil, error
defer w.Close()
// if error: run 2, 1 defer and retun error message
if check io.Copy(w, r) // return nil, error
}
func StartCopyFile() error {
res := check CopyFile("1.txt", "2.txt")
return nil
}
func main() {
err := StartCopyFile()
if err!= nil{
fmt.printLn(err)
}
}
prigornitskiy
commented
Sep 21, 2018
|
My variant:
|
ianlancetaylor commentedJul 25, 2017
•
edited
There have been many proposals for how to simplify error handling in Go, all based on the general complaint that too much Go code contains the lines
I'm not sure that there is a problem here to be solved, but since it keeps coming up, I'm going to put out this idea.
One of the core problems with most suggestions for simplifying error handling is that they only simplify two ways of handling errors, but there are actually three:
It is already easy (perhaps too easy) to ignore the error (see #20803). Many existing proposals for error handling make it easier to return the error unmodified (e.g., #16225, #18721, #21146, #21155). Few make it easier to return the error with additional information.
This proposal is loosely based on the Perl and Bourne shell languages, fertile sources of language ideas. We introduce a new kind of statement, similar to an expression statement: a call expression followed by
||. The grammar is:Similarly we introduce a new kind of assignment statement:
Although the grammar accepts any type after the
||in the non-assignment case, the only permitted type is the predeclared typeerror. The expression following||must have a type assignable toerror. It may not be a boolean type, not even a named boolean type assignable toerror. (This latter restriction is required to make this proposal backward compatible with the existing language.)These new kinds of statement is only permitted in the body of a function that has at least one result parameter, and the type of the last result parameter must be the predeclared type
error. The function being called must similarly have at least one result parameter, and the type of the last result parameter must be the predeclared typeerror.When executing these statements, the call expression is evaluated as usual. If it is an assignment statement, the call results are assigned to the left-hand side operands as usual. Then the last call result, which as described above must be of type
error, is compared tonil. If the last call result is notnil, a return statement is implicitly executed. If the calling function has multiple results, the zero value is returned for all result but the last one. The expression following the||is returned as the last result. As described above, the last result of the calling function must have typeerror, and the expression must be assignable to typeerror.In the non-assignment case, the expression is evaluated in a scope in which a new variable
erris introduced and set to the value of the last result of the function call. This permits the expression to easily refer to the error returned by the call. In the assignment case, the expression is evaluated in the scope of the results of the call, and thus can refer to the error directly.That is the complete proposal.
For example, the
os.Chdirfunction is currentlyUnder this proposal, it could be written as
I'm writing this proposal mainly to encourage people who want to simplify Go error handling to think about ways to make it easy to wrap context around errors, not just to return the error unmodified.