Navigation Menu

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: use symbol or function call in assignment for error handling #33150

Closed
ctlod opened this issue Jul 17, 2019 · 19 comments
Closed
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange Proposal Proposal-FinalCommentPeriod v2 A language change or incompatible library change
Milestone

Comments

@ctlod
Copy link

ctlod commented Jul 17, 2019

The final form of this proposal can be found further down in #33150 (comment)


Use ! (or some other symbol) as special symbol in assignment for error handling

My issues with a built-in function try as proposed is 4 fold.

  1. Excess of function calls all over the place.
  2. Allows chaining, exasperating the above point.
  3. defers should not be required to decorate an error.
  4. I don't use go full time, and the example code I've seen just felt messy.
func MyFunc() (error) {
  f, err := os.Open(filename)
  if err != nil {
    return …, err  // zero values for other results, if any
  }
}

can be simplified to

func MyFunc() (error) {
  f, ! := os.Open(filename)
}

This resembles the _ key symbol, but is only applicable to errors.

HandlerFunctions: func myHandler([...]) error
Takes at least 1 argument compatible with the type of error returned.

eg:

func myHandler(err error, x string) error {
  return fmt.Errorf("copy %s : %v", x, err)
}

And used as so:

func MyFunc() (error) {
  filename := "myFile"
  f, myHandler(!, filename) := os.Open(filename)
}

this is equivalent to:

func MyFunc() (error) {
  f, err := os.Open(filename)
  if err != nil {
    return myHandler(err, filename)
  }
}

Although, in this simple case, it would suffice to do:

func MyFunc() (error) {
  filename := "myFile"
  f, fmt.Errorf("copy %s : %v", filename, !) := os.Open(filename)
}

With support for error with return values:

func MyFunc() (error) {
  filename := "myFile"
  f, myHandler(!, f) := os.Open(filename)
}

I'd expect most function calls using ! to remain rather compact.

Special case, but probably marginal.

func MyFunc() (error) {
  f, panic(!) := os.Open(filename)
}

And of course for the cases where an error doesn't imply an immediate return, then the existing "if err != nil" remains appropriate, as flow is continuing after the function call statement, and the error is treated as nothing more than a variable.

Thanks for your time parsing this late and naive suggestion.

@gopherbot gopherbot added this to the Proposal milestone Jul 17, 2019
@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels Jul 17, 2019
@ianlancetaylor
Copy link
Contributor

Has some similarities to #32884.

@ianlancetaylor
Copy link
Contributor

I'm not fond of the way that the more complex variants put the error handling (the call to myhandler) before the actual work (the call to os.Open).

@ianlancetaylor ianlancetaylor changed the title proposal: Go2 :Use symbol in assignment for error handling proposal: Go 2: use symbol or function call in assignment for error handling Jul 17, 2019
@ctlod
Copy link
Author

ctlod commented Jul 18, 2019

I can understand that. Two function calls with orthogonal purposes on the same line is going to quickly feel like clutter, particularly if the error handling side is longer, and anything that makes the actual work less obvious is a no go.
I feel this would hold true whether the error handling comes on the left or the right of the actual work.

So if we exclude error handling on the same line, maybe all that's left is a handle keyword?

f, err := os.Open(filename)
if err != nil {
  return err
}

f, err := os.Open(filename)
handle err return err

f, err := os.Open(filename)
handle err return decorate(err)

f, err := os.Open(filename)
handle err {
	return decorate(err)
}

Reduces boilerplate, keeps returns explicit but not a significant improvement over a more relaxed format for if statements, for which I think there must already be another proposal somewhere.

f, err := os.Open(filename)
if err != nil { return err }

@ianlancetaylor
Copy link
Contributor

handle err has some similarities to #32611.

@mvndaai
Copy link

mvndaai commented Jul 18, 2019

I like that this proposal restricts the life of the err variable the line it is created.

I dislike that it makes error decoration more complex and forces an implicit return. Sometimes errors should be handled by logging, sometimes they should return, and in a loop they should continue or break.

@ctlod
Copy link
Author

ctlod commented Jul 18, 2019

And a mix of both ! and "handle" (changed to catch, as it feels fitting) would give us:

! denotes that the error is returned, the "catch" allows to name the error variable and control flow for cases that require more than a simple return.

f, ! := os.Open(filename)

f, ! := os.Open(filename)
catch err {
  return decorate(err)
}

f, ! := os.Open(filename)
catch err {
  log(err)
  cleanup()
}

And as it's not an if, gofmt could allow:

f, ! := os.Open(filename)
catch err { return decorate(err) }

or even

f, ! := os.Open(filename)
catch err return decorate(err)

Restricts the scope of the error and reads easily enough but it's both a new key symbol and a new key word, so from what I gather, not popular.

The catch would not have to directly follow the !, but scope would then get interesting.

@mvndaai
Copy link

mvndaai commented Jul 18, 2019

@ctlod I am one of the people that does not want inline if or catch statements. Mat Ryer made a point that if you avoid using else it is easy to see the happy path of a function by ignoring indented code. Inlining an if or catch would remove that ability.

@ctlod
Copy link
Author

ctlod commented Jul 19, 2019

Gofmt could enforce the following if the catch can only ever intercept that one error and is a single line. This is all about minimizing vertical space for what might be considered "unimportant" compared to the actual work.

f, ! := os.Open(filename)
    catch err { return decorate(err) }

Catching multiple errors, or multiline catch, could look like:

func MyFunc() (error) {
    ...
    f, ! := os.Open(filename)
    defer f.Close()
    g, ! := os.Open(otherfilename)
    ...
   catch err {
       ...
    }
}
  • Every ! becomes an auto-magic goto. (How to handle variable declarations? Goto rule too restrictive here)
  • ! is always caught by the next catch in current or outer block.
  • Any function using ! would require at least one catch after the last !. (Or implicit catch err { return err } )

In my opinion, enough proposals around to indicate that using a key symbol is intuitive enough when reducing boiler plate.
The challenge is whether it can be integrated into something that covers most "error" cases in a fashion that clearly outshines the current if err != nil.

@mvndaai
Copy link

mvndaai commented Jul 19, 2019

@ctlod I am weary of multiline catches. It means that decoration is not specific to a function rather than an error. It also stops the ability to have different flows for different errors. Most errors I decorate and return. In a loop I either break,continue, or return. If the error shouldn't stop flow I log and move on.

@donaldnevermore
Copy link

donaldnevermore commented Jul 20, 2019

The ! already means not in !=.

var r, ! = os.Open(filename)

The ! close to the = looks like !=.

@ctlod
Copy link
Author

ctlod commented Jul 22, 2019

@wdc-python-king I really missed the obvious there, and assignment is worse than declaration.
I like the double !! in #32884. I guess of all the standard symbols available, '!' is the one I associate most with "Danger ! ".

r, !! = os.Open(filename)
r, !! := os.Open(filename)

@mvndaai I prefer a multiline catch to an implicit return.
From http://go-database-sql.org/retrieving.html

var (
	id int
	name string
)
rows, err := db.Query("select id, name from users where id = ?", 1)
if err != nil {
	log.Fatal(err)
}
defer rows.Close()
for rows.Next() {
	err := rows.Scan(&id, &name)
	if err != nil {
		log.Fatal(err)
	}
	log.Println(id, name)
}
err = rows.Err()
if err != nil {
	log.Fatal(err)
}

VS

var (
	id int
	name string
)
rows, !! := db.Query("select id, name from users where id = ?", 1)
defer rows.Close()
for rows.Next() {
	!! := rows.Scan(&id, &name)
	log.Println(id, name)
}
!! = rows.Err()

catch err {
	log.Fatal(err)
}

VS

var (
	id int
	name string
)
rows, !! := db.Query("select id, name from users where id = ?", 1)
	catch err { log.Fatal(err) }
defer rows.Close()
for rows.Next() {
	!! := rows.Scan(&id, &name)
		catch err { log.Fatal(err) }
	log.Println(id, name)
}
!! = rows.Err()
	catch err { log.Fatal(err) }

Both would acceptable and have less baggage than the original, but my preference is for the multi-line catch as I feel that it detracts less from the intended purpose of the code.

@donaldnevermore
Copy link

@ctlod Wow, these symbols (! and !!) drive me crazy. How about just catch.

rows, err := db.Query("select id, name from users where id = ?", 1)
catch err { 
        log.Fatal(err)
        return 
}

OR

rows := db.Query("select id, name from users where id = ?", 1)
catch err { 
        log.Fatal(err)
        return 
}

@ctlod
Copy link
Author

ctlod commented Jul 22, 2019

@wdc-python-king The first brings nothing new over the "if err != nil" construct, as for the second, I personally like having a clear indication of where an error might be thrown or else I can't tell if an unfamilier function even has the possibility of returning an error.

