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: Go2: Drying up Error Handling in Go #36284

Closed
ghost opened this issue Dec 26, 2019 · 11 comments
Closed

Proposal: Go2: Drying up Error Handling in Go #36284

ghost opened this issue Dec 26, 2019 · 11 comments
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@ghost
Copy link

ghost commented Dec 26, 2019

Introduction

We are still experimenting with possible ways to simplify error handling in future releases of Go. I believe that most frustration comes from the repeating of simple error cases, and that we can introduce a simple syntax to help remove this tedious code. The new syntax is inspired by existing Go conventions, will not break existing code, and should feel familiar to the community.

The Proposal

I see there are 4 main cases of errors in Go that we handle as developers.

  1. Ignore the error and continue function execution
  2. Halt function execution and return the error as is
  3. Halt the function execution and return the error via a handler func, or an error wrapper
  4. Handle the error gracefully and continue execution of the function

Case 1

We already have special syntax for case 1. There are no changes here.

// Ignore the error and continue
val, _ := foo()

Case 2

Inspired by the _ operator, I propose a similar syntax for returning errors if they exist.

// If err, halt function and return err
val, ^ := foo()

Case 3

For case 3, it is important that we allow developers to wrap their errors. In many cases, the wrapper or error handler is simple enough to factor into an external function. We must be mindful of which handler is being used for the error, but we still want to clean up the if statement that hangs out below the source of the error. For this case, I'm proposing we use existing channel syntax to register an error handler with the ^ operator, and return the error through the handler.

This pattern is inspired by how we typically remove else statements from our code by beginning with the else case and checking the if case afterward :

^ <- handleErr // Set error handler to func handleErr(err Error) Error
val, ^ := foo() // If err, halt function and return result of the registered handler

^ <- nil // Set the error handler to No error handler (default)
val, ^ := bar() // If err, halt function and return err as is

As you can see above, registering a handler will persist until a new handler is registered. This is necessary to eliminate repeating ourselves if we have a logger that is used for most errors.

Case 4

This final case is our catch all. It allows developers to gracefully handle their errors and provides backward compatibility with the existing convention. There are no changes here.

// If err, handle and continue gracefully
val, err := foo()
if err != nil {
    handle(err)
}

Dependency Injection

One issue that is immediately obvious is the lack of additional arguments in the error handler. To inject additional dependencies, we would have to produce the handler as the result of a closure or use a package such as the popular wire package. I think that using a Handler constructor for dependency injection would be elegant enough to inline as in the following

^ <- makeHandler(dep1, dep2)
val, ^ := foo()

Restricting the Operator to Errors

Because errors are just values, there's really no reason that this new ^ operator couldn't be used for types that are not errors. It is a "Return val if exists" operator and it accepts a single function on a channel that acts as the error handler.

It's not clear to me that using this operator for any value other than errors would be beneficial in the way that using _ is. I recognize that it could be easy to miss in verbose code, and for this reason, I would recommend it be restricted to types that implement the Error interface. In restricting the operator to only error types, we would also need to restrict the handler func to a handler interface which would be the following :

interface Handler {
    Handle(err error) error
}

Multiple Return Values (Added in edit)

If a non error return value has not been initialized, it would be returned as the default "zero" value as is the current convention. If it has been initialized, it would be returned with its current value which is the case for named return values.

func example() (int, error) {
    ^ = foo() // If err, returns 0, err    
    return 3, nil
}
func example() (i int, e error) {
    i = 3
    ^ = foo() // If err, returns 3, err
    return 5, nil
}

Conclusion

I'm certainly not an expert in the field of language design, so I welcome all comments and criticisms. My favorite feature of Go is its simplicity, and I hope I can help move the conversation around error handling to be about "What is the smallest change we need to solve this problem?"

I believe that the frustration around error handling in Go is around the verbosity of the if err != nil check, and therefore the smallest change we must make is one that eliminates as many of these if checks as possible while still allowing for easy to read, easy to write, and easy to debug code. We don't need to eliminate them all. We just need enough to make Go a little bit more fun to write in.

Thanks for reading!

@gopherbot gopherbot added this to the Proposal milestone Dec 26, 2019
@ianlancetaylor ianlancetaylor added error-handling Language & library change proposals that are about error handling. v2 A language change or incompatible library change LanguageChange labels Dec 27, 2019
@ianlancetaylor
Copy link
Contributor

The suggested use of ^ in case 2 is similar, though not identical, to suggestions made in #22122 and #35644.

I can't recall seeing anything like case 3 before. To me it seems troubling to use channel syntax for an operation that doesn't seem to be anything like a channel.

@drornir
Copy link

drornir commented Dec 27, 2019

@ianlancetaylor how about this?

^ := handleErr

I guess it's also abusing a syntax for purpose it's not supposed to be used

@peterbourgon
Copy link

peterbourgon commented Dec 27, 2019

As the first comment on the corresponding /r/golang post observes, this proposal fails to address the important — and IME overwhelmingly most common — case of uniquely annotating errors in-situ.

@chewxy
Copy link

chewxy commented Dec 27, 2019

I would assume that ^ is just some sugar for returning an error in case 2 and 3.

How would it handle cases likethese:

func returnsTwoErrors() (err1, err2 error) 

func returnsErrorInMiddle() (int, error, bool)

func returnsNoErrors() int 

these three functions require further analysis, and if it's a syntactic sugar over the current grammar, then it also requires an additional desugar phase.

Not too sure if this is a good way forwards.

@urandom
Copy link

urandom commented Dec 27, 2019

@chewxy

Returning 2 errors seems wrong, and is probably very uncommon. I guess you can keep using an if for such a case.

If an error is not the last returned argument for some reason (does that ever happen?) it seems that with this proposal, one can put the ^ anywhere

For the third, I'm hoping that the ^ can just ignore the error returned from the handler function, so that you can write a function that handles the error somehow, returning nil.

@chewxy
Copy link

chewxy commented Dec 27, 2019

leaving in undefined behaviour is a Bad Idea with capital letters.

@ghost
Copy link
Author

ghost commented Dec 28, 2019

TL;DR : Great criticisms! Check out the examples below for my revision to this proposal.

Hey everyone. Great feedback so far here and on the reddit thread. Right now I'm feeling that the core idea still holds promise, but this implementation isn't the right fit for Go. I've been letting it roll around in my head and I'm ready to revise this into some new syntax that addresses the main concerns and more clearly gets at the core of the proposal.

The criticisms so far seem to fall into these general buckets:

Criticism Categories

  1. I don't see that the many occurrences of if err != nil negatively impacts the laguage, so I don't think a change is necessary. No changes!

  2. Using a magic symbol such as ^ and conflating the behaviors of <-, :=, and = breaks Go's orthogonality principle and therefore this syntax needs to be revised. It's un-Go-like to have a single operator behave differently depending on its context

  3. Because errors are values, it's not guaranteed that a function will return only 1 error as its rightmost value. It is not well defined how this proposal will map the error from the early return operator ^ to the return values of the encapsulating function. Especially in the case where there are multiple error values.

  4. If I can't get 100% code coverage in all my programs I'm going to Rust

Responding to Criticisms

  1. I don't think errors should be so totally solved that we will never see another if err != nil check. In fact, I think these checks will be used to handle unique and complicated cases. Cleaning up the trivial and simple cases makes the classic error handling pattern more prominent to the reader. Obviously, no change is the best way to satisfy this critique, but if we're going to see a language change, I think having cases where we still see if err != nil is a good compromise.

  2. I 100% agree that the syntax breaks Rob Pike's original design principle of language orthogonality. I think I can revise the syntax to address this without altering the core idea.

  3. @chewxy This is a great point, and one that really gets at the core idea. I can't get around this issue with syntax alone because errors are just values in Go. Therefore, I'm thinking that a solution to the problem will need to be extended beyond errors and must be usable for any value. As I stated in the original proposal, I don't think the proposed syntax is appropriate for that as I'm afraid it would be abused as a code golfing tool. Either we have to allow for a solution that works for non-errors, or we have to concede that errors are a special value. I hope we can find something for the former case.

  4. I mean... Rust looks like a pretty cool language. More seriously, the right syntactic sugar approach should allow for tooling to accurately trace test coverage in a project

The core idea behind this proposal

What seems to bother people the most is the many occurrences of if err != nil check. The core idea behind this proposal is that if we had some sort of operator or syntactic sugar that would allow us to "return early, if the argument is not nil", we could clean up 80% of the if err != nil checks.

The Revision

Lets scrap the single character magic symbol. What we need is a feature to replace if err != nil { return ... } with "Return early if all of my arguments are not nil". We can't use an inline operator because it would break orthogonality during error assignment. Therefore we need a special built-in function similar to defer. But instead of deferring logic until a return, the new built-in would stop execution and force a response to non-nil values. The best keyword I could come up is respond. I feel like it's related to return but different enough.

