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

Open
gwax opened this issue Jul 31, 2019 · 10 comments

Comments

@gwax
Copy link

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

@gopherbot gopherbot added the Proposal label 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

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

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

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

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

@creker

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

@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

This comment has been minimized.

Copy link
Author

commented Jul 31, 2019

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

@Yanwenjiepy

This comment has been minimized.

Copy link

commented Aug 6, 2019

I don't think it's a good idea, like try.. catch..., it's bad!

@TimSatke

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

commented Aug 27, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.