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: Go 2: the "watch" statement for easier error handling & more #40821

Closed
martinrode opened this issue Aug 16, 2020 · 13 comments
Closed

Proposal: Go 2: the "watch" statement for easier error handling & more #40821

martinrode opened this issue Aug 16, 2020 · 13 comments

Comments

@martinrode
Copy link

@martinrode martinrode commented Aug 16, 2020

Go 1

When I started with a Go a few years ago, I was forced to wrap my head around the error management as it is today in Go. Being used to throw, try, catch and friends in other languages it really felt weird to work with errors as values.

Today, I think it is a great thing that Go did not give way to the pressure to add something like try...catch.

Don't get me wrong, I like the way errors are handled as values, I think it is better than any of the alternatives which I heard of over the last two years.

However, working a lot with stream encoding and databases it can become an annoyance to spend three extra lines after each call into a library just to make sure all errors are dealt with. It makes it hard to read and follow the "Happy Path" of the code.

// Example snippets from code we are using, the infamous `if err != nil` on speed dial...
// This is not meant to compile, just to show examples of what we are facing today in
// a lot of real code which calls into packages like encoding/* or database/sql.
function fn() (err error) {
	err = enc.EncodeToken(start)
	if err != nil {
		return err
	}
	err = enc.Encode(headerStuff)
	if err != nil {
		return err
	}
	// imagine loops and more stuff here
	err = enc.Encode(bodyStuff)
	if err != nil {
		return err
	}
	err = enc.Encode(footerStuff)
	if err != nil {
		return err
	}
	err = enc.EncodeToken(start.End())
	if err != nil {
		return err
	}
        return nil
}

Go 2

So, after the dismissed proposal of introducing the idea of the built-in try (which I did not favor, as it try(made try(code try(much))) harder to read) , I am suggesting a new concept of the watch statement.

watch

The watch statement takes one or more comma separated variables as argument. It executes its code block when a new value is assigned to any of the watched variables.

Think of it as a sibling to defer, defer executes right before any return in the same function, watch executes right after a new value is assigned to the watched variables.

So the code above would look something like this:

function fn() (wErr error) {
	watch wErr {
		if wErr != nil {
			// Log your error here...
			return wErr
		} 
	}
	wErr =! enc.EncodeToken(start)
	wErr =! enc.Encode(headerStuff)
	// imagine loops and more stuff here
	wErr =! enc.Encode(bodyStuff)
	wErr =! enc.Encode(footerStuff)
	wErr =! enc.EncodeToken(start.End())
        return nil
}
  • watch would only be allowed at the top level of a function (like defer)
  • watch can watch one or more variables (separated by comma)
  • Variables need to be declared before the watch statement, this cannot include globals
  • Assignments to variables need to have an explicit =! for the watch to run
  • Multiple watches can be defined in one function, they can watch the same variables, run in FIFO or LIFO
  • Watch assignments must be in the same function (TBD)
  • As a convention watched variables could start with a small case w
  • When the function returns the variables are not longer being watched (before "defer" is run)
  • watch is not limited to be used just with errors

My own opinion

I know that this is a little weird to read at first (the code above can return at any wErr assignment), but I think I would get used to this very quickly and it would save a lot of repetition and would let one focus more on the "Happy Path" in your code.

If this idea is embraced, we could think about allowing watch in more scopes, may be even on the package level...

Please comment

I would love to hear everybody's opinion on this. What could you imagine to use watch for, if not errors?

@davecheney
Copy link
Contributor

@davecheney davecheney commented Aug 16, 2020

It makes it hard to read and follow the "Happy Path" of the code.

I disagree. It is easy to follow the happy path of the code. In well written Go code the happy path proceeds along the left hand side of the screen towards the bottom of the function.

@martisch martisch changed the title Proposal for Go 2, the "watch" statement for easier error handling & more Proposal: Go 2: the "watch" statement for easier error handling & more Aug 16, 2020
@gopherbot gopherbot added this to the Proposal milestone Aug 16, 2020
@gopherbot gopherbot added the Proposal label Aug 16, 2020
@martisch
Copy link
Contributor

