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 try err == nil {} except {} #33387

Closed
gwax opened this issue Jul 31, 2019 · 11 comments
Closed

proposal: Go 2: simplify error handling with try err == nil {} except {} #33387

gwax opened this issue Jul 31, 2019 · 11 comments
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

@gwax
Copy link

gwax commented Jul 31, 2019

Premised on the expectation that people frequently want to execute a series of statements and check for errors after each statement but frequently want to respond to any error in the same way, we can construct a special statement block to execute these repeated checks for us.

Try statements

"Try" statements specify conditional transfer of control to an "except" clause based on repeated checks of a boolean expression within a block. If the expression evaluates to false after any statement within the block, execution is immediately passed to the "except" branch.

TryStmt = "try" Expression Block "except" Block .

Example:

var err error
var people People
try err == nil {
    jsonFile, err := os.Open("people.json")
    jsonBytes, err := ioutil.ReadAll(jsonFile)
    err = json.Unmarshal(jsonBytes, &people)
} except {
    return nil, errors.Wrap(err, "failed to read people.json")
}

The above translates directly to:

var err error
var people People
{
	{
		jsonFile, err := os.Open("people.json")
		if !(err == nil) {
			goto Except
		}
		jsonBytes, err := ioutil.ReadAll(jsonFile)
		if !(err == nil) {
			goto Except
		}
		err = json.Unmarshal(jsonBytes, &people)
		if !(err == nil) {
			goto Except
		}
		goto NoExcept
	}
	Except:
        {
	    return nil, errors.Wrap(err, "failed to read people.json")
        }
	NoExcept:
}

And can be used to replace:

jsonFile, err := os.Open("people.json")
if err != nil {
    return nil, errors.Wrap(err, "failed to read people.json")
}

jsonBytes, err := ioutil.ReadAll(jsonFile)
if err != nil {
    return nil, errors.Wrap(err, "failed to read people.json")
}

var people People
err = json.Unmarshal(jsonBytes, &people)
if err != nil {
    return nil, errors.Wrap(err, "failed to read people.json")
}
@gopherbot gopherbot added this to the Proposal milestone Jul 31, 2019
@ianlancetaylor ianlancetaylor changed the title proposal: simplify error handling with try err == nil {} except {} proposal: Go 2: simplify error handling with try err == nil {} except {} Jul 31, 2019
@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels Jul 31, 2019
@ianlancetaylor
Copy link
Contributor

Has some similarities to #32795, #32804, #33002 and #33266.

@ianlancetaylor
Copy link
Contributor

Note that this requires a change in the scoping rules. In code like

try err == nil {
    jsonFile, err := os.Open("people.json")
}

the err in the block is being declared with :=. Since it is a different block, that means that it is a newly declared variable, and is not the same as the err variable declared in the outer block.

For example

var err error
if true {
    _, err := os.Open("does not exist")
    fmt.Println(err)
}
fmt.Println(err)

That program will print an error and then print <nil>. The outer err variable is not the same as the inner err variable.

So in order for this to work I think that either try will have not introduce a new block scope, or it will have to somehow inject all variables mentioned in the expression into the inner block. Either seems unusual for Go.

@gwax
Copy link
Author

gwax commented Jul 31, 2019

Could the scoping issue be cleaned up by moving the err declaration into the statement? Something akin to:

TryStmt = "try" [ SimpleStmt ";" ] Expression Block "except" Block .

used as:

var people People
try var err error; err == nil {
    jsonFile, err := os.Open("people.json")
    jsonBytes, err := ioutil.ReadAll(jsonFile)
    err = json.Unmarshal(jsonBytes, &people)
} except {
    return nil, errors.Wrap(err, "failed to read people.json")
}

which would then translate to:

var people People
{
	var err error
	jsonFile, err := os.Open("people.json")
	if !(err == nil) {
		goto Except
	}
	jsonBytes, err := ioutil.ReadAll(jsonFile)
	if !(err == nil) {
		goto Except
	}
	err = json.Unmarshal(jsonBytes, &people)
	if !(err == nil) {
		goto Except
	}
	goto NoExcept
	Except:
	return nil, errors.Wrap(err, "failed to read people.json")
	NoExcept:
}

@peterbourgon
Copy link

peterbourgon commented Jul 31, 2019

This proposal doesn't allow unique per-statement error handling (annotation) which I and others consider a requirement.

@creker
Copy link

creker commented Jul 31, 2019

Premised on the expectation

Again and again I see proposals with the same exact premise but don't remember ever needing that kind of error handling in practice. Where does that expectation comes from?

This looks exactly like Swift do/catch and has the same problem for me - it's really rare that I use the same error handling code for multiple call sites. In Swift, lacking any other idiomatic way of handling errors, this forces me to constantly wrap code in do/catch just to add some context. Also the fact that do/try block creates another scope means that any variable declared in that scope is invisible to the catch/except block. That's very annoying and forces you to declare variables outside of do/catch.

@ianlancetaylor
Copy link
Contributor

@gwax That suggested scoping is still not consistent with the rest of the language. Currently a block scope starts at the {. Writing something like if b := true; b {} effectively creates two blocks.

(Also, note that var err Error is not a SimpleStmt; err := error(nil) would work.)

@gwax
Copy link
Author

gwax commented Jul 31, 2019

@ianlancetaylor it sounds like a clean solution, from a scoping standpoint, likely depends on something happening with #377

@tsatke
Copy link

tsatke commented Aug 13, 2019

I liked the Handle-Proposal better than this, where you could define an error handler on the same level as the code being executed. What I am seeing here, is people wrapping whole function bodies and having the same handle for different errors, which, in your case, makes sense, but surely not everywhere.

@ianlancetaylor
Copy link
Contributor

This proposal does not have strong support. The scoping issue remains unresolved. This is a likely decline.

Leaving open for a month for final comments.

@ianlancetaylor
Copy link
Contributor

There were no further comments.

@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 Proposal-FinalCommentPeriod v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

8 participants
@bradfitz @peterbourgon @gwax @creker @ianlancetaylor @gopherbot @tsatke and others