Proposal: on...return for error handling

[jtlapp]

This is a response to the 2018 request for error handling proposals. My inspiration derives in part from the concluding statement in Liam Breck's response.

I have what I hope is a simple addition that may address some of the discomfort that people have with error handling. I'm new to Go as of this past week, and I absolutely love everything about this language (so far) but how the error handling obscures the primary code path. I like to be able to see the structure and logic of the primary code path at a glance in order to help me clearly understand and evaluate its behavior.

Just to set the right expectations, I do not believe this proposal offers any functional benefit over existing error handling. Its purpose is purely to visually de-emphasize error handling in order to make the the primary code path more cognitively available, while still abiding by the code formatter's existing rules. I am however suggesting a new formatting rule for a new syntactic sugar syntax.

I propose a new statement having one of the following three forms (we'd pick one):

• on <boolean-expression> return <optional-value>
• on <reference> return <optional-value>
• on <error> return <optional-value>

The proposal only addresses error handling that results in exiting the function, because I think code can generally be structured to fit this paradigm. I progressively modify the proposal as I go so you can see my thinking and the various alternatives.

Let's look at how we can clear up the following code, which I took from the above RFP:

func CopyFile_Original(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)
	}

	return nil
}

We start with an equivalence of the following two statements:

• on <boolean-expression> return <optional-value>
• if <boolean-expression> { return <optional-value> }

Here is how this looks, but mind you, I have more suggestions to come:

func CopyFile_OnBoolReturn(src, dst string) (err error) {
	var localErr = func() error {
		return fmt.Errorf("copy %s %s: %v", src, dst, err)
	}

	r, err := os.Open(src)
	on err != nil return localErr()
	defer r.Close()

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

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

	if err := w.Close(); err != nil {
		os.Remove(dst)
		return localErr()
	}

	return nil
}

I moved the fmt.Errorf() out to a closure and used it as the return value for the two on...return statements. We could easily have done this by using the equivalent if statement code, but the code formatter would expand the if statements and clutter up the primary code path. The code formatter needs to know when not to expand the statement, and the on...return statement tells it just that.

Moreover, we always want the code formatter to expand if statements across multiple lines, because when we see an if statement heading a line all by itself with no indented lines following, warning lights signal in our mind that there is something wrong and needing correcting. Having these signals in our mind while trying to read the primary code path defeats the purpose of trying to make the primary code path more cognitively available. Hence, we would need a different keyword (on) that does not expect subsequent indentation.

But the err != nil is also repetitive and distracting, so we might instead follow on with a reference such that the return statement only executes when that reference is not nil. Now we have this:

func CopyFile_OnNotNil(src, dst string) (err error) {
	var localErr = func() error {
		return fmt.Errorf("copy %s %s: %v", src, dst, err)
	}

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

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

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

	if err := w.Close(); err != nil {
		os.Remove(dst)
		return localErr()
	}

	return nil
}

However, we still have the multi-statement handlers distracting us from the primary code path. We can address that with another vanilla Go closure:

func CopyFile_OnNotNilAndHandler(src, dst string) (err error) {
	localError := func() error {
		return fmt.Errorf("copy %s %s: %v", src, dst, err)
	}

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

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

	handler := func() error {
		w.Close()
		os.Remove(dst)
		return localErr()
	}
	_, err := io.Copy(w, r)
	on err return handler()

	if err := w.Close()
	on err return handler()

	return nil
}

Now things are looking pretty clean, but we still have error handling statements mixed in with the primary code path, distracting us from the primary code path. One way of dealing with that is to parenthesize them, though I'm not sure how fond I am of this approach:

func CopyFile_OnNotNilAndParentheses(src, dst string) (err error) {
	localError := func() error {
		return fmt.Errorf("copy %s %s: %v", src, dst, err)
	}

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

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

	handler := func() error {
		w.Close()
		os.Remove(dst)
		return localErr()
	}
	_, err := io.Copy(w, r)
	(on err return handler())
	if err := w.Close()
	(on err return handler())

	return nil
}

Another way to deal with this is to allow on...return to extend an existing statement. Perhaps we would require that the on reference be assigned in the preceding portion of the statement. Now we have this:

func CopyFile_OnReturnExtension(src, dst string) (err error) {
	localError := func() error {
		return fmt.Errorf("copy %s %s: %v", src, dst, err)
	}

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

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

	handler := func() error {
		w.Close()
		os.Remove(dst)
		return localErr()
	}
	_, err := io.Copy(w, r) on err return handler()
	err := w.Close() on err return handler()

	return nil
}

This approach assumes that the initial portions of the statements are short, which will often not be the case. We can deal with this by having the formatter indent on...return on the next line:

func CopyFile_OnReturnContinuation(src, dst string) (err error) {
	localError := func() error {
		return fmt.Errorf("copy %s %s: %v", src, dst, err)
	}

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

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

	handler := func() error {
		w.Close()
		os.Remove(dst)
		return localErr()
	}
	_, err := io.Copy(w, r)
		on err return handler()
	err := w.Close()
		on err return handler()

	return nil
}

I think the primary code path is pretty clear in the prior two examples, and the error handling code is right where it belongs as well.

To prevent abuse of on...return, we might restrict its reference to just the error type, assuming the reference approach is preferred over the boolean approach.

FYI, I briefly considered allowing the following construct, where the on statement need not always return from the function, but decided it might invite abuse:

	if _, err := io.Copy(w, r); on err {
		w.Close()
		os.Remove(dst)
		return localErr()
	}

(Apologies in advance if I'm misunderstanding something about Go, as I'm still a newbie.)

[DeedleFake]

I don't see much benefit to this. It saves a few characters here and there, sure, but is it that much better to have a syntax sugar for an if statement that's specific to error handling just for that? It kind of seems like overkill. As you yourself point out, there's little difference between on err return fmt.Errorf("do something: %w", err) and if err != nil { return fmt.Errorf("do something: %w", err) }. There was even a proposal before, #33113, to have gofmt turn error checks with a single line inside the if's block into a one-line statement.

[jtlapp]

@DeedleFake, thanks for the response. I would agree that there is no functional benefit. But I'm not after functional benefit. I'm after cognitive benefit, and I believe the way this approach visually de-emphasizes error handling and visually emphasizes the primary code path addresses my biggest concern: I want to know at a glance what code is intended to do, without having to cognitively filter out the error handling. (Also, I actually argue against the approach of #33113, but thanks for the relevant reference.)

[DeedleFake]

Oh, one other issue: New keywords are generally not backwards compatible. Breaking the compatibility guarantee for relatively minor syntax sugar is not likely to happen.

[jtlapp]

I've discovered a functional benefit of this approach. We can rewrite the following:

	var req *http.Request
	var res *http.Response

	if req, err := http.NewRequest("GET", loginURL, nil); err != nil {
		return fmt.Error("failed to create login page request: %v", err)
	}
	if res, err := http.DefaultClient.Do(req); err != nil {
		return fmt.Error"failed to receive login page response: %v", err)
	}
	cookies := res.Cookies()
	...

Without initial declarations, as follows:

	req, err := http.NewRequest("GET", loginURL, nil)
		on err return fmt.Error("failed to create login page request: %v", err)
	res, err := http.DefaultClient.Do(req)
		on err return fmt.Error"failed to receive login page response: %v", err)
	cookies := res.Cookies()
	...

UPDATE: With the if notation, we need the initial declarations because req and res are needed outside the if blocks. With on, req and res can be declared at first use with function scope.

[ianlancetaylor]

Has some similarities to #32611, #32848, #32946.

As a technical matter

	req, err := http.NewRequest("GET", loginURL, nil)
		on err return fmt.Error("failed to create login page request: %v", err)

is going to have some trouble with semicolon automatically inserted at the end of the first line (see https://golang.org/ref/spec#Semicolons), but there may be some way around that.

[ianlancetaylor]

See #40432 for some general discussion.

[jtlapp]

@ianlancetaylor, thanks for the note and the status update. I was looking for an update!

I'm not sure the semicolon need be an issue, because these could logically be separate statements, with the formatter indenting on statements only for clarity.

This note about simplifications of err != nil seems relevant: "they don't reduce the boilerplate enough to make it worth changing the language..."

[jtlapp]

Here's a variation of the proposal that reduces boilerplate a bit more and doesn't introduce any new keywords, but which may have a semicolon problem:

	req, err := http.NewRequest("GET", loginURL, nil) else
		return fmt.Error("failed to create login page request: %v", err)
	res, err := http.DefaultClient.Do(req) else
		return fmt.Error("failed to receive login page response: %v", err)
	cookies := res.Cookies()
	...

[jtlapp]

I'm closing this. @ianlancetaylor mentioned several proposals that were basically variations of this one and they all ultimately died.

[jtlapp]

Thank you everyone for your help with evaluating this. I'm impressed with how much effort you gave me!