-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: error handling with functions and an error return? #27567
Comments
Operator ? looks less noticeable than "check". |
@PeterRK you might want to state whether that is a good or a bad thing! I am assuming it is a critique. One advantage of this proposal is that it is not breaking any new ground, but instead following the lead of Rust (but adding a handler component). So we could survey Rust users to see if noticeably of Although I have a preference for |
@gregwebs You are right. Less noticeable may be an advantage. |
@networkimprov you are right I should explicitly talk about how the question mark was mentioned in "considered ideas". In that, the case is made for I hope we can move the conversation from comparing |
There is no doubt in my mind that the next proposal draft from the Go team will add named handlers (or a func type) and drop implicit handler chains, given the feedback to date. It might drop However, that isn't enough IMO. My detailed critique: Golang, how dare you handle my checks! |
I'll say this a million times - I hate exiting from a function without a return. This should never happen unless there is a catastrophic error (panic). Also, returning an error without adding on to the context should be discouraged. The unary The rest of the proposal is interesting, but I'm not sure how much I like the idea of the I also feel like the built-in Using Either way, this shouldn't be about syntax, syntax can be rethought. Let's discuss the idea behind it. I think having different function signatures doing different things is an interesting idea, but I don't like it. I think it makes reading code confusing, especially at the call site ("check site"?) of the handler function. If I'm reading someone else's code, I don't want to have to scroll back up to the top of the function to see if my code continues or not. The nice thing about the I do like this idea though. Those are my critiques, I like the rest of the proposal. The use of anonymous functions rather than handler blocks is a good idea in my book. |
To expand on the whole function signature thing, here's what I mean: func errorProne() error {
handler := func(err error) {
fmt.Println("unimportant error occurred:", err)
}
// 50 lines later
// My internal monologue:
// "Does this function exit if there's an error,
// or does it continue execution?"
f := check(os.Create("Foo"), handler)
} Again, in the |
Error handling is a potential control flow changing point, so we care about it. The design in original draft introduces a new control flow changing rule, what we call "special stacking" or "chained handler". That brings confusion more than convenience. Some guys, include me, suggest to use a new control flow changing mark with normal function as error handler. However, how to implement this mark is controversial. |
A named catch block after check/?/etc does the trick nicely :-)
|
@deanveloper I agree with that @networkimprov @gregwebs I know you guys want to figure out a solution covering all cases. I hope it will be a lightweight one. I think heavy solution is against the philosophy of GO. And the design in original draft is already too heavy to me. |
Error handling consists of handler and trigger.
|
In #27519 (#id/catch model), |
@networkimprov I mean that filter can work with pipe. |
See link I posted above re "critique" for perspective on nesting calls that return error. (A "pipe" is an IPC or stream mechanism btw.) |
Good luck! @networkimprov |
@deanveloper it seems you have misread the proposal. Perhaps I wrote too much, let me know how I can make the section on handler function types more clear. Currently it does state: If you write handler := func(err error) {
fmt.Println("unimportant error occurred:", err)
} When used as a handler, the error will still be passed along (see the section on In this proposal, the usage of |
@deanveloper thanks for critiquing adding |
Ah, I see. You are correct I did misread it, although now there's no explicit exit to the function. (See how that can get confusing to a reader? The function doesn't mark an exit, so I didn't think there was one) Anyway, there shouldn't be a special case to allow a developer to return the error which occurred without additional context. If they want to do that, they should do it explicitly with Also, assuming you want to return the zero value for the other numbers is a dangerous game. For instance, let's say I wanted to write the following function: // Returns how many occurrences of find exist in the UTF8 encoded reader r
func CountOccurences(r io.Reader, find string) (n int, err error) If an error occurs, I don't want to return
Perhaps the handler should always be in the form |
@bmeh (or anyone else that comes along), it would be great if you left your reason for the thumbs down. The proposal has received a lot of useful critiques around the edge cases of language interaction. @networkimprov similarly, it would be great to see critical comments of the core idea here and leave promotions of your proposal on the github issue that is already open for that. |
I posted a link to a pure critique of check/handle, which largely applies to this proposal. It does not mention #id/catch. I urge you to read it. I mentioned a catch block here as a solution to the control flow issue raised above, and used a prefix variation of your |
@deanveloper zero values: thanks for bringing that up. This proposal is essentially for discriminated unions. That is, the non-error value should not exist. I know that use cases do exist for actually returning a tuple of two meaningful values. However, I believe they are rare enough (I have seen thousands of lines of go code that never do this) that it is a mistake to place them as a design constraint on an error handling system. One can still use one of two approaches:
The latter looks something like this: type CountOccurencesError struct {
Count int
Err error
}
func (e CountOccurencesError) Error() string { return e.Err.Error() }
// Returns how many occurrences of find exist in the UTF8 encoded reader r
func CountOccurences(r io.Reader, find string) (n int, err CountOccurencesError) I believe you do need generics to be able to return the concrete type through an error check. |
@deanveloper no unary form of the check: I would be okay with always requiring an error handler that adds context. But I thought always requiring a handler was probably too heavy-handed for a go community that is not already consistently doing that. If you define a function An additional consideration is that some users may be satisfied enough by using stack traces that they don't feel the need to add context in every passing of an error. |
That's not what I'm trying to say here - what I'm saying is that the zero-value of I want to be clear. I don't want the caller to see Most of the time |
@deanveloper sorry for missing your actual concern. I think your level of programming defensiveness is probably appropriate given the lack of discriminated unions in go and the prevalence of zero values. However, it seems not generally applicable (what if I have a |
I personally think that handle is sugar for goto rather than an anonymous function. It seems to be doing this: func something() {
var __reservedVar error
{
errHandle:
return __reservedVar
}
blah, err := x()
if err != nil {
__reservedVar = err
goto errHandle
return
}
} aka func something() {
handle err {
return err
}
blah := check x()
} If you read it like that, the return makes perfect sense. Simplified example. |
This is a very fair point. Although I think that I've said this before, I really like the proposal. It feels very Go-like (at least when using a I added a +1. Sorry if it seems like I'm nitpicking it pretty hard, just want to make sure everything is considered, this is a really good proposal
Yeah I was the same way. I saw Although both views work and I can see it going both ways. I think it personally makes more sense as a goto (it's how it probably works under the hood, AND just works better in general when it comes to how it returns). |
@deanveloper thanks for the 👍 thorough review, and the good questions! @ianlancetaylor is there a process to moving this proposal forward with more reviews? |
There is a relatively slow moving Go2 proposal review process. |
Let me expand on that to say that nothing is going to happen in a hurry. We're going to take the time required to make changes that seem good. |
It sounds like this proposal is suggesting that the func ahandler(err error) { return fmt.Errorf("process: %v", err) }
func process(user string, files chan string) (n int, err error) {
for i := 0; i < 3; i++ {
bhandler := func(err error) error { return fmt.Errorf("attempt %d: %v", i, err) }
do(something()) ? ahandler.ThenErr(bhandler)
}
do(somethingElse()) ? ahandler
} It's not so far fetched to think that if this capability is available, people will naturally want to consolidate their error handlers in the form of a function. When using
With this one it seems unnecessary to do that at all. But does it work? |
@ianlancetaylor I understand that the go team will be thorough and take time with proposals as they should. I am just trying to understand if I am following the proposal process properly. As to your comments on the proposal: I seems my proposal is hard to make it through parts without mis-reading. I think I can fix this by showing how it would actually be implemented in terms of syntax expansion. I will try that out, along with some re-wording, let me know how else I can make things more clear. The We could require that a handler always return an error. As a convenience (and I am open to the possibility that this is a bad idea), we can allow a handler function to not return the error. I call this type of handler a cleanup handler. That just means that we transparently upgrade the cleanup handler to return its error (see the ThenErr appendix section for how this can be done). Yes, f() ? unifiedHandler |
I see, so the handler is passed the error, and can return a modified error, and that error is returned by the function using the (For a language change proposal I find that it's normally best to present the suggested changes by themselves as a standalone document, rather than by comparison to a different change.) |
@ianlancetaylor yes, I can see that the comparison is causing problems for everyone. I will remove it to a gist. I did now write a section "Implementation as syntactic expansion" that should hopefully make things very clear. |
@ianlancetaylor I think you got it now (should be easy to understand with the syntactic expansion section). I cleaned up the proposal by moving out critiques to a separate gist, let me know what else I can do to clarify things. |
I think our key goal is making the consequence of error handling more clear.
|
This is the most natural way for programmers who use the Rust language. There are some risks in introducing the 'check/handle' keywords. |
I would hereby like to follow @freman's line of reasoning and would like to express my dislike of the new Currently, Interesting observation cases:
Current func A() error {
x, err := f()
if err != nil {
return fmt.Errorf("…%v…", err.Error())
}
} proposal of @gregwebs func B() error {
errorHandler := func(e error) error {
return fmt.Errorf("…%v…", e.Error())
}
x := f() ? errorHandler
} The confusion becomes more evident when different formatting functions are called. func A() error {
x, err := f1()
if err != nil {
return fmt.Errorf("…%v…", err.Error())
}
y, err := f2(x)
if err != nil {
return fmt.Errorf("…%v…", err.Error())
}
} proposal of @gregwebs func C() error {
errorHandler1 := func(e error) error {
return fmt.Errorf("…%v…", e.Error())
}
errorHandler2 := func(e error) error {
return fmt.Errorf("…%v…", e.Error())
}
x := f1() ? errorHandler1
y := f2(x) ? errorHandler2
} |
But I want to code in the Go language, not the Rust language - or I'd be coding in the Rust language :) Are our goals to be like other languages? Error handling Go results in the same questions and issues every time a new one of our devs picks it up. Over time, so long as they actually care to improve their craft they usually come to find it refreshing, especially as they learn of better ways of dealing with the errors. Sure it's not entry level easy, and it can be repetitive... but visual basic has If I want more sugar, I can go back to perl where half of what I write in a maintainable way in Go can be squeezed into one line of code. |
Syntax highlighting is never a solution to a problem and a programming language should NOT rely on syntax highlighting to make it readable. Here are a few lines of code peeled from the draft:
15 lines of error handling code, 5 lines of "what we want to do" code. That's why it's visually unappealing - there is way too much boilerplate and code repetition. Go is all about writing programs that do what we want them to do, and it gets a lot harder to do that when we need at least 3 lines of code on every function call that even has a chance of returning an error. |
If so, please tell me, why do so many programmers use syntax highlighting? Because they want to paint beautiful letters pictures?
It seems we do not agree on this point. I think it's an obsession and a relic from other programming languages to think
It seem to me that your use case or expectation for GO may be inappropriate. We dont start to adapt Go for the styling of web pages, as a replacement of CSS. |
Because it's helpful. But that doesn't mean that a language should require syntax highlighting to be readable.
I agree that errors really shouldn't have special treatment in Go. It's nice that in Go errors are just values, but having all of the code repetition that we have is not okay and needs to be addressed. Code repetition leads to many issues, especially when it comes to refactoring. There is no current solution to the code repetition problem for handling error values, and that's what the Again, we only cared about 5 lines of code for that example I showed. But because we wanted to handle errors, we ended up needing to write 15 lines of code specifically to handle if something goes wrong.
When did I say anything like this? CSS isn't programming, and it shouldn't be considered programming. I never said anything about Go doing things outside of programming. What I am saying is that Go is meant to be a language where what we write is what happens when it's run, contrasted to lots of OOP languages which have extremely confusing type systems and constructs. (Also, stop spelling it GO, it's spelled Go.) |
Go is absolutely readable in its current form. I just said, in case it's not for a minority group of people... they could use it... Let's drop this, it does not help the actual Diskursion...
Especially when refactoring functions, I can not grip your problem. Because using your example function err := CopyFile(a,b)
if err != nil {
…
} |
Well, if this is your opinion, I wonder why you want to change this! That's the whole point, let GO as it is! If something needs to be changed, add something useful ... like add generics… but dont rush it…
miiiiaaaaauuuu 😄 |
I meant refactoring the CopyFile function. If I wanted to change the error message, I'd need to remember to change it in all 4 places.
Again, as I said, it really is nice for errors to just be treated as a value, but when we have an issue as bad as bad as how it is now (referencing code repetition), we need to weigh what matters more, and I believe that fixing the code repetition problem is more important than errors "not being special" |
I completely disagree question |
Because now we've got 20 lines of error handling code, while still maintaining the 5 lines of code that we really care about. Sure it helps with error handling, but it doesn't help with the boilerplate issue. Also, it completely falls apart when you want a handler for a specific scope (especially loops!) or if you want to return a different error halfway through the function. Examples of how the defer solution fails:
If we call Under the handler structure, it would instead look like
For the same input and same error location we get a more expected result: |
I also read the proposals, but all examples do little change to my opinion. Both examples you put forward are specifically designed to provoke a constructed problem, knowing that both cases are far-fetched. Moreover, the current standard way is still shorter and more elegant, then your second example. func LoopHandle(arr []string) (err error) {
for _, str := range arr {
if err := f(str); err != nil {
return fmt.Errorf("inloop str(%q) err(%v)", str, err)
}
}
if err := f(); err != nil {
return fmt.Errorf("loop arr(%q) error(%v)", arr, err)
}
return nil
} |
@gomarcus, I encourage you to paste your first comment here into a gist linked from the feedback wiki, as it's mostly a critique of the error handlers concept, not the substance of this proposal :-) @deanveloper, this issue isn't a forum to debate a commenter who took issue with handlers :-) Regarding @gomarcus' critique of handlers, I have tried to document all the possible requirements for a new errors idiom (and thus benefits) on golang-dev. |
Thanks @networkimprov for trying to keep the discussion on track: I would like to keep it focused on the proposal at hand. I see one point that is quite relevant to address: Is it bad to introduce this error handler concept if it doesn't scale down to simple usage? That is, if I don't need a shared handler, would I use this "improved" error handling? This is the benefit of the handler being a second argument to the check: you can write your handler inline without the overhead of naming it or registering it. So my version given in the above examples would still be: func C() error {
x := f1() ? func(e error) error {
return fmt.Errorf("…%v…", e)
}
y := f2(x) ? func(e error) error {
return fmt.Errorf("…%v…", e)
}
} I don't think this is inherently better than doing the traditional But this example is quite contrived as is: the real way you would write it is func C() error {
x := f1() ? prependToError("…")
y := f2(x) ? prependToError("…")
}
func prependToError(format string, v ...interface{}) func(error) error {
return func(e error) error {
v = append(v, e)
return fmt.Errorf(format + ": %v", ...v)
}
} This is why I believe it is important to use normal functions as handlers: function composition is such a powerful tool. It is probably possible to come up with an example more like the first that doesn't benefit from handler helper functions. If go lang had a good anonymous function syntax (type inference and no func C() error {
x := f1() ? fn(e) { fmt.Errorf("…%v…", e) }
y := f2(x) ? fn(e) { fmt.Errorf("…%v…", e) }
} |
Although I can understand the hint of @networkimprov, I refuse the accusation by @gregwebs that my objections had little to do with the proposal being discussed. The basic problems of the proposal remain. |
@gomarcus your objections are real. I only meant that some of them are not specific to this proposal (e.g. suggestions for adding special syntax highlighting, stating that the current verbosity is a non-problem, etc) but apply to essentially any proposal including this one. In that case we would prefer to keep it as general feedback on the wiki and just point to that on new error handling proposals instead of re-hashing the same conversations on every error handling proposal. I did respond to the given code sample critique that was specific to the proposal. I know you did have some other specific points, but unfortunately I wasn't able to make sense of them (code is the clearest expression, nobody else was understanding my proposal until I wrote it in code). |
It’s time for everyone to have a time out and cool off. |
The right way to submit feedback and alternate designs like this is to post it somewhere else (a blog post, Medium, a Gist, etc) and then link it from the wiki page. See https://go.googlesource.com/proposal/+/master/design/go2draft.md. Thanks. |
To everyone else, please note that detailed discussion like this does not belong on the issue tracker. |
Background (go 2 Process)
go 2 has laid out the problem of error handling (Please read before reading this proposal).
I am told that alternative proposals should be raised as git issues here. Please add anyone else you think would like to join the discussion.
Introduction
It is amazing to see the error handling problem being properly addressed. The existing draft proposal is good, but I think we can iterate to make it better.
To avoid confusion by comparison to the existing proposal, I will avoid mentioning it in this one.
However, if you are a supporter of the existing proposal, please separately read my critique of it.
It's useful to take a step back and clearly state what we are trying to do with our implementation:
In the existing go language, I cannot write a handler function which will create an early return from the function.
There are a few approaches that use existing languages features for this control flow:
try!
macro).For sequences with short-circuits, see the errors are values post for how this can be done in go. However, this severely alters how one would write go code.
Proposal: handlers as functions, just a special check
Lets repeat our goals again:
Composition can be handled with ordinary functions that take and return an error.
That means we just need a mechanism to insert a
return
.For early return in my proposal, I will use a question mark operator
?
rather than acheck
keyword. This is for two reasonscheck
, but it functions differently, so this may help avoid confusion.See "Appendix: Operator versus check function" for a discussion on using
?
or acheck
keyword.Implementation as syntactic expansion
expands to
Where
handler
is a function that takes anerror
and returns one.Zero
is the zero value for the (success) value returned before the error, assuming the function returns a single success value. A function that returns 4 values, the last one being an error, would have.This is a simple, easy to understand transformation. It is easy to underestimate the value from being able to understand the usage site without searching for context. I am trying to avoid comparisons to other proposals, but I want to say that none of the others I have seen can be described this simply.
All of the transformation is performed entirely by
?
. It inserts the nil check, thereturn
, and creates the needed zero values. The handler is just a normal function and an argument to?
.For some small convenience in writing cleanup handlers, the return section would actually expand to this:
See the section on handler types and the appendix section on
ThenErr
andToModifyError
.Basic example from the original proposal, re-written
Putting this together, lets re-write SortContents, which wants different handlers in different places.
Let's show another example from the proposal (slightly simplified) that has handler composition:
It is possible to combine handlers in the same way one would combine functions:
Or
The example uses a
.ThenErr
method (see appendix) as a way to compose error handler functions together.Results
?
defer
.Checking error returns from deferred calls
This alternative proposal can support returning errors from
defer
:Notes on error handler function types
To respond to errors we want to do one of two things:
(error) -> nil
or() -> nil
(error) -> error
An error handler function must always have the type of the modifier, but we may not want the extra noise when writing a purely cleanup handler. The question mark operator can accept all forms. A cleanup function can be automatically converted to return the original error that would have been passed to it.
This is also true of helpers that compose error functions such as
ThenErr
.See the Appendix section on
ThenErr
to see how this is implemented.Appendix
Appendix: Handle and anonymous function syntax
This proposal is slightly more verbose than others that introduce a special anonymous function syntax that is lighter-weight and infers types.
Without this syntax, the proposal would read:
I think it is worthwhile to explore having anonymous functions that are lighter-weight.
However, I think this should be usable anywhere rather than just with a single keyword.
But please leave this for another proposal rather than throw it in the mix with error handlers!
Appendix: unary and binary.
The question mark operator can be used as a unary to just return the exception without any handlers running.
something()?
This is equivalent to
I am favoring writing the unary form without any spaces in this case (more similar to Rust), but we should use whatever syntax the community finds best.
Appendix: Handling errors within the handler itself
A cleanup handler may generate a new error that should be propagated in addition to the current error.
I believe this should just be handled by a multi-error technique, e.g. multierr.
Appendix: custom error types
The existing proposal seems like it would cast a concrete error type to the
error
interface when it is passed to a handler.I don't think this proposal is fundamentally different.
I think this issue should be solved by the generics proposal.
Appendix: ThenErr and ToModifyErr
An implementation of ThenErr and ToModifyErr. See the syntax expansion section for how the
?
operator usesToModifyError
.Appendix: Operator versus check function
The original proposal rejected the question mark and gave some reasons why.
Some of those points are still valid with this proposal, and others are not.
Here is another proposal that I believe advocates the same solution proposed in this alternative, but with a
check
function. I would be happy with that as a solution, but below I give my preference for?
.The original proposal had just one argument given to
check
. This alternative favors the question mark in large part because there are now 2 arguments.The original proposal states that there is a large readability difference in these two variants:
However, I think this is a matter of personal preference. Once there is a left-hand-side assignment, the readability opinion may also change.
Now lets add in a handlers and check our preference again.
I believe
?
will be slightly nicer to use due toNote that it is also possible to put all the error handling on the left-hand-side of the error source.
But I prefer keeping error handling on the right-hand-side for two reasons
Appendix: built-in result type
A go programmer that has used Rust, Swift, Haskell, etc will be missing a real result type.
I would like to see a go 2 proposal for discriminated unions which includes a result type.
However, I think both the original proposal and this alternative proposal would work fine with the addition of a result type.
This is because go effectively already has a result type when dealing with errors. It is a tuple where the last member is of type
error
.A future version of go with discriminated unions should be able to use
?
for dealing with a discriminated union result type.Appendix: intermediate bindings for readability
Error handling on the right-hand-side may increase line length undesirably or seem to be easy to miss. Its always possible to use an intermediate binding.
Appendix: left-hand-side
It is possible to support placing the handler on the left-hand-side.
This could make more sense for
check
. One of the ideas behind this would be to emphasize the handler, for example in the case wheref(...)
is an enormous expression (see above section on intermediate bindings which is another way to handle this).Appendix: returning the zero value
This proposal does not allow for the defensive practice of returning
-1
as the success value, along with the error. Where-1
is useful because zero or a positive number are an allowed value in the problem domain, so someone may notice a-1
propagating. I don't think we need to support this use case for a few reasons:uint
).go vet
.Appendix: all proposal examples re-written
Below are the rest of the code snippets shown in the original proposal, transformed to this alternative proposal.
The text was updated successfully, but these errors were encountered: