cpl/README.md Secret

## README.md

      
    Raw
  

              README.md
            
          
    Feedback

TLDR

In support with slight modifications.

Agree with inverse chained handlers, but they should be discouraged and instead encourage having a single top handler (documentation & examples)

Handler chains may come in useful for adding context that only appears in the middle of a function
Handler chains can be used due to their lexical scope where defer can't be used without some conditional statement and has no way of recovering
Handlers in general can differ from defer by giving you the option to exit/stop the error handling and resume


Keep it simple, no magic/cryptic |, ?, !, #, ^ symbols/operators that hide complexity
Make it obvious

The new system should be obvious when used (making me in favor of having keywords)
Some proposals involve removing a comma (e.g. data, n err := foo()), sounds terrible


Allow recovery from within handler

Make use of an existing keyword (break? fallthrough? continue?) to allow a handler to exit and continue running from where the check reported the error
Without this, any place where errors.Is(err, SomeOtherErr) will end up using the old pattern of if err != nil followed by Is and then a decision which could be continuing and ignoring the error (sql.ErrNoRows, io.EOF)


Chaining functions/methods using check that normally you wouldn't be able to due to having the error returned,  should not be allowed

buffer := check bytes.NewBuffer(io.ReadAll(r))
res := check sql.Open("example", "example").Query("SELECT ...")
Or any other variation


This is not some in-depth design/feedback document, there's possibly edge-cases, Go-designs and popular opinions I have not considered

Prologue

After reading the draft design site and the feedback wiki articles and any other linked materials, I can say without a doubt there is potential for greatness, and plenty of traps along the way.
As highlighted by the draft design problem overview the current system is very repetitive, encourages the most common anti-pattern in Go error handling for newcomers and veterans alike.
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
	}
}
The above code is something you'd expect to see, but in no universe can
if err != nil {
	return err
}
be considered "error handling". Nothing is being handled, state isn't cleaned-up, no relevant information is returned, no attempt at recovery is made.
By no means am I saying this should be frowned upon, there are valid use-cases where all you want to do is yeet the error back to the caller, and you know for sure (or hope) that something somewhere will know to handle it. In some scenarios, even wrapping the error may not be necessary.
The "proper" way to handle errors in the above code, as illustrated in the design draft:
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)
	}
}
This is actually one of my struggles and frustrations with Go error handling in the current system. Responsibility.
Who (which function) is responsible for including the relevant information/context?
Is information/context duplication acceptable? What if the error returned by os.Open(src) already includes the source?
Because of the flexibility and confusion around responsibility, and especially to avoid the very repetitive nature of returning the same formatted string, or applying the same error handling logic over and over and over in the same function, in different places, I usually opted for designing my error handling as follows (in reality/production using a custom error type with defined global variables, not fmt.Errorf, but the idea is the same):
func CopyFile(src, dst string) error {
	r, err := os.Open(src)
	if err != nil {
		return fmt.Errorf("failed os open, %w", err)
	}
	defer r.Close()

	w, err := os.Create(dst)
	if err != nil {
		return fmt.Errorf("failed os create, %w", err)
	}

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

	if err := w.Close(); err != nil {
		os.Remove(dst)
		return fmt.Errorf("failed writer file close, %w", err)
	}
}
The basic idea is that each error check, should (in this design choice) describe the called functions' action and then wrap the error. Then, when calling this function somewhere else in our codebase, the responsibility to include the source and destination filenames, falls to the caller.
func HandleBackupNotebook(notebookPath string) error {
	backupPath := notebookPath + ".bkp"

	err := CopyFile(notebookPath, backupPath)
	if err != nil {
		return fmt.Errorf("failed copy file from '%s' to '%s', %w",
			notebookPath, backupPath, err)
	}
	return nil
}
With this we can keep pushing the information up and up, but it's still not an optimal design. There can be inconsistencies between the error message used in different invocations of CopyFile across the codebase, or it may be skipped altogether.
You could argue that defer can be used to enforce some consistency, but defer has the downside (and sometimes the upside) of being function scoped, and you won't always be able to use the pattern:
defer func() {
	if err != nil {
		// ...
	}
}
Anyhow, some of this is covered by the Errors Value draft and gopkg.in/errgo.v2 or similar. But I think that handle/check has a good place, especially with chaining that is lexically scoped.
So I try to think about the handling aspect of errors, as having the goal of:

Improving

readability
productivity


Maintaining

flexibility
standards


Anything else is/should-be touched by the Errors Value draft and relates to improving the context and reporting of errors.
Feedback

Continue

This is where the design lacks. There is no recovery from the error within a handler. And I believe recovery to be the most important goal of error handling where possible. A good example, is the io.EOF error, which is not an error in some cases, or trying to parse a string into an int and falling back to a default depending on the error, or continuing from a sql.ErrNoRows error.
func CopyFile(src, dst string) error {
	handle handleCopyFileErr {
		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 {
		if err == io.EOF {
			continue
		}
	
		w.Close()
		os.Remove(dst)
	}

	check io.Copy(w, r) // imagine io.Copy returning io.EOF
	check w.Close()

	return nil
}
Conditional

func isTemp(path string) bool {
	return strings.HasPrefix(path, "/tmp/")
}

func CopyFile(src, dst string) error {
	// ...

	handle err; !isTemp(src) { // skip handler if isTemp(src)
		w.Close()
		os.Remove(dst)
	}

	// ...
}
Naming

I'll be nitpicking the naming choice "handle". Handle implies that it handles, but that's not necessarily the case. As seen you can have a handler for cleanup purposes (w.Close and os.Remove(dst)). A better name (in my opinion) would have been something along the lines of catch (but I understand it may cause confusion with the try catch crowd) so maybe a better choice would be detect/observe (Naming things is hard), but I guess handle is fine.
Problems will most definitely occur from people already using the name handle and check for variables (but the same can be said for any imaginable and relevant alternative to handle or check, oh well).
Opinions on other Feedback

The following are just my opinion on the interpretation of other peoples feedback, if I am wrong, let me know.

A comment on go#21161

Hit the nail on the head


Golang, how dare you handle my checks!

Point 2. is very good, and the current design lacks the mentioned recovery
Point 3. I think it's unrelated to the draft, as that falls in the realm of validation, and it's up to the developer if they want to return an error (thus starting the chain) or simply continue with a default or something ,meaning that yes, we'll still have if smth { return Err... } or maybe if smth { check Err...} ?
Point 7. is a good argument against allowing multiple or chained handlers


Thoughts on the Go 2 Error Handling proposal

Makes a great point that chaining is not necessary and that it can always be added at a later stage without breaking backwards compatibility, if its lack is deemed unbearable
In their view, the handler becomes the base for adding context to an error, while defer should be used for cleanup
I agree that the proposed handler chaining adds some mental burden
With all the good points they make, I still think chaining is ok, and as a general rule developers should try and use one handler as much as possible, but I do see the value in having something like defer but lexically scoped


Error Handling Feedback

An interesting proposal to have the handlers be typed, and only trigger a handler with a check based on the checked type
But, lots of extra complexity that encourage even more handlers to be defined


Requirements to Consider for Go 2 Error Handling

I like the idea of named handlers and having control over which handler gets to run next, but overall I believe that is too complicated


Improve error handing using guard and must keywords #31442

Puts way too much pressure on the developer to remember what keyword to use where and how
Involves defer in the equation


Import symbol ! to passthrough errors

I dislike the idea of using some arcane, easy to miss symbol to represent an error handler
Same applies to hunting down !checkErr named functions and trying to find out if any function could potentially be !...


Go Error Handling – Using Closures

Clean implementation using mostly existing concepts, with the addition of the special use case check(f) where f is func(err error) error
It sounds useful to be able to re-use a "handler closure", or even generate one depending on the passed arguments
Dislike the idea and complexity of having a "check statement"
Risks leading to something like this which would make keeping track of what each "check func" does and which func to use where


Summary (returnfrom)

This is madness