The respond built-in would be a variable argument function that forces the return of its encapsulating context when all of its arguments are not nil. To handle the correct mapping of multiple return values, the encapsulating context returns the arguments given to respond positionally. Lets try a few examples and their classic equivalents:

Examples Revised
Lets say we have an encapsulating function that returns an id and an error. The function definition has to deal with error prone dependencies. And to make things weird, I'll break convention and return my error as the left-most value rather than the right-most value.

To make this work nicely, we'll need some help from an errors library. The functions of this library will return an error if given error(s), or nil if given nil errors.

// Classic example
func getId() (error, int) {
	// ...

	val, err := returnsTypicalError()
	if err != nil {
		return errlib.Wrap("Custom err text here", err), 0
	}

	// ...
}

// Converted to use `respond` syntactic sugar
// 0 is not nil. It satisfies the "all arguments are not nil" condition.
// `respond` only forces the return when all of its arguments are non-nil
func getId() (error, int) {
	// ...

	val, err := returnsTypicalError()
	respond(errlib.Wrap("Custom err text here", err), 0)

	// ...
}
// Because we're not handling errors inline, we can deal with non-conventional
// return patterns
func getId() (error, int) {
	// ...

	a, err, b := returnsErrorInMiddle()
	respond(errlib.Wrap("Custom err text here", err), -1)

	// ...
}
// The classic pattern handles multiple errors quite nicely. We can use Go's
// if-scoped variable syntax to combine errors and respond cleanly.
func getId() (error, int) {
	//...

	err1, err2 := returnsTwoErrors()
	if err := errlib.CombineErrors(err1, err2); err != nil {
		return errlib.Wrap("Custom err text here", err), 0
	}

	//...
}

// In the `respond` equivalent, nesting our lib functions would be too verbose
// and difficult to read. We'll have to break the code into two lines just the way
// we would with normal Go code. If our final err is not nil, we'll get an early return.
// If we end up passing a nil error around, we continue beyond `respond`. 
func getId() (error, int) {
	// ..

	err1, err2 := returnsTwoErrors()
	err := errlib.CombineErrors(err1, err2)
	respond(errlib.Wrap("Custom err text here", err), 0)

	// ..
}
// This is a compile time error because the types given to `respond` do not match up
// positionally with the types returned by the encapsulating function
func getId() (error, int) {
	// ..

        v, err := -1, errors.New("test error")
	respond(v, err)

	// ..
}

Sorry that got way longer than I anticipated. But what do you guys think? Anything interesting there?

@ianlancetaylor
Copy link
Contributor

The revised proposal seems to have a lot in common with #32437.

You may want to glance over the various issues, both open and closed, with the label error-handling.

@ghost
Copy link
Author

ghost commented Dec 28, 2019

@ianlancetaylor Well... yeah. This proposal and the try proposal are both attempts at 80% solutions using an early return feature. I think try was a great proposal, but I was uncomfortable with how it encourages the use of named return parameters and the defer statement as an error handler. respond is more generic than try and so the use of named returns and defer shouldn't be necessary (though nothing would be stopping a dev from doing that).

One criticism of try that also applies to respond is that the function doesn't really act like a function. And nesting respond into other functions isn't well defined. It might be more appropriate as a keyword that more resembles return. Something like this:

func getId() (int, error) {
	// ...
	v, err := returnsTypicalErr()
	respond -1, err
	// ...
	return id, nil
}

I'll continue to read more about try and see what else can be applied here. I may close this and move it to a new proposal

@ianlancetaylor
Copy link
Contributor

Hi, we're trying out a new process for language changes. Can you please fill out the template at https://go.googlesource.com/proposal/+/bd3ac287ccbebb2d12a386f1f1447876dd74b54d/go2-language-changes.md .

When you are done, please reply to the issue with @gopherbot please remove label WaitingForInfo.

Thanks!

@gopherbot gopherbot added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 21, 2020
@ghost
Copy link
Author

ghost commented Jan 28, 2020

Thanks. I think that respond has enough merit to look into further. I'll close this issue and consider moving the discussion to a forum as suggested in your linked post.

@ghost ghost closed this as completed Jan 28, 2020
@golang golang locked and limited conversation to collaborators Jan 27, 2021
This issue was closed.
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 WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

6 participants