@martisch martisch commented Aug 16, 2020

The proposal makes the "normal" control flow (not including panics) that previously was easy to detect by looking at return statements now hidden. Any assignment to a variable that is under "watch" can now exit the function.

This proposal I think shares similar properties with the check/handle proposal listed in #40432
The check part in this proposal instead of explicit is implicit on every variable assignment.

There also a bunch of open question about behaviour that would need to be clarified:

  • do map deletes or the copy builtin trigger the watch?
  • what if a pointer to a variable is passed into another function. Is changing the value of the variable then triggering the watch even while in the other functions?
  • does another watch trigger when a value is assigned to a variable that is watched inside the watch itself?
  • can multiple watches be defined in a function?
  • can multiple watches be defined on the same variable?
  • are variable assignment in defers still watched? what defers are still executed if a watch triggers in defer?
  • what happens if a function closure is returned containing an assignment to a watched variable? Is the watch still active in the closure when returned or passed to a leave function, when called in the same function?
  • for multiple assignments (f1, f2 = ....) and a watch on f1, f2. Is the watch triggered once or twice?
@martinrode
Copy link
Author

@martinrode martinrode commented Aug 17, 2020

The proposal makes the "normal" control flow (not including panics) that previously was easy to detect by looking at return statements now hidden. Any assignment to a variable that is under "watch" can now exit the function.

This proposal I think shares similar properties with the check/handle proposal listed in #40432
The check part in this proposal instead of explicit is implicit on every variable assignment.

Thanks for your feedback! I thought about this, and you are right. Its way better to make it explicit, so I changed my idea and updated my comment.

With my change, most of your concerns are adressed:

There also a bunch of open question about behaviour that would need to be clarified:

  • do map deletes or the copy builtin trigger the watch?

No

  • what if a pointer to a variable is passed into another function. Is changing the value of the variable then triggering the watch even while in the other functions?

No

  • does another watch trigger when a value is assigned to a variable that is watched inside the watch itself?

No, watch assignments must be in the same function as the watch.

  • can multiple watches be defined in a function?

Yes, I don't see why not. Same as with defer, they could run LIFO or FIFO.

  • can multiple watches be defined on the same variable?

Yes.

  • are variable assignment in defers still watched? what defers are still executed if a watch triggers in defer?

No, defer runs a different function, plus the watch ends before defer runs

  • what happens if a function closure is returned containing an assignment to a watched variable? Is the watch still active in the closure when returned or passed to a leave function, when called in the same function?

No, watch ends before defer and the updated idea requires the watch assignment in the same function as the watch.

  • for multiple assignments (f1, f2 = ....) and a watch on f1, f2. Is the watch triggered once or twice?
    That would run once (per watch assignment)
@martisch
Copy link
Contributor

@martisch martisch commented Aug 17, 2020

With my change, most of your concerns are adressed:

To me now it looks even more like the abandoned handler/check proposal: https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling.md

Could you please clarify what the main differences are that makes this proposal stand out it utility and avoiding problems vs the above proposal. This would help the discussion about the intent of the proposal and not reintroducing proposals that are (apart from minor details) the same. Minor details may be important so it is nice to understand the thought process behind them.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 18, 2020

Has some similarities to #32795, #32804, #33002, #33266, and #33387.

@martinrode
Copy link
Author

@martinrode martinrode commented Aug 22, 2020

In reply to @martisch

In brief, the check/handler proposal as drafted introduces:

  • A check applies to an expression of type error or a function call returning a list of values ending in a value of type error
  • Handler(s) is/are invoked with the error, in reverse order of their declaration
  • A default handler is avoiding the infamous if err != nil { return err }

