Skip to content

Instantly share code, notes, and snippets.

@bhenderson
Last active September 10, 2019 13:26
Show Gist options
  • Save bhenderson/32a86f3315ac109dd0ce5a9884c50f83 to your computer and use it in GitHub Desktop.
Save bhenderson/32a86f3315ac109dd0ce5a9884c50f83 to your computer and use it in GitHub Desktop.
Error handling can already be simplified by introducing single-line conditionals, which were already rejected.

These are my thoughts on the Go 2 draft for error handling.

The example go program they give is:

func CopyFile(src, dst string) error {
	r, err := os.Open(src)
	if err != nil {
		return fmt.Errorf("copy %s %s: %v", src, dst, err)
	}
	defer r.Close()

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

	if _, err := io.Copy(w, r); err != nil {
		w.Close()
		os.Remove(dst)
		return fmt.Errorf("copy %s %s: %v", src, dst, err)
	}

	if err := w.Close(); err != nil {
		os.Remove(dst)
		return fmt.Errorf("copy %s %s: %v", src, dst, err)
	}
}

But I imediately saw two issues, 1) The error formatting can be extracted out to a defer function and 2) We can extract out the close logic

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

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

        _, err = io.Copy(w, r)
        ec := w.Close()
        if err == nil {
                err = ec
        }
        if err != nil {
                os.Remove(dst)
        }
        return err
}

Now, what I see as the remaining problem is people wanting to address the number of lines. I've always longed for oneline conditionals (coming from ruby) The code shortens to

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

        r, err := os.Open(src)
        return err if err != nil

        w, err := os.Create(dst)
        return err if err != nil

        _, err = io.Copy(w, r)
        ec := w.Close()
        err = ec if err == nil
        os.Remove(dst) if err != nil
        return err
}

This is now 1 line shorter than the proposed solution and all we did was remove multi-line conditionals. No new keywords.

We could further simplify by allowing implicit nil checks (I have no idea how hard this would be technically)

func CopyFile(src, dst string) (err error) {
        defer func() {
                err = fmt.Errorf("copy %s %s: %v", src, dst, err) if err
        }()

        r, err := os.Open(src)
        return err if err

        w, err := os.Create(dst)
        return err if err

        _, err = io.Copy(w, r)
        err = w.Close() || err
        os.Remove(dst) if err
        return err
}

(This last point goes a little beyond the draft, but it's also something I've always wanted!)

I also would like to bear in mind something that Rob Pike talked about a while ago (sorry can't find the link) about how programming languages are converging and I want to be careful about making go look more like ruby (or whatever)

I look forward to everyone's feedback.

@barisere
Copy link

barisere commented Sep 7, 2018

Your first refinement of the function is 💯 for reporting errors uniformly in a function, especially for functions that have only one concern.

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

       // Make many errors here...
        return err
}

However, there is a separate draft for error values that covers both error inspection and error formatting. I think the main thrust of the error handling draft is to reduce the amount of program text necessary for robust error handling, for which you propose single-line conditionals. The title of this gist tells us where that will go 😉. Besides being unidiomatic Go, it will require changes to the syntax and the semantics of other features of the language that are orthogonal to error handling (return expression, conditionals, ...). It would also require complementary changes to tooling such as go lint to check for error handling with single-line conditionals, since "errors must be checked". I think adding constructs specific to error handling is much less change to the language.

Also, the goal of the error handling draft is not exactly "to reduce the number of lines". In fact, the stated goal is

For Go 2, we would like to make error checks more lightweight, reducing the amount of Go program text dedicated to error checking...
https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling-overview.md#goals

To me, it seems more about reducing needless repetition (which cannot be handled correctly using named return values and deferred expressions).

Single-line conditionals can be arguably too terse to maintain the following property (from the error handling problem overview).

Both error checks and error handling must remain explicit, meaning visible in the program text. We do not want to repeat the pitfalls of exception handling.
https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling-overview.md#goals

If single-line conditionals were desirable, nothing stops us from writing

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

as

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

Checked expressions and handler functions express the intent in a more declarative way, while remaining explicit.

We could further simplify by allowing implicit nil checks (I have no idea how hard this would be technically)

func CopyFile(src, dst string) (err error) {
        // truncated

        r, err := os.Open(src)
        return err if err

        w, err := os.Create(dst)
        return err if err

        _, err = io.Copy(w, r)
        err = w.Close() || err
        os.Remove(dst) if err
        return err
}

Now this is going the way of JavaScript 🙂
Well, it will also require unnecessary changes to the language, and we may have to start a philosophical exercise in determining what constitutes truth (truthy values vs falsy values).

@bhenderson
Copy link
Author

Thanks @barisere for your detailed response! I'm working on my response, but busy, so hopefully I can respond soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment