Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: Go 2: simplify error handling with || err suffix #21161

Closed
ianlancetaylor opened this issue Jul 25, 2017 · 519 comments
Closed

proposal: Go 2: simplify error handling with || err suffix #21161

ianlancetaylor opened this issue Jul 25, 2017 · 519 comments
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal v2 A language change or incompatible library change
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Jul 25, 2017

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

if err != nil {
    return err
}

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:

  1. ignore the error
  2. return the error unmodified
  3. return the error with additional contextual information

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:

PrimaryExpr Arguments "||" Expression

Similarly we introduce a new kind of assignment statement:

ExpressionList assign_op PrimaryExpr Arguments "||" Expression

Although the grammar accepts any type after the || in the non-assignment case, the only permitted type is the predeclared type error. The expression following || must have a type assignable to error. It may not be a boolean type, not even a named boolean type assignable to error. (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 type error.

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 to nil. If the last call result is not nil, 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 type error, and the expression must be assignable to type error.

In the non-assignment case, the expression is evaluated in a scope in which a new variable err is 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.Chdir function is currently

func Chdir(dir string) error {
	if e := syscall.Chdir(dir); e != nil {
		return &PathError{"chdir", dir, e}
	}
	return nil
}

Under this proposal, it could be written as

func Chdir(dir string) error {
	syscall.Chdir(dir) || &PathError{"chdir", dir, err}
	return nil
}

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.

@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange Proposal labels Jul 25, 2017
@ianlancetaylor ianlancetaylor added this to the Proposal milestone Jul 25, 2017
@bradfitz
Copy link
Contributor

	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?)

@ianlancetaylor
Copy link
Contributor Author

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.

@mvdan
Copy link
Member

mvdan commented Jul 25, 2017

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.

@object88
Copy link

object88 commented 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?)

@ianlancetaylor
Copy link
Contributor Author

@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
Copy link

Yeah, my example is pretty poor. But not permitting the operation inside an if, for, or switch would resolve a lot of potential confusion.

@jimmyfrasche
Copy link
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)

@ghost
Copy link

ghost 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

boolean := BoolFunc() || BoolExpr

and

err := FuncReturningError() || Expr

It doesn't look good.

@dup2X
Copy link

dup2X commented Jul 26, 2017

less is more...

@mattn
Copy link
Member

mattn commented Jul 26, 2017

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)

@tandr
Copy link

tandr commented 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 :)

@henryas
Copy link

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

@ianlancetaylor
Copy link
Contributor Author

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.

@jimmyfrasche
Copy link
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)
}

@jba
Copy link
Contributor

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

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.

@henryas
Copy link

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.

@rodcorsi
Copy link

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
}

@ALTree
Copy link
Member

ALTree commented Jul 27, 2017

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 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 <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).

@bhinners
Copy link

bhinners commented 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.

@ALTree
Copy link
Member

ALTree commented Jul 27, 2017

@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.

@go-li
Copy link

go-li commented 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

@rodcorsi
Copy link

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, vet warning or will it be accepted?

func Chdir(dir string) (err error) {
	syscall.Chdir(dir) || err
	return nil
}

@ianlancetaylor
Copy link
Contributor Author

@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
}

@henryas
Copy link

henryas commented 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.

@tandr
Copy link

tandr commented 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 😄

@lukescott
Copy link

lukescott commented Jul 5, 2018

@Azareal

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

@msr1k
Copy link

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

  • Its return means return from its caller context. (return type must be same as caller)
  • It takes optionally if before {, in above example it is if err != nil.
  • It must be delegated by the caller with delegate keyword

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.

@vcaesar
Copy link

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 err
result, 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 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 {}*, 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).

@rogpeppe
Copy link
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.

@earthboundkid
Copy link
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.

@earthboundkid
Copy link
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
}

@earthboundkid
Copy link
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
}

@rogpeppe
Copy link
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.

@deanveloper
Copy link

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.

@cthackers
Copy link

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)
}

@deanveloper
Copy link

@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.

@chen56
Copy link

chen56 commented 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
} 

@thejerf
Copy link

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.

@earthboundkid
Copy link
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.

@sdwarwick
Copy link

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,

@ianlancetaylor
Copy link
Contributor Author

@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.

@ianlancetaylor
Copy link
Contributor Author

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.

@claygod
Copy link

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

)
P.S. Excuse me for my English))

@prigornitskiy
Copy link

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)
  }
}

@tobimensch
Copy link

tobimensch commented Oct 21, 2018

Hello,

I've a simple idea, that's based loosely on how error handling works in the shell, just like the initial proposal was. In the shell errors are communicated by return values that are unequal to zero. The return value of the last command/call is stored in $? in the shell. Additionally to the variable name given by the user we could automatically store the error value of the latest call in a predefined variable and have it be checkable by a predefined syntax. I've chosen ? as syntax for referencing the latest error value, that has been returned from a function call in the current scope. I've chosen ! as a shorthand for if ? != nil {}. The choice for ? is influenced by the shell, but also because it appears to make sense. If an error occurs, you naturally are interested in what happened. This is raising a question. ? is the common sign for a raised question and therefore we use it to reference the latest error value that was generated in the same scope.
! is used as a shorthand for if ? != nil, because it signifies that attention has to be paid in case something has gone wrong. ! means: if something has gone wrong, do this. ? references the latest error value. As usual the value of ? equals nil if there was no error.

val, err := someFunc(param)
! { return &SpecialError("someFunc", param, ?) }

To make the syntax more appealing I'd allow for placing the ! line directly behind the call as well as omitting the braces.
With this proposal you could also handle errors without using a programmer defined identifier.

This would be allowed:

val, _ := someFunc(param)
! return &SpecialError("someFunc", param, ?)

This would be allowed

val, _ := someFunc(param) ! return &SpecialError("someFunc", param, ?)

Under this proposal you don't have to return from the function when an error occurs
and you can instead try to recover from the error.

val, _ := someFunc(param)
! {
val, _ := someFunc(paramAlternative)
  !{ return &SpecialError("someFunc alternative try failed too", paramAlternative, ?) }}

Under this proposal you could use ! in a for loop for multiple retries like this.

val, _ := someFunc(param)
for i :=0; ! && i <5; i++ {
  // Sleep or make a change or both
  val, _ := someFunc(param)
} ! { return &SpecialError("someFunc", param, ? }

I'm aware that ! is primarily used for negation of expressions, so the proposed syntax might cause confusion in the uninitiated. The idea is that ! by itself expands to ? != nil when it's used in an a conditional expression in a case like the upper example demonstrates, where it's not attached to any specific expression. The upper for line can't be compiled with the current go, because it doesn't make any sense without context. Under this proposal ! by itself is true, when an error has occurred in the most recent function call, that can return an error.

The return statement for returning the error is kept, because as others commented here it's desirable to see at a glance where your function returns. You can use this syntax in a scenario where an error doesn't require you leaving the function.

This proposal is simpler than some other proposals, since there's no effort to create a variant of try and catch block like syntax known from other languages. It keeps go's current philosophy of handling errors directly where they occur and makes it more succinct to do so.

@networkimprov
Copy link

@tobimensch please post new suggestions to a gist, and link that in the Go 2 Error Handling feedback wiki. Posts on this closed issue may be overlooked.

If you haven't seen it, you might want to read the Go 2 Error Handling Draft Design.

And you might be interested in Requirements to Consider for Go 2 Error Handling.

@mdaliyan
Copy link

mdaliyan commented Aug 23, 2019

It may be a little too late to point out, but anything that feels like javascript magic bothers me. I'm talking about the || operator that should somehow magically work with an error intedface. I don't like it.

@bradfitz bradfitz added the error-handling Language & library change proposals that are about error handling. label Oct 29, 2019
@golang golang locked and limited conversation to collaborators Oct 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests