This is a counter-proposal to #32437
catch
would function much like the proposed try
with a few specific differences:
1.) catch
would not return any values, meaning it must be on a line by itself, like panic()
2.) catch
takes 1..N arguments
2a.) the first argument must be of type error
2b.) The remainder of the arguments are optional and wrap the error using the given format string and args, as if sent through the new fmt.Errorf which does error wrapping similar to github.com/pkg/errors.
e.g.
func getConfig(config string) (*Config, error)
f, err := os.Open(config)
catch(err)
defer f.Close()
// use f to make a c *Config...
return c, nil
}
In this code, catch is the equivalent of
if err != nil {
return nil, err
}
If err is non-nil, it will return zero values for all other return values, just like try
. The difference being that since catch
doesn't return values, you can't "hide" it on the right hand side. You also can't nest catch
inside another function, and the only function you can nest inside of catch
is one that just returns an error. This is to ensure readability of the code.
This makes catch just as easy to see in the flow of the code as if err != nil is now. It means you can't magically exit from a function in the middle of a line if something fails. It removes nesting of functions which is otherwise usually rare and discouraged in go code, for readability reasons.
This almost makes catch like a keyword, except it's backwards compatible with existing code in case someone already has the name catch defined in their code (though I think others have done homework saying try and catch are both rarely used names in Go code).
Optionally, you can add more data to an error in the same line as catch:
func getConfig(user, config string) (*Config, error)
f, err := os.Open(config)
catch(err, "can't open config for user %s", user)
defer f.Close()
// use f to make a c * Config...
return c, nil
}
In this configuration, catch is equivalent to
if err != nil {
return nil, fmt.Errorf("can't open config for user %s: %v", user, err)
}
And would utilize the new implicit error wrapping.
This proposal accomplishes 3 things:
1.) It reduces if err != nil boilerplate
2.) It maintains current legibility of exit points of a function having to be indicated by the first word on the line
3.) It makes it easier to annotate errors than either if err != nil or the try
proposal.
For the record, I don't mind if err != nil
, and all things being equal, I would rather not remove that boilerplate, since errors are just values and all that jazz. I know many, many long-time gophers that agree.
However, if we're going to change the language, this is a much better change than the proposed try
in my opinion. This maintains legibility of code, maintains ease of finding exit points in the code, still reduces boilerplate, and encourages people to add context to an error.
IMO it removes the major faults of try, which are the high likelihood of missing a call to try
embedded in the right hand side somewhere when reading code, and the problem of encouraging nesting of functions which is distinctly bad for code readability.
One variation would be:
catch
only takes one argumentcatch
to take additional arguments for format string and args decorate := func(err error) error { return fmt.Errorf("foo failed: %v", err) }
...
f, err := os.Open(config)
catch(err, decorate) // decorate called in error case
or using helper functions that could live somewhere in the standard library:
catch(err, fmt.Annotatef("foo failed for %v", arg1))
That would be more parallel to the current proposal for try
as a builtin.
edit: part of the rationale for a handler function is it means a builtin does not directly rely on fmt
semantics, which is the concern @Merovius expressed better in the next comment.
My main reason for disagreeing with that is that it couples the fmt
-package into the language. i.e. to actually add this to the spec, you'd also need to define what fmt
does in the spec - and IMO, fmt
is far too large (conceptually) to be part of the language proper. So, IMO, the multiple-argument form you are suggesting should be removed.
That's a good point, and actually why I made 2a and 2b. I think we could remove 2b pretty easily. It could easily be replaced by a helper function in fmt (or user code):
// Wrap returns nil if err is nil, otherwise it returns a wrapped
// version of err with the given fmt string as per fmt.Errorf.
func Wrap(err error, msg string, args ...interface{})
then you could do
catch(fmt.Wrap(err, "error opening config for user %s", user))
It's not quite as nice, though.
And for the record, I'd be ok with making this part of fmt be part of the language if it encourages people annotate their errors.
@natefinch What about if i have more than two returns?
func getConfig(user, config string) (*Config, error, result)
f, err := os.Open(config)
catch(err)
defer f.Close()
// use f to make a c * Config...
return c, nil, nil
}
It works like try
from the linked issue, where using it requires that it's in a function where the last return value is of type error, and it returns zero values for everything except the error.
so like if you had
func getConfig(user, config string) (*Config, result, error)
catch(err) would return nil, result{}, err
You can simplify this a bit. Since it expects an error value, you can give it any error value. So then it can be catch err
or catch fmt.Errorf(“can’t open config for %s”, user)
— no need to implicitly use this error generating function and you can return any kind of error that makes sense in your situation. Though I don’t like the use of catch. It would be perfectly legal in your scheme to say something like this but it looks weird!
If !inrange(low, value, high) {
catch OutOfRangeError
}
Except for the name, this would be a perfectly legitimate use at the where error conditions are first detected and even better than having to say
If !inrange(low, value, high) {
return nil, OutOfRangeError
}
On IBM systems on can use a macro ABEND to abnormally terminate a task. It was a bit like panic
but the name was suggestive of its intended use. catch
doesn’t have that. Neither did try
. I thought of returnOnError
or errorReturn
or just ereturn
or abreturn
or may be even raise
as in C. Any of these work for me better than catch
or try
but none seem ideal.
So, it can't be a keyword, because that's not backwards compatible. So it would need to be catch(OutOfRangeError)
....
I do agree that the name might need some tweaking. Naming is hard, and I welcome suggestions for another name that might be more appropriate.
The problem is really that it is most accurately called maybeReturn
How about yield
? Where it's keep going, or stop if something is in the way? Also yield is to produce, like it produces the error.
f, err := os.Open(config)
yield(err)
defer f.Close()
and then it could be
yield(OutOfRangeError)
which looks better to me
(Ignoring its use as giving up the processor to another coroutine or thread) yield is better than catch!
returnOnError or returnIfError exactly captures the semantics but it is a bit long.
returnIf can also work provided error values have err or error in their name.
returnIf(OutOfRangeError)
Both yield
and catch
have very strong connotations in other languages, and I wouldn't suggest them for your proposal. As you say, naming is _hard_.
But I have to disagree with this if for no other reason than its rigidity; if
catch(err, "can't open config for user %s", user)
is really just
if err != nil {
return fmt.Errorf("can't open config for user %s: %v", user, err)
}
... it's really not that valuable. _Most_ of my error handling is done with github.com/pkg/errors
, although I have yet to evaluate the new proposals around wrapped errors. And others perhaps like dedicated typed errors, while others may use constants. This solves a narrow set circumstances.
But to think this through further... what happens in this case:
func DoYourThing(a string) (x string, err error) {
x = fmt.Sprintf("My thing is '%a'", a)
// always returns an err
err = DoSomethingBad()
catch(err)
// process some other stuff.
return
}
func DoAThing() {
x, _ := DoYourThing("cooking")
fmt.Printf("%s!!\n", x)
}
@provPaulBrousseau, IMHO catch or its equivalent shouldn’t be used if you are returning an error as well as a legit value. [This is another reason I bemoan the lack of a sum type in Go. With sum type you’d use it in most cases where an error or a legit value is returned but not both. For examples such as yours, a tuple or a product type would be used. In such a language catch
would only work if the parent function returned a sum type]
But if you do use catch
, it would return “”, err
.
@provPaulBrousseau note that my intent is that this use the new fmt.Errorf which actually does wrapping like github.com/pkg/errors (which is what we use at work, as well). So that fmt.Errorf looks like the oldschool "just reuse the string", but it's actually wrapping under the hood. I'll update the original post to make that clear.
Certainly, if you want something more complicated, like turning one error into some other type of error, you'd instead use the methods you used before, with an if statement. This is just trying to remove a little boilerplate and remove some friction from adding context to errors.
oh.... and yes, just like try
, catch
, and yield
have meanings in other languages.... but this isn't other languages, it's Go. And if the name is a good one, reusing it to mean something different seems ok, IMO... especially since it's not a keyword.
Option 2c
?
catch takes an error
value. When the error
value is nil processing continues. When it's !nil it returns from the enclosing function with zero values for each return parameter except the last error value, for which it uses the supplied error.
If named return parameters are used, then the function returns with those values and the supplied error when the error is not nil.
It's a compile time error to use catch in a function that does not have an error value as it's last return.
It's a compile time error to provide more than one argument to catch.
Examples:
func Foo() (int, int, error) {
err := ....
catch err // nil processing continues, !nil return 0,0, err
user := "sally"
err := ....
catch fmt.Errorf("can't open config for user %s: %w", user, err) // note new %w flag, so it's an `error` and an `errors.Wrapper`, nil processing continue, !nil return 0,0, <formatted error>
I like this for all the reasons you pointed out. And there is less "magic" wrt formatting, and if after a release or two it's felt that formatting should be built in it can still be added later by allowing the catch statement to take multiple arguments.
catch <error>
makes it look more like a statement than a function, which IMO helps with it standing out visually and is less likely to lead newcomers to think it's a function.
PS: I realize this would require a tweak to fmt.Errorf
to make it return nil
if the provided error is nil. So what you said with the Wrap
function instead:
func Foo() (int, int, error) {
user := "sally"
err := ...
catch errors.Wrap("can't open config for user: %s: %w", user, err)
catch is a statement, i.e. it must be on a line by itself, like panic()
This seems like a non-starter. Can it just be a func catch(err error, args ...interface{})
instead? That'd be the same result, and backwards compatible. That makes the biggest problem the fact that you need some static analysis to ensure that the return type of the enclosing function returns an error... (not sure how difficult that'd be to achieve in Go's compiler)
WRT Backwards compatibility (catch <error>
isn't backwards compatible)..... I'd be more than happy to wait for something like this until modules becomes the default build mode ala https://blog.golang.org/go2-next-steps
At first glance, this is generally what try
and check
should have been. check
is a great name (and way to avoid the baggage of catch
), whether meaning "verify" or "halt/slow progress".
@apghero why is it a nonstarter? That's basically the whole difference between this and the try proposal. I do not want a function on the right hand side of a complicated statement to be able to exit. I want it to have to be the only thing on the line so it is hard to miss.
@daved check
SGTM, just not all the other bits.
This is many ways worse than try() function proposal in my opinion because it doesn't really solve the underlying goal of reducing boilerplate code.
A function with multiple error check looks like this:
func Foo() (err error) {
var file1, err1 = open("file1.txt")
catch(err1)
defer file1.Close()
var file2, err2 = open("file2.txt")
catch(err2)
defer file1.Close()
}
Vs. originally proposed try() function:
func Foo() (err error) {
var file1 = try(open("file1"))
defer file1.Close()
var file2 = try(open("file1"))
defer file2.Close()
}
Later looks more concise and elegant in my opinion.
@ubikenobi It's always trade-offs. In my opinion, the reduction provided by try
costs far too much in readability. More so when the extra "flexibility" try permits is weighed. The clarity that comes from that single newline (and possibly two columns of text) is worthwhile. The surety that comes from reduced flexibility? Even more critically so.
@apghero why is it a nonstarter? That's basically the whole difference between this and the try proposal. I do not want a function on the right hand side of a complicated statement to be able to exit. I want it to have to be the only thing on the line so it is hard to miss.
Me either. But that can be accomplished without syntax. The syntax part is the thing that breaks backwards compatibility, so why not just do what panic
does and return nothing? You can't use a function that returns nothing on the right hand side of a statement, or in an expression. Therefore it's limited to the left most column of a block, which is what we're looking for.
This still would take up a keyword: catch, but only barely improve on the try variant.
While I do think this is a step forward, it still only allows "lazily" error handling.
What for users that use custom errors that use wrapping functions?
What for users that want to log for developers & operators and return human-understandable errors to the users?
if err != nil {
log.Println("Hey operating, big error, here's the stack trace: ...")
return myerrorpackage.WrappedErrorConstructor(http.StatusInternalServerError, "Some readable message")
}
If we plan on taking up a whole keyword, can we at least attach some function to it?
Counter proposal to the whole try or catch: do try/catch with check & handle (better keywords imho), scope specific.
Suggestion is part of the whole mix of error handling proposals: scoped check/handle proposal
This still would take up a keyword: catch, but only barely improve on the try variant.
The point is to reduce the footgun rating of a solution, not to offer an "improved" novelty.
What for users that use custom errors that use wrapping functions?
This was covered in comments.
If we plan on taking up a _whole_ keyword, can we at least attach some function to it?
Now I'm not sure if this is a troll post. Most keywords serve exactly one function. return
, if
, etc.
This proposal states several times that catch(...)
is a statement, like panic(...)
. But panic
is not a statement, it's a built-in function with no return value.
@daved
Inconsistencies and unnecessary code clutter also negatively impact readability in my opinion. This is why I think catch() proposal is less readable and less safe compared to try() proposal. I'll try to illustrate my three main concerns with a different example below.
//Example 1
func Dispense() (err error) {
var motErr= SetMotorSpeed(5)
catch(motErr)
var fanErr= StartFan(5)
catch(fanErr)
var tempErr =SetTemperature(200)
catch(tempErr)
var itErr = DropItem()
catch(itErr)
}
//Example 2
func Dispense() (err error)
var motErr= SetMotorSpeed(5)
var speedErr= StartFan()
var tempErr =SetTemperature(200)
var printErr = DropItem()
catch(motErr)
catch(fanErr)
catch(tempErr)
catch(itErr)
}
As appose to catch(), with try() function (example below):
func Dispense() (err error){
try(SetMotorSpeed(5))
try(StartFan(5))
try(SetTemperature(200))
try(DropItem())
}
I would like to point out that when flexibility is needed err!=nil pattern works better than both catch() and try() proposal IMO. The main goal of the original try() as a function proposal s to remove unnecessary boilerplate when flexibility is NOT needed.
If y'all want to manifest something other than try()
in 1.14, you need to first
prove that try()
isn't useful for the great majority of existing code.
The Go team has told us that if we can't prove that, the merits of alternatives are moot.
EDIT: This is a fact. Down-voting facts doesn't make them fiction.
This still would take up a keyword: catch, but only barely improve on the try variant.
The point is to reduce the footgun rating of a solution, not to offer an "improved" novelty.
Many of the arguments again try is that it not improves anything, just dumbifies error handling.
This catch can just be written as a package that provides this as a single function. As @networkimprov stated, as long as there's no proof that would improve the try case it's void of use.
What for users that use custom errors that use wrapping functions?
This was covered in comments.
Comments are personal discussions, not part of the proposal, the proposal only has catch with error formatting.
If we plan on taking up a _whole_ keyword, can we at least attach some function to it?
Now I'm not sure if this is a troll post. Most keywords serve exactly one function.
return
,if
, etc.
Return takes multiple values and it's not a function, it has a space after it, clearly marking it as a keyword.
Reserved items should be space separated just so it's clear they're not a local function.
All of this can be replaced by having a local package called: errorHandler
with 1 public function: Catch
.
No need to change the language for the sake of changing it (same does technically count for try() proposal).
I support this proposal.
It is important that any change to error handling allows each error in a function to be annotated explicitly and independently from other errors. Returning the naked error, or using the same annotation for every error return in a function, may be useful in some exceptional circumstances, but are not good general practices.
I also support using a keyword instead of a builtin. If we add a new way to exit a function, it is important that it isn't nestable within other expressions. If we have to wait until further improvements to the language allow this to be accomplished without breaking the Go 1 compatibility promise, then we should do that, rather than make the change now.
_edit_: @networkimprov's #32611 also satisfies my requirements, and I support it as well.
IMO, this doesn't really improve on if err != nil { ... }
fmt on one line.
A more flexible one-extra-line proposal is #32611
err = f()
on err, <single_statement>
EDIT: And here is a catch
proposal where _catch_ does what you'd expect: #27519
The most common error handling code I have written is the classic:
..., err := somefunc1()
if err != nil {
return err
}
..., err := somefunc2()
if err != nil {
return err
}
Like @ubikenobi, I think the catch solution (or the on solution just suggested) adds little value:
..., err := somefunc1()
catch(err)
..., err := somefunc2()
catch(err)
while try lets you focus on the code, not the errors:
... := try(somfunc1())
... := try(somfunc2())
Just as well quickly learned that r is probably an io.Reader and w is probably an io.Writer, we will quickly learn to ignore the try while reading code.
The try proposal really shines when you want to have chaining types and still support errors. Consider:
v, err := Op1(input)
if err != nil {
return err
}
v, err = v.Op2()
if err != nil {
return err
}
result, err := v.Op3()
if err != nil {
return err
}
With try this becomes:
result := Op1(input).Op2().Op3()
This would actually encourage this paradigm more, which I think would be a good thing.
Catch, on the other hand, make it:
v, err := Op1(input)
catch(err)
v, err = v.Op2()
catch(err)
result, err := v.Op3()
catch(err)
Quite honestly, if the error solution does not support chaining then its value is reduced. If it does not eliminate the extra lines of code between statements, its value is reduced.
One scenario where try is clearly not helpful and the check proposal has a very slight advantage over straight Go is:
..., err := somefunc1()
if err != nil {
return ..., fmt.Errorf("somefunc1: %v", err)
}
..., err := somefunc2()
if err != nil {
return ..., fmt.Errorf("somefunc2: %v", err)
}
While catch would add some slight value, it certainly does not carry its own weight vs the existing paradigm give it's other issues.
I can contrive a solution that uses try, but it is worse than the original code because each return requires different processing.
As mentioned, the try solution can easily be extended in the future by adding additional parameters, though they need to be readable.
As for the name try, I am not super enamored with it, it will make many people think "where is the catch?". I could suggest must, instead, but right now we associate must with a panic. I don't know a better word choice so try is okay. I certainly don't want something like returnOnError even though it is pretty descriptive!
@ngrilly mentioned that panic is not a statement, and that's a good point. I'm not entirely sure what the limitations are on what we can enforce on a new builtin. If we can't make catch (or whatever we call it) a statement, making it not return a value is good enough... it still means it'll basically have to be on its own line.
I have updated the proposal to remove the language about being a statement, to avoid confusion.
@pborman
[try] would actually encourage this paradigm more, which I think would be a good thing.
I don't think it's a good idea to encourage returning ~the naked~ _an un-annotated_ error. As a general rule, errors should be individually annotated with context before being yielded up the callstack. Although if err != nil { return err }
is the trope often cited in Hacker News comments or whatever, it isn't great practice.
@peterbourgon , the proposal examples already addressed this (fmt.HandleError or something like that). A majority of cases can be handled by a single defer call. Probably should not call this a naked return as return err
is not a naked return. return
with named return values is a naked return and probably should have never made it into the language. If your error handling becomes to complicated (unique handling per error) then all of these solutions are not ideal and you should use standard Go.
@pborman
Probably should not call this a naked return
You're right, I've edited to call this an un-annotated error.
If your error handling becomes to complicated (unique handling per error)
My claim is that this is not complicated error handling, this is [what should be] the default way to handle errors, and that anything less is shortcuts that shouldn't be encouraged in the general case.
My claim is that this is not complicated error handling, this is [what should be] the default way to handle errors, and that anything less is shortcuts that shouldn't be encouraged in the general case.
I think this is an argument against try, check, handle, throw, must, or any other proposal for simplified error handling.
in truth I don't mind Go's error handling that much. I can see try being beneficial, at least to me. It is the least obtrusive, lightest weight, solution I have seen (it also happens to be an improved version of my second suggestion last fall, which was try foo()
rather than try(foo())
.
For my own code I might consider adding a function, something like HandleErrorf, that optionally wrapped the returned error with the file, function, and line number of where try was called from. When debugging code I would turn that on so all my errors tell me where they originated (actually, the full call chain). I think some other packages do this already.
@pborman
if the error solution does not support chaining then its value is reduced.
It seems you and I want different things. I specifically made this proposal because I don't want to encourage people to jam more logic into a single line. I don't want to encourage method chaining. I don't want to hide away error handling. Having to write catch(err)
every time your function might exit is good IMO. I want to be very explicit about what can fail and where the function can exit.
It is important that any change to error handling allows each error in a function to be annotated explicitly and independently from other errors.
@peterbourgon Most of the time, the error context is the same in the whole function. In such a case, I'd prefer to centralize the annotation in one place, instead of annotating each error independently. Russ commented about this: https://github.com/golang/go/issues/32437#issuecomment-503297387
@ubikenobi As far as I understand, your example (where only errors are returned) can be duplicated with this proposal.
func Dispense() (err error){
catch(SetMotorSpeed(5))
catch(StartFan(5))
catch(SetTemperature(200))
catch(DropItem())
}
The main goal of the original try() as a function proposal s to remove unnecessary boilerplate when flexibility is NOT needed.
And yet it is designed to facilitate flexibility. If what you suggest is true, it's flexibility can/should be reduced to only that which is necessary.
@pborman
... we will quickly learn to ignore the try while reading code.
That's the problem.
Also, function chaining is not something I'm on board to promote. It's only really useful for accumulator style types. If it's, otherwise, some machine being controlled, then the relevant functions don't need to return errors at all since "errors are value" and whatnot. I would use a container type that either provides a final error checking function, or use an error channel with the "body" being called in a goroutine.
If the error solution does not support chaining then its value is reduced. If it does not eliminate the extra lines of code between statements, its value is reduced.
I fully agree with this.
@natefinch, I think you should support no changes to the language in that case. It does everything you want already (expect that you still need the if statement, which is even more precise if err != && err != io.EOF ...
)
I would support no change to the language over check as I believe check has no real benefits over how Go is today. My feeling is if we are going to add something to Go, the try proposal at least seems like a choice that provides actual benefit at little cost. I actually like it and am not opposed to it going in. If it doesn't go in, I am okay, as long as none of the other proposals I have seen do not go in, either.
@ngrilly
Most of the time, the error context is the same in the whole function.
I don’t agree. Error annotations usually are and should be unique for each received error in a function block.
I also agree with @natefinch that error chaining should not be encouraged.
@natefinch About "panic not being a statement": the proposal just needs to mention that catch
is a built-in function (instead of being a statement).
I don't want to hide away error handling.
@natefinch The try
function doesn't "hide" error handling any more than the catch
function proposed here.
Having to write catch(err) every time your function might exit is good IMO. I want to be very explicit about what can fail and where the function can exit.
try
is not different from catch
on this matter: You also have to write try(err) every time the function might exit. It's also "very explicit about what can fail and where the function can exit".
@ngrilly
The try function doesn't "hide" error handling any more than the catch function proposed here.
It's position in code differs, it's flexibility differs. The difference results in noise. The noise results in it being ignored. try
, as it is, will be something to locate within it's own noise. This proposal reduces the broadness of the noise down to a clear tone that is easy to parse and hard to ignore.
@peterbourgon
I don’t agree. Error annotations usually are and should be unique for each received error in a function block.
Have you read the comment from Russ I linked previously?
Russ agrees with you when you write that "error annotations should be unique for each received error in a function block". But he argues that it is the callee responsibility (not the caller responsibility) to decorate the error with the context. For example, the error returned by os.Open is "open /etc/passwd: permission denied," not just "permission denied."
I also agree with @natefinch that error chaining should not be encouraged.
Why? This is one of the main use case for similar constructs in Rust, Swift, Haskell, etc. People keep repeating try
will make our code unreadable, but Rust has introduced the try! macro and then the ? operator a long time ago now, and they haven't suffered from this hypothetical problem. I fail to see why it would have worked for Rust, and it wouldn't work for us.
People keep repeating try will make our code unreadable, but Rust ...
All due respect to Rust, please don't use it as an example of readability. For it's own goals, it is clearly a great language. For the benefits that Go provides and I am drawn to, there is no relation whatsoever.
@daved
It's position in code differs, it's flexibility differs. The difference results in noise. The noise results in it being ignored.
try
, as it is, will be something to locate within it's own noise. This proposal reduces the broadness of the noise down to a clear tone that is easy to parse and hard to ignore.
It's mostly an aesthetic argument. It's in the eye of the beholder.
@ubikenobi has shared earlier an example in which try
seems more readable to me than catch
:
With catch:
func Foo() (err error) {
file1, err1 := open("file1.txt")
catch(err1)
defer file1.Close()
file2, err2 := open("file2.txt")
catch(err2)
defer file1.Close()
}
With try:
func Foo() (err error) {
file1 := try(open("file1"))
defer file1.Close()
file2 := try(open("file1"))
defer file2.Close()
}
Edited: used :=
instead of var =
(thanks @daved)
@ngrilly
Why? This is one of the main use case for similar constructs in Rust, Swift, Haskell, etc.
I understand one of Go's fundamental principles to be that errors are not exceptional but rather common, and that the code to deal with them (i.e. the sad path) is at least as important, and often more important, than the code to accomplish the main work (i.e. the happy path). I also understand that the relative reliability and predictability of Go programs comes in large part from this principle, that errors are lifted up front and center for every expression that can generate them, forcing—or at least strongly encouraging—the programmer to deal with them in situ.
Error chaining subverts this principle in the name of expediency. I don't disagree that in some cases this can be useful. But I would hate to see Go become a language where it is common. We would lose one of its most important strengths.
@ngrilly Your insistence on using var = instead of := is unsettling. ;)
A quick scan produces...
catch:
file open
catch err
defer close
file open
catch err
defer close
try:
file twcvpfn
defer close
file twcvpfn
defer close
edit to add: No hyperbole... The noise of the wrapping is that notable/offensive to me. If I/we avoid that ish in code already, why would it be something to add as a language primitive?
@daved
All due respect to Rust, please don't use it as an example of readability. For it's own goals, it is clearly a great language. For the benefits that Go provides and I am drawn to, there is no relation whatsoever.
I agree that Rust as a whole is less readable than Go. But the argument is not necessarily transposable to specific features of the language, like try! and ?.
@peterbourgon
I understand one of Go's fundamental principles to be that errors are not exceptional but rather common, and that the code to deal with them (i.e. the sad path) is at least as important, and often more important, than the code to accomplish the main work (i.e. the happy path).
Rust error handling relies on the exact same principle. They even mention Go in their RFCs!...
I also understand that the relative reliability and predictability of Go programs comes in large part from this principle
I think the choice of words matters here: "reliability" and "predictability". The check/handle proposal and the try proposal don't affect the reliability and predictability of Go programs. These constructs are perfectly explicit and determinist. There is, maybe, a "readability" issue, but not a "reliability/predictability" issue.
errors are lifted up front and center for every expression that can generate them, forcing—or at least strongly encouraging—the programmer to deal with them in situ.
I agree with this. This is my main doubt about the try
proposal. People are supposed to decorate their error with a defer statement. But they can also omit it. That said, we already have a lot of code in the wild with if err != nil { return nil, err }
.
Error chaining subverts this principle in the name of expediency. I don't disagree that in some cases this can be useful. But I would hate to see Go become a language where it is common. We would lose one of its most important strengths.
It looks like the main source of our disagreement is that you (and other gophers) think each error should be decorated differently, while Russ (and other gophers including me) think the decoration can be the same for the whole function (or the whole call chain in the case of chained calls).
@daved
@ngrilly Your insistence on using var = instead of := is unsettling. ;)
LOL. I've blindly copied the code shared earlier in this thread. I've fixed it.
@Merovius
My main reason for disagreeing with that is that it couples the
fmt
-package into the language.
What if catch
would be allowed to (re)use runtime's print
only for its formatting?
catch(err, "can't open config for user ", user, ". Uid: ", ruid)
The code is in the runtime and it will be there forever and changes needed to make a catch
's sprint
of it are in the 10-15 LoC range.
Overall, I can undersigned on @natefinch proposal. Once upon a time I was on the _let me write it short_ side. Now I value clarity and locality of reading way more.
P.S. Naming is hard. And consequences of bad or just a bit off name are way more serious than we usually would like to think. Just think what if the go1 had had a "fail" instead of "error"?
If either "try" or "catch" are implicit returns, then the first thing that strikes me is that the "return" statement might be the one to enhance for this task. That may be hard to do while keeping backwards compatibility, but looking at the problem from that perspective may reveal different solutions.
The construct "return err if err != nil" (roughly) has been positively received elsewhere, I think that also needs to shift the focus towards "return" as the key principle.
I think the main thrust of Nate's "catch" suggestion is a good one: one per line, modelled on panic.
I also want to add that the existence of bare returns could be brought to bear on this discussion: what the compiler already knows may come in very handy (the last argument needs to be of type error, for example) if one feels that the return values need to be set up appropriately.
And "fail" (stolen from @ohir) may in fact be an excellent alternative:
fail(fmt.Errorf("reporting: %v", err))
although more likely to collide with existing use.
One more thing: fmt.Errorf need not be the only way to assemble an error object with a useful description, errors.Stringf(string, ...interface{}) could be a new name for the same functionality if it can avoid importing the entire fmt baggage. If not, then the code must just use fmt.Errorf sparingly.
The bit about fmt.Errorf cleverly returning nil when err == nil is not throw away either. Half the time I make the mistake of creating an err where there wasn't one, the runtime reminds me: this approach would just be a less subtle hammer.
I'm not familiar with this medium, so please forgive the presentation.
@ohir If you look at the spec, it seems clear to me that that wouldn't be a good idea. Both in that it's implementation-defined what it does and that it is limited in what arguments it can receive. ISTM using that would be a perfect example of how a compromise usually ends up making neither of the parties happy. Really, as a regular Gopher, you should just forget print
exists and not encourage its use even more :)
I like this better than the vast majority of error handling change proposals.
I had a notion this morning that I'd like to throw on the pile. There is a convention in several packages, both in the standard library and the wider ecosystem, of defining Must*
variations of functions for simplicity, and... well--
Perhaps (as @pborman also alluded to already) a must
keyword could be defined, as a shorthand prefix operator for functions that return one error and any number of other values, which captures the error returned and _panics_ if it's not nil
, but otherwise proceeds with the remaining return values.
To use the example from the initial proposal:
func getConfig(config string) (returnCfg *Config, returnErr error)
defer func() {
if r := recover(); r != nil {
returnCfg, returnErr = nil, fmt.Errorf("config: Unexpected error: %w", r)
}
}()
f := must os.Open(config)
defer must f.Close() // Succinctly set up error handling for usually-ignored unlikely-to-error functions
// use f to make a c *Config...
return c, nil
}
Ideally (I think), this would combine with some solution I haven't thought of for the problem of named return values being very easily shadowed (which seems to be a separate common complaint) so that this somewhat-common formulation of deferred panic-handling is easier to write.
Edit: On careful re-reading of the history, it looks like basically every variation on this has already been proposed at some point, with this specific version being kind of the worst (as measured by amount of time before it was erased in favor of another revision). So... never mind! <_<
@jesse-amano The must
keyword you propose doesn't differentiate between a standard error returned by a function call, and a real panic triggered by a bug in your code.
Yes, the thought more-or-less aligned with the same thinking that drove earlier incarnations of the try
proposal, and is at least as flawed. I also concur with earlier remarks that a keyword is in principle a more drastic change to the language than "merely" a magic built-in function like try
.
@pborman,
Just as well quickly learned that r is probably an io.Reader and w is probably an io.Writer, we will quickly learn to ignore the try while reading code.
The try proposal really shines when you want to have chaining types and still support errors. Consider:
v, err := Op1(input) if err != nil { return err } v, err = v.Op2() if err != nil { return err } result, err := v.Op3() if err != nil { return err }
With try this becomes:
result := Op1(input).Op2().Op3()
Actually it becomes
result := try(try(try(Op1(input)).Op2()).Op3())
Finding out which try failed would be a trying exercise! In this single expression there are quite a few basic blocks! It would be very hard to learn to "unsee" the trys.
Oh, you are right! Good catch (no pun intended.)
While not suggesting this, it seems like:
result := try(Op1(input)).try(Op2()).try(Op3())
would be more readable but would complicate the semantics of try a bit. I guess it would say that try(r.f())
and r.try(f())
are equivalent. I haven’t thought about it enough to think if this introduces ambiguity.
In any event, the point is you don’t care which try failed (which is actually one of the reasons Go did not have try-catch). If you care you use the well understood if statement. I still see the proposed catch in this thread as of nearly no value as it still causes a visual break and only limits a slight amount of boilerplate. If something is added I think the proposed try is probably the most reasonable path.
Of course, if try is added, you can continue to use an if statement. Go does not eliminate all personal choice and style.
I came here to propose a very similar solution. I do think this proposal is simpler and clearer than the try(...) proposal put forward by the Go team, and would have little impact on existing code except to cut a lot of boilerplate.
I'd use check as the name, catch has too many other connotations (and is used elsewhere by the calling function, not the one throwing). I would prefer if you could pass an optional error as a second argument, to allow neatly wrapping errors in one line. That would get rid of problems with requiring fmt etc. and make the proposal simpler and still allow inline wrapping. You could even use a function returning error as the second argument then and replace some of the line noise caused by the if err := ... ; err != nil { idiom.
To show some examples, which I think it improves:
f, err := os.Open(filename)
if err != nil {
return …, err // zero values for other results, if any
}
becomes:
f, err := os.Open(filename)
check(err)
And the much longer example from the overview goes from:
func CopyFile(src, dst string) error {
r, err := os.Open(src)
if err != nil {
return fmt.Errorf("copy %s %s: %v", src, dst, err)
}
defer r.Close()
w, err := os.Create(dst)
if err != nil {
return fmt.Errorf("copy %s %s: %v", src, dst, err)
}
if _, err := io.Copy(w, r); err != nil {
w.Close()
os.Remove(dst)
return fmt.Errorf("copy %s %s: %v", src, dst, err)
}
if err := w.Close(); err != nil {
os.Remove(dst)
return fmt.Errorf("copy %s %s: %v", src, dst, err)
}
}
to this shorter example where the extra 1 line for error handling helps a lot with clarity when compared with the try example IMO, and even optionally replace the if err := xxx; err != nil { pattern with something simpler:
func CopyFile(src, dst string) error {
r, err := os.Open(src)
check(err, fmt.Errorf("copy %s %s: %v", src, dst, err))
defer r.Close()
w, err := os.Create(dst)
check(err, fmt.Errorf("copy %s %s: %v", src, dst, err))
_, err := io.Copy(w, r)
check(err, func() error {
w.Close()
os.Remove(dst)
return fmt.Errorf("copy %s %s: %v", src, dst, err)
}())
err := w.Close()
check(err, func() error {
os.Remove(dst)
return fmt.Errorf("copy %s %s: %v", src, dst, err)
}())
}
The main reason I like this proposal is that it doesn't obfuscate the function calls like os.Open(src), w.Close() as try would, and doesn't encourage chaining or wrapping as try would.
The error handling starts on the line after, as now, but can be shorter if required (even in the case of annotation, but especially when there is no annotation). I don't want fewer lines of code at the expense of clarity, so one extra line at times vs try is a win in my book.
This proposal gets rid of the bit people don't like (verbose error handling), and leaves the rest of the code alone.
That expects a level of function overriding that isn't a part of the Go paradigm. Your check
prototype can accept 1 argument (of type error
) or 2 arguments (of type error
and something that _looks_ like a defer, except that it returns an error whereas defers are non-parameterized). Go doesn't support "optional" arguments, and this isn't quite variadic.
@provPaulBrousseau Go does support optional arguments in certain built-in functions, and this would be a built-in function. You can make(map[int]int)
, make([]int, 5)
and make([]int, 0, 5)
.
WRT @kennygrant's example: isn't that actually more verbose than just using an iferrnil?
I think this is not much better than the try
proposal. All we need is to be able to write if err { ... }
without having to put != nil
.
I guess you should close this proposal as the one to keep it simple and keep it as it is got way more positive attention... :hankey:
I think this is not much better than the
try
proposal. All we need is to be able to writeif err { ... }
without having to put!= nil
.
Thank you sir, I do think Go should allow implicit casting from nil interface to bool.
I think this is not much better than the
try
proposal. All we need is to be able to writeif err { ... }
without having to put!= nil
.
Everyone seems to have a different opinion on error handling ;)
I'd be fairly happy with that solution too - allow if err {} on one line, and simple error handling would be a lot more succinct. It would perhaps have a more wide-ranging impact though (allowing implicit conversion of non-nil to bool in all code). Not sure all the heat and noise around this issue is warranted, but I would prefer a solution that shortens error handling and doesn't insert itself into the happy path instead (as try does).
I'm just gonna throw this out there at current stage. I will think about it some more, but I thought I post here to see what you think. Maybe I should open a new issue for this?
So, what about doing some kind of generic C macro kind of thing instead to open up for more flexibility?
Like this:
define returnIf(err error, desc string, args ...interface{}) {
if (err != nil) {
return fmt.Errorf("%s: %s: %+v", desc, err, args)
}
}
func CopyFile(src, dst string) error {
r, err := os.Open(src)
:returnIf(err, "Error opening src", src)
defer r.Close()
w, err := os.Create(dst)
:returnIf(err, "Error Creating dst", dst)
defer w.Close()
...
}
Essentially returnIf will be replaced/inlined by that defined above. The flexibility there is that it's up to you what it does. Debugging this might be abit odd, unless the editor replaces it in the editor in some nice way. This also make it less magical, as you can clearly read the define. And also, this enables you to have one line that could potentially return on error. And able to have different error messages depending on where it happened.
Edit: Also added colon in front of the macro to suggest that maybe that can be done to clarify it's a macro and not a function call.
@nikolay
All we need is to be able to write
if err { ... }
without having to put!= nil
.
It's a much larger change than try
. It would imply to change the language spec to accept implicit casting from nil to bool, not just in if
, but everywhere (an exception for if
would be really weird). The implicit casting could introduce hard to track bugs. And we would still need to change gofmt to format this on 1 line instead on 3.
@Chillance You're essentially proposing to introduce an hygienic macro system in Go. This opens another can of worms.
An issue was created proposing this: https://github.com/golang/go/issues/32620.
Here is what it looks like in Rust: https://doc.rust-lang.org/1.7.0/book/macros.html.
@ngrilly Yeah, I'm sure there other things to consider for the macro way. I just wanted to throw it out there before things are added too quickly. And thanks for the link issue. Great to see others brought it up too. Naturally, if macros would be overused it could get complicated, but then you have to blame yourself writing the code like that I suppose... :)
@Chillance I'm personally opposed to adding hygienic macros to Go. People are complaining about try making the code harder to read (I disagree and think it makes it easier to read, but that's another discussion), but macros would be strictly worse from this point of view. Another major issue with macros is that it makes refactoring, and especially automated refactoring, so much harder, because you usually have to expand the macro to refactor. I repeat myself, but this is a whole can of worms. try is 1 year old toy compared to this.
@ngrilly My initial thought with Go macros is that they would be simple, shorter tidbits. And you would use those in the function to make it easier to read. That is, not overuse macros. And, use for simpler things that you don't want to repeat, such as error handling. It is not suppose to be very large, because then just use a function I suppose. Also, having a macro, doesn't that make refactoring easier as you have one place to change things instead of maybe many in the function?
@Chillance The problem with macros is that tools (especially refactoring tools) have to expand the code to understand the semantics of the code, but must produce code with the macros unexpanded. It's hard. Look at this for example in C++: http://scottmeyers.blogspot.com/2015/11/the-brick-wall-of-c-source-code.html. I guess it's a little bit less hard in Rust than in C++ because they have hygienic macros. If the complexity issue can be overcomed, I could change my mind. Must be noted it changes nothing to the discussion about try
, since Rust which has macros, has implemented a strict equivalent of try
as a built-in macro ;-)
Why not use existing keywords like:
func ReadFile(filename string) ([]byte, error) {
f, err := os.Open(filename)
return if err != nil
defer f.Close()
return ioutil.ReadAll(f)
}
f, err := os.Open(filename)
return if errors.Is(err, os.ErrNotExist)
return x, y, err if err != nil
A more complicated example doesn't seem to have advantages in terms of readability compared to the traditional if clause, though:
f, err := os.Open(filename)
return nil, errors.Wrap(err, "my extra info for %q", filename) if errors.Is(err, os.ErrNotExist)
This seems to be nothing more than an equivalent to a macro such as the following:
#define catch(a) if a != nil { return }
Why not just make macros a feature and let users implement their own shorthands? Could even shorthand the decoration
#define catch(a, msg) if a != nil { return fmt.Errorf("%s: %w", msg, a) }
@fabian-f
Why not just make macros a feature and let users implement their own shorthands?
Because we have no time to learn every other library's own flavor of Go just to be able to comprehend what is it doing. We've been there, done that @ C, we've been suffocated by it in C++. RiP Rust was such a wĂĽnderkid until got the feature-fever and passed away.
I wish not the same with Go.
Could even shorthand the decoration
#define catch(a, msg) if a != nil { return fmt.Errorf("%s: %w", msg, a) }
...and possibly will send user keys to the moon in yet other decoration you'll skim over.
@natefinch
I humbly inform you that I reused the catch
word for my #32968 check
proposal.
I would be really glad to see your opinion there, too.
The check
abstract:
func check(Condition bool) {} // built-in signature
check(err != nil)
{
ucred, err := getUserCredentials(user)
remote, err := connectToApi(remoteUri)
err, session, usertoken := remote.Auth(user, ucred)
udata, err := session.getCalendar(usertoken)
catch: // sad path
ucred.Clear() // cleanup passwords
remote.Close() // do not leak sockets
return nil, 0, // dress before leaving
fmt.Errorf("Can not get user's Calendar because of: %v", err)
}
// happy path
check(x < 4) // implicit: last statement is of the sad path
{
x, y = transformA(x, z)
y, z = transformB(x, y)
x, y = transformC(y, z)
break // if x was < 4 after any of above
}
Last one already compiles at playground.
@Merovius [about print/println]
If you look at the spec, it seems clear to me that that wouldn't be a good idea. Both in that it's implementation-defined what it does and that it is limited in what arguments it can receive.
You're right. I was about reusing print
short implementations for rudimentary sprint
- its too cumbersome though.
I agree with @Merovius that we can't put fmt.Errorf
into the language proper. I think that reduces this proposal to the single argument case. It then seems to me that this is isomorphic to the try
proposal (#32437) except that try
(here called catch
) does not return any values. Does that seems correct?
Of course, then the new built-in doesn't help with error annotation, which is one of the objections that people raise about try
.
The name catch
doesn't seem right to me for this. It doesn't catch anything. Seems like check
would be a better choice.
@ianlancetaylor
Of course, then the new built-in doesn't help with error annotation, which is one of the objections that people raise about try.
As a fallback position regarding the fmt
concern, the proposal author had suggested passing in a handler to check
(using your suggested term for catch
) in https://github.com/golang/go/issues/32811#issuecomment-506461203 above.
So under that alternative, a wrapping example could be:
f, err := os.Open(config)
check(fmt.ErrorHandlerf(err, "error opening config for user %s", user))
A similar-in-spirit solution would be to have an error handler as an optional second argument to check
for in-place error annotation (e.g., as suggested by @eihigh in https://github.com/golang/go/issues/32437#issuecomment-499294383):
f, err := os.Open(config)
check(err, fmt.ErrorHandlerf("error opening config for user %s", user))
Some of the concerns that have been raised in the discussion about the try
proposal (#32437) include:
As far I understood the discussion, I think this proposal targets all three of those.
One final question / comment: if a builtin such as check
was to be introduced without returning any values, it seems there would be at least a possibility to allow it to return values in the future in a backwards compatible way? In other words, if this particular proposal was to be pursued, it seems that check
could be introduced without the ability to nest check
, but the door would still be open to allow nesting of check
as a follow-on change if experience suggests it is needed?
@thepudds
f, err := os.Open(config) check(fmt.ErrorHandlerf(err, "error opening config for user %s", user))
FWIW, this code would also work with try
. The changes between the other proposal and this one would seem to be: a) rename try
to check
and b) remove the return value.
The former seems fairly minor.
The latter seems more significant and will essentially require the builtin to be used in a statement context. Rolling out try
(or check
) in two separate phases as you suggest actually sounds pretty good to me.
As I understand it, the proposal as it stands is:
1) Add a new builtin function check
. This function takes a single argument of type error
. It may only be called in a function whose last result parameter is of type error
. If the argument to check
is not nil
, then it immediately returns from the calling function, returning its argument as the last result. Other results remain as they are; if unnamed, they are the zero value of their type.
2) Add a new function fmt.ErrorHandlerf
. This takes a value of type error
, a formatting string, and other arguments. If the first argument, of type error
, is nil
, then fmt.ErrorHandlerf
returns nil
. Otherwise it returns the result of calling fmt.Errorf
on the arguments. Maybe the first argument is also passed to fmt.Errorf
, though I'm not sure quite how that part works.
The advantage is that the block if err != nil { return ... }
turns into a single statement, and there is some support for wrapping errors using fmt.ErrorHandlerf
. We could also add new similar functions as needed.
A disadvantage is that the fmt.ErrorHandlerf
function is always called, although it would return very quickly after not doing anything.
The proposed check
function is similar to the earlier try
proposal (#32437) (which could also have used the suggested fmt.ErrorHandlerf
) except that check
, unlike try
, cannot be used in an expression.
Does this seem like a correct summary of the current state? Thanks.
That is my understanding.
However, this was a counterproposal to try, which has been discarded. I don't think this proposal actually solves that much, I mostly did it as a compromise between people that wanted something like try, and those that thought try would hurt readability too much.
Do you want to withdraw the proposal? The votes on it aren't all that great.
I'm happy to withdraw it given the votes.
Thanks.
Most helpful comment
For the record, I don't mind
if err != nil
, and all things being equal, I would rather not remove that boilerplate, since errors are just values and all that jazz. I know many, many long-time gophers that agree.However, if we're going to change the language, this is a much better change than the proposed
try
in my opinion. This maintains legibility of code, maintains ease of finding exit points in the code, still reduces boilerplate, and encourages people to add context to an error.IMO it removes the major faults of try, which are the high likelihood of missing a call to
try
embedded in the right hand side somewhere when reading code, and the problem of encouraging nesting of functions which is distinctly bad for code readability.