Skip to content

Instantly share code, notes, and snippets.

@mathieudevos
Last active July 17, 2019 05:04
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save mathieudevos/2bdae70596aca711e50d1f2ff6d7b7cb to your computer and use it in GitHub Desktop.
Save mathieudevos/2bdae70596aca711e50d1f2ff6d7b7cb to your computer and use it in GitHub Desktop.
Go2 Error Handling proposal: Check/Handle as TryCatch Scoped

Go2 Error Handling Proposal: Scoped Check/Handle

Based on the current proposals and counter proposals in: (Go2ErrorHandlingFeedback)[https://github.com/golang/go/wiki/Go2ErrorHandlingFeedback] I have concerns that the current proposed check/handle does create a code mess where it becomes extremely hard to follow the logic.

My proposal is to add the option to have fully scoped blocks as well as requiring the handle to scoped, and enforce the variable assignment already used in golang. Finally the handle would move to the bottom, to maintain the logic of top to bottom programming logic (which the original check/handle breaks).

A simple example could be the following:

check {
	foo := check FunctionThatReturnsErrorAsLast() // returns var, err
	bar := NoErrorFunction() // returns only value, no error
	check AnotherErrorFunction() // returns err
} handle err {
	// you have the error, so you MUST consume it
	return err
}

Alternative style is accepted as well, however, this only works on top level functions

foo := check FunctionThatReturnsErrorAsLast() // returns var, err
bar := NoErrorFunction() // returns only value, no error
check AnotherErrorFunction() // returns err
handle _ {
	// err != nil, but no err assignment, as such no consumption
	fmt.Println("Saying hi from the handle")
}

Below we have the "basis file" which is used as an example further below.

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)
	}
}

Applying my proposal to the file gives the following:

func CopyFile(src, dst string) error {
	r := check os.Open(src)
	defer r.Close()

	w := check os.Create(dst)

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

or alternatively, a mix of the old style with the new, enabling both:

func CopyFile(src, dst string) error {
	r := check os.Open(src)
	defer r.Close()

	w := check os.Create(dst)

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

or the fully scoped functions:

func CopyFile(src, dst string) error {
	check {
		r := check os.Open(src)
		defer r.Close()

		w := check os.Create(dst)

		check {
			check io.Copy(w, r)
		} handle _ {
			w.Close()
			os.Remove(dst) // (only if a check fails)
		}
		check {
			check w.Close()
		} handle _ {
			os.Remove(dst) // (only if a check fails)
		}

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

This makes generic error handling (non-lazy as opposed to: if err != nil { return err; }) a lot simpler. Furthermore it's clean for the people reading the code since this is perfectly scoped.

As such the requirements are the following:

* Every check must have at least 1 handle below it
* `check` keyword is followed by either `{`, a function that returns an error as last parameter, or a variable of type `error`
* `handle` keyword is followed by a variable name, or `_` for unused variable name
* Checked errors (through the use of check) go STRAIGHT to the next handle block, in order of top to bottom, or "bubble up" in the scope
* handle always picks up an error type, it is either straight jumped to by the check as long as it is within its scope or an error can just continue down there.

The goal is to make the codebase repeat itself less while maintaining a clear flow of function handling, while giving the developers the possibility to handle the errors in whichever way they want.

Discussion

We could suggest the ommittance of the handle block would simple do the following:

if err != nil {
	return err
}

Which can be seen as "lazily" error handling / returning. However, I'll leave this up for discussion.

Issues with current check / handle proposal

If we then look at the check & handle current proposal:

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

	r := check os.Open(src)
	defer r.Close()

	w := check os.Create(dst)
	
	handle err {
		w.Close()
		os.Remove(dst) // (only if a check fails)
	}
	check io.Copy(w, r)
	check w.Close()
	return nil
}

This is where I'm having issues with: the errors are happening at the top, where they return. They do so without a defer statement. Defer cannot and should not return, however, having a return at the top which "handles" your error is just confusing. This also means that your error is lexically scope, going up goes "up" according to your code, as you can see:

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

However, since it follows the whole scope you end up with annoying side effects, such as a requirement to always check all the errors in your code. Because it will follow them all.

This also means that code like this:

func returnE() {
	fmt.Println("Function received an error")
}

func main() {
	handle err {
		return err
	}

	handle err {
		fmt.Println("First error handled")
	}
	check returnE()

	handle err {
		fmt.Println("Second error handled")
	}
	check returnE()
}

Will print the following:

First error handled
Function received an error
Second error handled
First error handled
Function received an error

This is EXTREMELY confusing. Because now you get to take your magnifying glass and try to follow the error through all your code ... Furthermore, we write handle err while we actually do not consume the error, so it is being handled by not handling it ... again confusing.

Changelog

  • Formatting improved to be consistent tabs
  • Value after handle is now the error value, so just _ for not used
  • Added example of only using check { // ... }
  • Fixed misunderstanding about check & handle + added suggestion of check/handle function scoped
  • Moved current check/handle proposal to its own section in the discussion
  • Added fully checked functions to the proposal
  • Moved the scope of the check functionality
@200sc
Copy link

200sc commented Jun 27, 2019

I believe there's a misunderstanding of the existing check / handle proposal here.
Checks do not go down into handles that follow them. In the doc, the examples state It executes all handlers in lexical scope in reverse order of declaration.

In the CopyFile example, if the error isn't nil when we get a file then we don't want to try to close that file, because w could also be nil. The handler following the w := check os.Create(dst) call does not check the error in that statement.

@mathieudevos
Copy link
Author

I believe there's a misunderstanding of the existing check / handle proposal here.
Checks do not go down into handles that follow them. In the doc, the examples state It executes all handlers in lexical scope in reverse order of declaration.

In the CopyFile example, if the error isn't nil when we get a file then we don't want to try to close that file, because w could also be nil. The handler following the w := check os.Create(dst) call does not check the error in that statement.

You are correct, I have updated the correct handling. However, the error is still a problem in many cases.

Let's say you group 5 functions and you have a generic handler for them, as such:

func main() {
  handle err {
    fmt.Println("Generic handler")
    return fmt.Errorf("error message: %s", err.Error())
  }

  check function1()
  check function2()
  check function3()
  check function4()
  check function5()
}

Next up we write a custom return function for the 3 one!

func main() {
  handle err {
    fmt.Println("Generic handler")
    return fmt.Errorf("error message: %s", err.Error())
  }

  check function1()
  check function2()

  handle err {
    fmt.Println("Function 3 handled!")
    return fmt.Errorf("error message: %s", err.Error())
  }
  check function3()
  check function4()
  check function5()
}

But now we have a problem! If function 3, 4, or 5 returns an error, it will print "Function 3 handled!" -> This is the biggest issue I have with this whole thing, to fix it we should need to do the following:

func main() {
  handle err {
    fmt.Println("Generic handler")
    return fmt.Errorf("error message: %s", err.Error())
  }

  check function1()
  check function2()

  handle err {
    fmt.Println("Function 3 handled!")
    return fmt.Errorf("error message: %s", err.Error())
  }
  check function3()

  handle err {
    fmt.Println("Generic handler")
    return fmt.Errorf("error message: %s", err.Error())
  }
  check function4()
  check function5()
}

Which is not much cleaner, doing this with check & handle scopes would give the following:

func main() {
  check {
    function1()
    function2()

    check {
      function3()
    } handle err {
      fmt.Println("Function 3 handled!")
      return fmt.Errorf("error message: %s", err.Error())
    }

    function4()
    function5()
  } handle err {
    fmt.Println("Generic handler")
    return fmt.Errorf("error message: %s", err.Error())
  }
}

(or use the function scoped as suggested in the discussion as following:

check func main() {
  function1()
  function2()

  check {
    check function3()
  } handle err {
    fmt.Println("Function 3 handled!")
    return fmt.Errorf("error message: %s", err.Error())
  }

  function4()
  function5()
} handle err {
  fmt.Println("Generic handler")
  return fmt.Errorf("error message: %s", err.Error())
}

Both of these are cleaner & clearer of what is actually happening. Obviously some of this is shaping the problem to fit the solution, but I hope this clarifies things.

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