So yes, the idea of watch is quite similar, but I see some advantages:

  • watch is not limited to errors. Personally i think this is a big advantage, think of easier debug code etc...
  • If we introduce check/handler we are leaving the path that errors are just values, it turns errors in something which is specially treated leaving simplicity behind
  • watch is not magically checking the last return value of a function, here the check/handler proposal turns the convention (to use error as the last return value) into a requirement, don't know if that is a good thing
  • watch lets you watch multiple variables at the same time and exactly declare them
  • The scope of the watch block is like with any other block (e.g. if), so it is not called with the only parameter error like the handler is
  • watch acts more like a goto in your code and thus is a imho easier to read understand and follow
  • check/handler introduces two new keywords whereas watch is only one
  • check may change your code flow and return right after that line were its in, and so does !=. but since the exclamation mark by definition tells you to pay attention, i find it easier to remember to be aware of the fact that the function might be left after the execution of that line.
@martinrode
Copy link
Author

@martinrode martinrode commented Aug 22, 2020

Thanks for the list of other proposals @ianlancetaylor, let me quickly summarize them to get a better understanding of the watch proposal.

#32795 with...handle...

  • two new keywords with and handle
  • puts the happy code one indent to the right
  • proposal was closed

#33002 check...handle

  • think try...catch without the throw 🧐
  • confusing syntax, something is checked which is not assigned... even after reading the proposal the third time I did not really get it
  • proposal was closed

#32804 expect

  • looks like watch with the condition built-in
  • has the problem that whenever err changes expect is checked (like watch without !=)
  • not easy to understand if you don't know what it does
  • proposal was closed

#33266 on

  • Like #32804, same problem (checking err is implicit)
  • proposal was closed

#33387 try...except

  • Like #3302, same problems
  • Proposal was closed

So basically, I can see two groups here, one is introducing two keywords (with the indented code problem) and with the implicit error checking problem (#33002 is even doing an implicit secret check of a otherwise voided return value), and the other group where we have some sort of keeping an eye on err, but implicitly.

@martinrode
Copy link
Author

@martinrode martinrode commented Aug 22, 2020

After reading all of the other proposals, I noticed most of them use an example CopyFile, so I tried how watch would look like here:

func CopyFile(src, dst string) (err error) {
        var r, w *os.File

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

	r, err != os.Open(src)
	defer r.Close()

	w, err != os.Create(dst)
	if _, err := io.Copy(w, r); err != nil {
		w.Close()
		os.Remove(dst)
                 err != err
                 // err != errors.Wrap(err, "Copy") is an alternative
	}

	if err := w.Close(); err != nil {
		os.Remove(dst)
                 err != err // same as in the original example
                 // err != errors.Wrap(err, "Close") is an alternative
	}
}
  • The err != err is weird, but why not, the != allows you to easily decorate the error instead with more info
  • Maybe we can allow :!= or change != to =! and allow :=!, saving the declaration of r and f
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 25, 2020

It does not seem like a good idea to reuse !=, which is a binary operator, for an assignment statement. In fact, I don't think we can do it, because a != f() is already a valid expression statement, albeit one that doesn't do anything.

In effect it seems that this proposal is using != as something like the try expression in #32437, with the addition of a handler. So similar complaints arise: this proposal lets code change the flow of control in an obscure way without using a keyword.

The differences between this proposal and earlier proposals that were declined seem fairly subtle.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 1, 2020

Based on the discussion above, this is a likely decline. Leaving open for four weeks for final comments.

@martinrode
Copy link
Author

@martinrode martinrode commented Sep 18, 2020

It does not seem like a good idea to reuse !=, which is a binary operator, for an assignment statement. In fact, I don't think we can do it, because a != f() is already a valid expression statement, albeit one that doesn't do anything.

In effect it seems that this proposal is using != as something like the try expression in #32437, with the addition of a handler. So similar complaints arise: this proposal lets code change the flow of control in an obscure way without using a keyword.

The differences between this proposal and earlier proposals that were declined seem fairly subtle.

I changed my proposal to use =!.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 18, 2020

For what it's worth =! is also ambiguous, as ! is a unary operator. a =! b is a valid Go statement today.

@martinrode
Copy link
Author

@martinrode martinrode commented Sep 18, 2020

I think you guys are right and I like to retreat my proposal. I liked the idea to have a „jump to“ block like „watch“, but it needs a clearer code path when called.

Maybe the idea to treat errors as values is the right way to do. After all I guess we all got used to it anyway over the last years...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.