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: aggregate error handling #34140

Closed
latitov opened this issue Sep 6, 2019 · 10 comments
Closed

proposal: Go 2: aggregate error handling #34140

latitov opened this issue Sep 6, 2019 · 10 comments
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Milestone

Comments

@latitov
Copy link

latitov commented Sep 6, 2019

Aggregate error handling

A lot of people already tried to raise a question to improve close-to-perfect error handling in Go in one way or another, so my apologies that I do it once again! I totally agree to the consensus that it's already good as it is now. However, there's this quite typical case, where the same code is typed all over again, polluting the screen, and making what otherwise would fit in half the screen, to span two screens. Here it is, in shortest form:

	    f, err := excelize.OpenFile("./BestRestaurantsInLA.xlsx")
	    if err != nil {
	        fmt.Println(err)
	        return
	    }
	    // 
	    cellVal, err := f.GetCellValue("Japanese", "A2")
	    if err != nil {
	        fmt.Println(err)
	        return
	    }

or this:

		r, err := os.Open(src)
		if err != nil {
	        	fmt.Println(err)
			return
		}
		defer r.Close()
	
		w, err := os.Create(dst)
		if err != nil {
	        	fmt.Println(err)
			return
		}
		defer w.Close()
	
		if _, err := io.Copy(w, r); err != nil {
	        	fmt.Println(err)
			return
		}

As you can see, the same thing is repeatedly typed again and again. And it's not just typing, it also consumes valuable space on the screen, and disperses the focus of attention.

The proposal:

  1. Aggregate error handling code at the end of the function. The function body would also serve as a block of scope for this (similar as it is done in Ada).
  2. Use "if err != nil { goto fail }" idiom at every place where you may get an error, which you want to be handled in aggregated way. (Or handle it individually, if you wish so).
  3. The "if err != nil { goto fail }" can be substituted by something shorter, e.g. "!!!". This can be anything in fact, but "!!!" is easy to type and it doesn't conflict with existing syntax. This can then be unfolded either by preprocessing of the source code (as I do now), or added to the Go standard syntax in one form or another.

Here's an example how it might look like. Let's take the first code example from above, and re-write it:

	func myf1() {
		f, err := excelize.OpenFile("./BestRestaurantsInLA.xlsx") !!!

		cellVal, err := f.GetCellValue("Japanese", "A2") !!!

		return
	fail:
		fmt.Printf("myf1() failed: %v\n", err)
	}

In effect this would unfold into (be equivalent of) this standard Go syntax:

	func myf1() {
		f, err := excelize.OpenFile("./BestRestaurantsInLA.xlsx")
		if err != nil { goto fail }

		cellVal, err := f.GetCellValue("Japanese", "A2")
		if err != nil { goto fail }

		return
	fail:
		fmt.Printf("myf1() failed: %v\n", err)
	}

Imagine how it would affect the code, if there are not two, but (as it is typically) 4..10 of them.

P.S. God bless the absence of try/catch blocks in Go.

@ianlancetaylor ianlancetaylor changed the title Aggregate error handling proposal: Go 2: aggregate error handling Sep 6, 2019
@gopherbot gopherbot added this to the Proposal milestone Sep 6, 2019
@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels Sep 6, 2019
@latitov
Copy link
Author

latitov commented Sep 6, 2019

It's not that simple though. The specification of "goto" clearly forbids jumping over variable definitions. In the above code, cellVal is defined after preceding "goto fail", so this, as it is, won't work, at least in the current rules of the language.

To make it work, we can either manually make sure that all the variables are defined beforehand, specifically before the very first "goto fail", or make changes to the rules (but not the syntax, and (I think) not the semantics) of how goto works in Go, and allow it to jump over var definitions.

The first would allow us to make it work now. The latter would lead to a better structuring of a program, basically as it is now in Go, not deviate it to the enforcement of where exactly the vars should be defined in a scope. But to implement it, careful analysis of consequences is required.

@latitov
Copy link
Author

latitov commented Sep 8, 2019

To be honest, I am new to Go, and don't know the internals of its compiler, so it's totally unknown to me why authors decided to forbid jumping over var defs with goto. Probably it was specifically to discourage people from arbitrarily using the goto? Because, technically, it already works, but in different construct:

for {
   break
   w, err := os.Create(dst)
}

It works. So, why goto can't? After all, all these implicit definitions are "hoisted" up the scope, aren't they? And this example with for and break works. So, there should be no technical problem for goto doing the same.

By the way, there might be a temptation to use this for/break idiom to handle errors, but it won't work because of scope shadowing. This:

package main
import "fmt"
type myerr int
func (e myerr) Error() string {
	return "myerr error"
}
func main() {
	var err error
	for {
		f, err := sub_f()
		if err != nil {
			break
		}
		fmt.Printf("main() SUCCEEDED, %v\n", f)
		return
	}
	fmt.Printf("main() FAILED: %v\n", err)
}
func sub_f() (int, error) {
	var e myerr = 1
	return 1, e
}

Will not print err value, because it's not assigned outside of "for" scope.

But would you make this change, it will start working the right way:

	for {
		var f int
		f, err = sub_f()

But then it nullifies all the convenience of ":=" syntax.

Returning to the main subject, I am certain that there should be no technical problem for lifting up the restriction of goto jumping over the variable definitions.

@latitov
Copy link
Author

latitov commented Sep 8, 2019

Ughhh... After drinking a cup of tea, I figured it out that my proposal with gotos will suffer from scope shadowing as well. Here's an example:

	func myf1() {
		for {
			// deeper scope, just for example
			f, err := excelize.OpenFile("./BestRestaurantsInLA.xlsx") // outer err is masked
			if err != nil { goto fail } // the err won't be visible at fail section
		}
		cellVal, err := f.GetCellValue("Japanese", "A2")
		if err != nil { goto fail } // and this will
		return
	fail:
		fmt.Printf("myf1() failed: %v\n", err)
	}

The solution for this problem would be the unlikely one to be approved.... To make "err" (or "e" or "_e") an exception of scoping rules, to make it always in the scope of a function, just like labels are.

The moment we do so, not only "goto"-based approach will work, but also for/break idiom as well.

As of now, I am 80% sure that this proposal will be dismissed. Yet, I simply so much like the beauty of the simplified code it might lead to, that I gonna type it here once more.

	func myf1() {
		f, err := excelize.OpenFile("./BestRestaurantsInLA.xlsx") !!!
		cellVal, err := f.GetCellValue("Japanese", "A2") !!!
		return
	fail:
		fmt.Printf("myf1() failed: %v\n", err)
	}

and

		r, err := os.Open(src) !!!
			defer r.Close()
		w, err := os.Create(dst) !!!
			defer w.Close()
		err := io.Copy(w, r) !!!
		return
	fail:
		fmt.Println(err)

Hope some will value it too.

@latitov
Copy link
Author

latitov commented Sep 8, 2019

I just watched a video "Gopherpalooza 2018 - Ian Lance Taylor: Transition to Go 2". In this context, the changes proposed:

  1. "!!!" is just a new keyword, and it won't break any code at all, because there were no identifiers of such form allowed previously. It's completely orthogonal syntax construct.
  2. The fact that scope rules for "err" change won't affect much code, because currently err is anyway handled in local scope.
  3. Allowing goto jump over var defs won't break existing code either, because such behavior isn't used now at all, nor nothing depends on this behavior.

@latitov
Copy link
Author

latitov commented Sep 11, 2019

Here's just another way to handle this (and also an illustration that this bothers not only me). Found on astaxie: Build web application with Golang. Here's a snippet of code:

 func main() {
        dbinfo := fmt.Sprintf("user=%s password=%s dbname=%s sslmode=disable",
            DB_USER, DB_PASSWORD, DB_NAME)
        db, err := sql.Open("postgres", dbinfo)
        checkErr(err)
        defer db.Close()

        fmt.Println("# Inserting values")

        var lastInsertId int
        err = db.QueryRow("INSERT INTO userinfo(username,departname,created) VALUES($1,$2,$3) returning uid;", "astaxie", "研发部门", "2012-12-09").Scan(&lastInsertId)
        checkErr(err)
        fmt.Println("last inserted id =", lastInsertId)

        fmt.Println("# Updating")
        stmt, err := db.Prepare("update userinfo set username=$1 where uid=$2")
        checkErr(err)

        res, err := stmt.Exec("astaxieupdate", lastInsertId)
        checkErr(err)

        affect, err := res.RowsAffected()
        checkErr(err)

        fmt.Println(affect, "rows changed")

        fmt.Println("# Querying")
        rows, err := db.Query("SELECT * FROM userinfo")
        checkErr(err)
        .....

and the checkErr() function is:

    func checkErr(err error) {
        if err != nil {
            panic(err)
        }
    }

It does the job.

But, it's not perfectly efficient, because it needs additional function call each time, just to check a condition. I know, inline functions exist, and probably Go compiler does inlining, I don't know, but still...

It's also an answer to those who say that "there is no cases where there are many error check needed in sequence", or something like that. Here are 6 calls to checkErr() function. Six of them.

@deanveloper
Copy link

deanveloper commented Sep 12, 2019

I would like to mention that this breaks a proverb of Go - "errors are values". Errors in Go do not have any special treatment other than being a predeclared interface.
This also forces functions to only have one handler, which may not be ideal.

it's totally unknown to me why authors decided to forbid jumping over var defs with goto.

It's to prevent this case:

goto end
x := 5
end:
fmt.Println(x) // what gets printed? x was never defined

Any time you jump forward in a block with a goto, this is an issue. break and continue don't suffer from this, as they both jump to the beginning/end of a respective block, so any variables defined in said block can be disposed of and redeclared as needed.

"!!!" is just a new keyword, and it won't break any code at all, because there were no identifiers of such form allowed previously. It's completely orthogonal syntax construct.

If this is made a keyword (or an operator), it would technically be a breaking change, as !!! bool is valid syntax (as shown here). I'm not sure if the Go team is worried at all about breaking that, however.

@latitov
Copy link
Author

latitov commented Sep 12, 2019

Hi @deanveloper, I actually tried it. To make it compilable, let's transform it to its equivalent form:

func main() {
	var x int
	fmt.Println(x) // what gets printed? x was never defined
}

So, what get's printed? This:

0
Program exited.

I'll explain further. The Go already has clear definition of what defined but not initialized variables hold, a "zero values". Here it is, right from the Tour of Go:

Zero values
Variables declared without an explicit initial value are given their zero value.

The zero value is:

  • 0 for numeric types,
  • false for the boolean type, and
  • "" (the empty string) for strings.

I think that the decision to prevent goto from jumping over var defs, was someone's impulsive temptation "to make it a little more safer". But you can't make it totally idiot-safe, or otherwise it'll lose all expressive power and become a dull language-for-itself. And still it won't be totally safe against programmers who don't know what they are doing. There were many efforts towards that, e.g. Ada, e.g. Java. The result? Here's Go.

@deanveloper
Copy link

Hi @deanveloper, I actually tried it. To make it compilable, let's transform it to its equivalent form:

that is not it's equivalent form.

goto printLbl
x := 5
printLbl:
fmt.Println(5)

that does not compile because you skip over the x := 5. := defines a variable, so x is never defined fmt.Println runs.

Variables in Go are not hoisted like they are in some other languages (ie JavaScript), so you cannot use variables until they are defined. If the definition is skipped over because of the use of a goto, then the variable does not get defined.

I'll explain further. The Go already has clear definition of what defined but not initialized variables hold, a "zero values". Here it is, right from the Tour of Go:

The issue is not that the variable does not get initialized, it's that the variable does not get defined. For instance, the following compiles fine:

var x int
goto last
x = 5
last:
fmt.Println(x) // prints 0

This is because x gets defined outside of the goto.

@latitov
Copy link
Author

latitov commented Sep 12, 2019

Too bad. I thought they are hoisted.

@latitov
Copy link
Author

latitov commented Sep 12, 2019

I took a closer look into it.

The lang ref doesn't say a word if it is hoisted or not, I wasn't able to find there any clarification.

The implementation doesn't do hoisting, though, that's the fact. @deanveloper was right. This doesn't compile:

	fmt.Println(x) // what gets printed? x was never defined
	var x int

What can I say? Too bad. I think that making Go hoist variables would be too much re-design work and effort, to just allow save someone, here and there, sporadically, few lines of code. I think that it should have been designed properly from the beginning. Still, I am sincerely glad that Go as it is exists.

And I close this issue.

@latitov latitov closed this as completed Sep 12, 2019
@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 Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

5 participants