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 expect #32804

Closed
integrii opened this issue Jun 27, 2019 · 7 comments
Closed

proposal: Go 2: simplify error handling with expect #32804

integrii opened this issue Jun 27, 2019 · 7 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

@integrii
Copy link

integrii commented Jun 27, 2019

I am here to propose my idea for error handling in Go 2. I do not particularly dislike typing the full err != nil if statement but I understand the problems identified in the problem statement. If a few people think my approach below sounds good, I'll write something for my weblog and follow proper procedures for posting a language change proposal.

I simply propose that we introduce an expect keyword to the language that triggers whenever the expected condition occurs on the specified variable. The variable scope would be limited to the function codeblock as to not conflict with globals and the variable can not be a property on a method if being invoked as such.

Furthermore, these expect statements can stack just as defers do so that we can add on important "back-out" steps as required.

Example time. Before (from the problem statement):

func CopyFile(src, dst string) error {
	r, err := os.Open(src)
	if err != nil {
		return err
	}
	defer r.Close()

	w, err := os.Create(dst)
	if err != nil {
		return err
	}
	defer w.Close()

	if _, err := io.Copy(w, r); err != nil {
		return err
	}
	if err := w.Close(); err != nil {
		return err
	}
}

After, with expect:

func CopyFile(src, dst string) error {
        var err error
        expect err != nil {
            return fmt.Errorf("copy %s %s: %v", src, dst, err)
        }

	r, err := os.Open(src)
	defer r.Close()

	w, err := os.Create(dst)
	defer w.Close()

        expect err != nil {
            os.Remove(dst)
        }

	err = io.Copy(w, r)
	err = w.Close()
        return nil
}

There are obviously many various ways to make similar functionality, but I feel like if we make a change to the language's error handling, this one is the closest to what I feel like is the "Go" way. We don't repeatedly call out that we are trying something on every line and we don't depend on invisible rules like the last parameter being an error. We keep our code blocks small (as you should) and we explicitly tell each code block what to expect and how to deal with it. As more things need doing, we instruct the runtime what else to do when the expected event takes place.

In the example above, we expect err to become not nil at some point, and when that happens, we always return the error up the stack. Additionally, after we have created a dst file, we must also clean that file up. I think this code gracefully conveys that without much magic and in a way that new coders can quickly understand.

@gopherbot gopherbot added this to the Proposal milestone Jun 27, 2019
@laher
Copy link

laher commented Jun 27, 2019

Nice idea.

Nitpick - I think expect seems to imply an expectation that the condition will occur (rather than the exceptional case) ... maybe whenever works ? or something similar?

i.e.

whenever err != nil {
  return fmt.Errorf("copy %s %s: %v", src, dst, err)
}

r, err = os.Open(src)
...

@integrii
Copy link
Author

I like the sound of whenever more than expect, but I have a feeling that will be left to personal taste. With whenever, the program would change to this:

func CopyFile(src, dst string) error {
	var err error
	whenever err != nil {
		return fmt.Errorf("copy %s %s: %v", src, dst, err)
	}

	r, err := os.Open(src)
	defer r.Close()

	w, err := os.Create(dst)
	defer w.Close()

	whenever err != nil {
		os.Remove(dst)
	}

	err = io.Copy(w, r)
	err = w.Close()
	return nil
}

@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels Jun 27, 2019
@ianlancetaylor
Copy link
Contributor

This has some similarities to the recent #32795.

@mmaedel
Copy link

mmaedel commented Jun 27, 2019

Hello I am a newbie....

I do have questions about this proposal...

First hand you allocate the "err" and with subsequent method calls do "reallocations" by _, err := .... How is this possible to handle? I was thinking about some "while err == nil" loop encapsulation to manage the wrapping... but I am not sure. So still in favor of "if e != nil" routines. Thank you

@DisposaBoy
Copy link

This makes it impossible to know what a line of code does without (re-)reading every single line that came before it. Furthermore... the code now looks like a bug, because there's a random err variable that doesn't appear to be used.

@integrii
Copy link
Author

While, I agree with you @DisposaBoy , if a change is going to be made for Go2, I would much prefer this over wrapping my entire codebase in try() functions. My favorite option is to not change the error handling in go.

@integrii
Copy link
Author

Given the unhealthy up/down votes on this, and the fact that I only made this to try and find a better alternative to the try() proposal mentioned in the go 1.13 release draft, I am closing this proposal. I honestly prefer the verbose handling that exists today.

#32825 should stand to represent the silent majority who prefer that we simply do nothing for now.

@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

7 participants