The rules I'm using to evaluate my suggestion are as follows: (And they have changed since I first wrote the proposal)

  1. Change must be backwards compatible. (The code dept is just too large)
  2. Any change to assignment/declaration will have to be very simple and very restricted.
  3. Things should be explicit.
  4. Throwing/Catching should not cross function boundaries. (That's what panic() is for)
  5. No handling code on the line where the error occurs. (I want to avoid clutter)
  6. Existing gofmt is not open for rediscussion.
  7. If I can do it with a simple macro, then why does it need to be part of the language?

For the symbols, I feel the learning curve is quite shallow. How long did it take to be familiar with '_' ? After using it a couple of times, do you ever find yourself going back to the specs to lookup what it does?

Out of curiosity, Does '!_' bother you as much as '!!' ?

f, !_ := os.Open(filename)

@donaldnevermore
Copy link

@ctlod Yes, it does. The !_ is unusual, thus I'm not familiar with it. It did take times for me to be familiar with symbols such as _, := and <-, and I definitely won't do that again.

My first example could obey some of your rules.

  1. Change must be backwards compatible. (The code dept is just too large)
    Sure. catch err is similar to if err != nil.

  2. Any change to assignment/declaration will have to be very simple and very restricted.
    Change is very little.

  3. Things should be explicit.
    Explicit catch err.

  4. Throwing/Catching should not cross function boundaries. (That's what panic() is for)
    Catch one function only.

  5. No handling code on the line where the error occurs. (I want to avoid clutter)
    Not sure about this.

  6. Existing gofmt is not open for rediscussion.
    OK.

  7. If I can do it with a simple macro, then why does it need to be part of the language?
    Not sure about macro.

I have a idea. Since ignoring an error is a bad practice, how about this

f, _ := os.Open(filename)

instead of

f, !_ := os.Open(filename)

Will a bad practice become a good one?

@ctlod
Copy link
Author

ctlod commented Jul 23, 2019

@wdc-python-king I'm not a fan of changing the meaning of existing language constructs.

@ctlod
Copy link
Author

ctlod commented Jul 23, 2019

Use !_ (Not Irrelevant) to denote an error that prematurely ends the current function if not nil.

In a nutshell:

var (
	id int
	name string
)
rows, err := db.Query("select id, name from users where id = ?", 1)
if err != nil {
	log.Fatal(err)
}
defer rows.Close()
for rows.Next() {
	err := rows.Scan(&id, &name)
	if err != nil {
		log.Fatal(err)
	}
	log.Println(id, name)
}
err = rows.Err()
if err != nil {
	log.Fatal(err)
}

becomes:

var (
	id int
	name string
)
rows, !_ := db.Query("select id, name from users where id = ?", 1)
defer rows.Close()
for rows.Next() {
	!_ := rows.Scan(&id, &name)
	log.Println(id, name)
}
!_ = rows.Err()

catch err {
	log.Fatal(err)
}

The catch statement declares the variable available within the scope of the catch block, and the blank identifier is legal.

catch _ {
	...
}

The type can be determined at compile time, using type error if multiple types.

To reduce vertical space consumption, a one line variant could be allowed by gofmt in cases where it only catches the preceeding line.

rows, !_ := db.Query("select id, name from users where id = ?", 1)
	catch err { return err }
defer rows.Close()

Some restrictions:

  • !_ can only apply to error types.
  • When using a !_, a catch statement must exist further down in the current or an outer block.
  • A catch will only apply to !_ declarations preceeding the catch in the current or any inner block.
  • !_ is always caught by the first applicable catch statement.
  • A catch with no associated !_ is a compile error.
  • !_ may not be used inside a catch block.
  • Only variable declarations before the first !_ associated with the catch may be used in the catch block.
  • A catch block must return from the function, panic or exit the application.

For errors that do not prematurely end the current function, I'd argue that they continue to be treated just as variables and use the if err != nil approach.

@ianlancetaylor
Copy link
Contributor

Currently !_ is an expression: the unary ! operator applied to the blank identifier _. It's unfortunate to have syntax that can be either an expression or an identifier.

What if we have

func F() (error, error) { return errors.New("1"), errors.New("2") }

func G() {
    !_, !_ = F()
    catch err {
        // What is the value of err here?
    }
}

This doesn't improve the case where different function calls should handle errors in different ways.

This proposal seems to shift for each comment that people make, which makes it harder to evaluate. It might be better to discuss this elsewhere and try to produce a new issue with a more polished version.

For these reasons this particular proposal is a likely decline.

Leaving open for four weeks for final comments.

@ctlod
Copy link
Author

ctlod commented Oct 9, 2019

I would argue that !_ is currently a compiler error? I admit it is a slim argument.

My understanding is that some people (myself included) would appreciate a more concise way to simply exit a function when they get an error, with the option for limited enhancing/logging/cleanup.

For a function that returns 2 error variables, where both are flagged as "return on error", a simple left to right precedence would suffice. I personally don't have an issue with allowing only 1 !_ in an assignment.

For function calls where errors need to be handled properly (instead of a quick exit) then the existing constructs are more appropriate, and for functions where errors imply a quick exit, but the limited enhancing/logging/cleanup differs, I personally don't see any way of making that more concise.

Sorry for the evolving proposal, was not sure on the usual practice.

@bradfitz bradfitz added the error-handling Language & library change proposals that are about error handling. label Oct 29, 2019
@ianlancetaylor
Copy link
Contributor

Thanks for the additional comment. A concise way to return when an error occurs might be nice, but this doesn't seem to be the best way to do it in Go.

@golang golang locked and limited conversation to collaborators Nov 4, 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 Proposal Proposal-FinalCommentPeriod v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

6 participants