-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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: abort?
statement to bail early on non-nil errors
#63575
Comments
This looks like #46655, with |
|
Not sure what you mean by this. I'm aware of the history, but perhaps you mean it's not a technical issue? From a technical perspective, I've seen the idea of returning from a function when an error is present, but the ones I've seen don't give the programmer much control of the values returned.
Only lines that need to bail early would start with
func (d *decodeState) unmarshal(v any) error {
rv := reflect.ValueOf(v)
if rv.Kind() != reflect.Pointer || rv.IsNil() {
return &InvalidUnmarshalError{reflect.TypeOf(v)}
}
d.scan.reset()
d.scanWhile(scanSkipSpace)
// We decode rv not rv.Elem because the Unmarshaler interface
// test must be applied at the top level of the value.
err := d.value(rv)
if err != nil {
return d.addErrorContext(err)
}
return d.savedError
}
// With abort?
func (d *decodeState) unmarshal(v any) error {
rv := reflect.ValueOf(v)
if rv.Kind() != reflect.Pointer || rv.IsNil() {
return &InvalidUnmarshalError{reflect.TypeOf(v)}
}
d.scan.reset()
d.scanWhile(scanSkipSpace)
// We decode rv not rv.Elem because the Unmarshaler interface
// test must be applied at the top level of the value.
abort? d.value(rv); d.addErrorContext(err)
return d.savedError
} First clause is checked for errors, last clause is what's returned. |
@seankhliao I don't see the similarity with #46655
// Before
func (t *multiWriter) Write(p []byte) (n int, err error) {
for _, w := range t.writers {
n, err = w.Write(p)
if err != nil {
return
}
if n != len(p) {
err = ErrShortWrite
return
}
}
return len(p), nil
}
// After
func (t *multiWriter) Write(p []byte) (int, error) {
for _, w := range t.writers {
abort? n, err := w.Write(p)
if n != len(p) {
return n, ErrShortWrite
}
}
return len(p), nil
} |
I'm sympathetic to Ian's second point above:
I wonder whether this concern would be alleviated by using a postfix expression instead of the Rust underwent a similar transformation in its error handling functionality when it introduced a postfix question mark operator. Instead of saying This discussion also reminds me a bit of Jonathan Amsterdam's 2017 talk on Errors as Side Notes about formatting code to place all error-handling code to one side, so that the main-line control flow can be more easily understood: |
What I mean is: we've rejected similar ideas in the past. I agree that as far as I know this exact idea has not been proposed before.
Thanks for the example. This is quite different from other Go language constructs. |
Rust's I think it almost has to be on the left-hand side if you are going to return multiple values. |
I was imagining that code currently written like this: err := d.value(rv)
if err != nil {
return d.addErrorContext(err)
} could be written as something like this: err := d.value(rv) returniferror d.addErrorContext(err) where the semantics of I am not actually proposing this since I don't think I have internalized the Go design sensibility quite yet, but I found it interesting to think about. |
I think a specific challenge with this particular genre of proposal is that there have been two different levels of disagreement interleaved in all of the previous discussions I've seen:
(Note: I am not arguing either of those points here. For the sake of this comment, I'm neutral on the specific proposal in this issue.) I think some of the "this is similar to previous proposals" reaction might be thinking about it from the perspective of the second kind of objection: those who expressed that they don't think this sort of goal is even desirable see this proposal as equivalent to all of the others because all it's changed is some irrelevant (to them) details about where on the line each clause goes and what delimits them. I think all proposals for shorthands to avoid writing a normal Perhaps it would be helpful to have a broader discussion (in a different place than this one issue) about what parts of "the error handling problem" seem to have sufficient consensus for it to be worth proposing solutions, what parts the proposals group feels have been retreaded enough to rule as "decided", and what parts of the problem space remain murky and perhaps in need of further problem-space research. |
@apparentlymart Thanks for the comment but I don't entirely agree with you. I think there are objections to this specific proposal that go beyond "we don't need any syntax change."
I agree that it's very hard to satisfy everyone. But any syntactic change here would be the biggest change to the language other than generics. It would affect some 1% of all Go lines in existence. It really has to be an excellent change, not just adequate. And if we can't make it excellent, we shouldn't do it. |
I'm sorry that I was unclear. I didn't mean to say that there are no legitimate criticisms of this proposal in particular, and for what it's worth I agree with the criticisms you shared and didn't mention them in my comment only because I was trying to make a broader point and wanted to separate it from my feelings about this proposal. My comment was in response to the debate above about whether this is or is not a duplicate of a previously-rejected proposal. And in particular, that it's hard to say whether this is "different enough" from the previous proposals to constitute new information. It certainly isn't identical to any previous proposal, but it shares many characteristics with previous rejected proposals; it's unclear if what it shares are reasons for the previous proposals being rejected or not. With that said, I can see that my comment could be read as dismissing all of the above dissent as being in my category 2. That was not my intention, and I apologize. |
Sorry for misunderstanding. |
i agree with you,it's good idea!!! |
I felt the same. Was E.g,
where
This may help remove clutter when simply bubbling up the error. If we want to add more context or wrap the error instead, use P.S Please ignore it if this has been proposed & rejected before. |
TBH we are already in that boat. In most functions that do something interesting in go ~60-70% of lines concern error handling. Why is one keyword at the start more distracting than having to mentally skip the majority of the code lines when you are trying to read the flow of the happy path? Having said that, a postfix notation would certainly be more helpful. |
I don't that's quite the right question. Exactly because so many lines of Go handle errors (though in my measurements it's not quite as high as you suggest), we have to be very careful about changing it. It will affect all existing Go code, which is a lot of code. And we really only get one chance at making a change: we can't keep changing so much code. So the question is not "is this proposal a bit better?" It's "is this the best possible proposal? Are there any possible problems with it that we may regret later?" |
I do agree about it perhaps not being the most important question, but to try to answer it anyway: In today's Go, the error handling and the "happy path" are typically separated onto separate lines, so that when scanning along the left edge of the code a sighted reader can (with some practice) mentally ignore the error handling boilerplate when interested in the happy path, or can pay attention to the one-level-indented blocks when interested in the error handling. This possibility is also what motivates some parts of idiomatic Go style, such as preferring to put the error paths inside nested blocks and "happy path" code at the top-level, and using the Putting the action being taken and the error handling on the same line effectively forces prioritizing one over the other, because only one thing can be first on a line. I think it's valid to debate whether the happy path and error handling should be combined onto a single line at all for this reason, but if you accept that as okay then it's also valid to debate whether the error handling or the happy path should appear first, since one will necessarily be harder to quickly scan than the other. In previous proposals of this genre I think we've seen three broad variations:
Given that each of these options makes a different compromise, it seems like these are material differences that contribute to making a particular proposal unique from others, rather than just details that could be figured out later after accepting the proposal. Assuming that reducing the amount of code spent on error handling is a goal (which, as I was noting earlier, also seems to be a broader point of disagreement), it does still seem important to consider the fine details of how this changes the "texture" of the code. I subconciously wrote the three categories above in my personal preference order; error-handling first seems to me like the worst option, because it would make almost all lines of code be dominated by their error handling. I most prefer the first category, although I must admit I've also become accustomed to how Go code is currently written and so I don't feel any particular urgency to change it. (I don't object to changing it on principle, but agree with Ian that if it's going to change at all then it should be a change that is very clearly better, due to the cost of making a large amount of code retroactively non-idiomatic.) |
I think that's a helpful summarization @apparentlymart. As pertains to having the an error handling keyword like I think it's fair to assume that most programmers scan code for patterns rather than read code carefully line for line, at least initially. I'd also like to push back a bit against the notion that errors don't belong on the left-hand side. I think errors that force a function to immediately return are very different from errors that can be handled in any manner.
I obviously can't speak to whether or not this constitutes an excellent change, but I will say if there is a construct powerful enough to both check for errors and return from the function, it should not be hidden somewhere programmers aren't accustomed to scanning. It should be obvious that a function could potentially return at that point. |
@apparentlymart
it still gives priority to the happy path and reduces the information overload on error path. I'm not sure if it something like this can be implemented purely as sugar and the compiler then adds the usual if err != nil that at this point is just noise. On those cases that need a different condition (different than is not nill) the current pattern can be used. Anyway I think that bridging the divide between those who think the happy path should be more readable and those who think that happy path is/should be no different than the error path is almost impossible as the history of countless proposals shows. In that light I don't agree that every proposal should be seen under the "is this the best possible proposal?" question. |
Per #63575 (comment), this is a rather large change in syntax that doesn't quite fit with the rest of the language. Additionally, the emoji voting is not in favor. Marking as likely decline and leaving open for four weeks for additional comments. |
I think annotations are very convenient, func do() error {
var err error
@error("some error")
res , err := foo()
@error("some error")
res , err2 := foo2()
} or unified processing func do() error {
@error("some error")
var err error
res , err = foo() // if err != nil,directly return without executing subsequent code
res , err = foo2()
} |
No change in consensus. |
I’m aware of the long history of this particular topic. What’s different about this proposal is that it introduces a return construct conditional on errors. I’ve looked through previous proposals and none from what I saw build on the
return
statement. As I hope to show in the background section, keeping the currentreturn
semantics is the core constraint of any solution for this problem.In a nutshell, I propose an
abort?
statement that transforms functions like this:into something like:
I think it’s helpful to systematically build up to a solution, so before diving in I want to build up an understanding of the problem. If my understanding of the problem is correct, then the solution is probably appropriate. However, if my understanding is flawed then the solution is likely also flawed, but we can at least catch that error and learn from it.
I tried my best to follow the language change template, but the questionnaire format was limiting as I prefer a systematic build up. However, I used the questions as a guide and all the relevant questions should be answered.
Background
Ironically, the “error-handling problem” is not about handling errors at all, but quite the opposite: not handling them and just bailing early.
It’s a problem that mostly affects what I'm calling mid-stack functions — functions that call other functions that can error but don’t have the context to decide how best to handle the error. These functions typically just return early and propagate the error to the caller. They have some variation of the familiar
if err != nil { return ... }
pattern.Functions near the bottom of the stack are closer to the application’s entry point and thus have more context as to what to do on errors. Their code is likely to handle errors even if just to log or add a metric or exit altogether. The key characteristic for these is that after checking for an error, they don't immediately return.
Functions near the top of the stack typically create errors though they might also contain parts resembling mid-stack functions if they happen to make system calls. They check for logical errors or invalid states and then return
error
values, but they typically don't check forerror
values to begin with.Why is this an issue just in Go?
This problem isn’t unique to Go, it’s something that happens in all languages. Whether or not you notice it is largely a matter of how the language treats errors.
Whenever an invalid state happens at the top of a call stack, you want to unwind quickly and get to functions inside the main application that have more context on what to do with that error.
Languages that use exceptions for errors throw an exception at the top of the stack and expect application functions to catch those exceptions. Mid-stack functions in these languages simply ignore the exception and let it bubble up. This transforms the problem into one of guessing which function might blow up. And even if it is clear which functions might throw, it means any function that throws forces the calling code to use a
try/catch
. It also means that if you want to transform or wrap an exception it means you have to catch one exception and throw another. But perhaps the biggest downside to this pattern is that it's not clear what invalid states are truly exceptions.Languages that return result types also encounter this problem and sometimes use some special construct to quickly return in the failure case. This is the most they can do since each function in the chain still has to return a result. Go is most similar to these languages. Though it doesn’t strictly have result types, Go's return statement is powerful enough to mimic them. These languages have the downside of having to propagate errors up manually but they have an advantage in that they create a distinction between errors and truly exceptional states.
I think the most you can do in a language that treats errors as values is to create an abstraction that returns quickly when an error is encountered. Anything more powerful than that strays into exception territory.
Examples of Failing Early
Whenever designing an abstraction, it’s helpful to see the different variations in order to figure out what’s constant and what varies. We should be able to abstract away the constant elements while providing a way to express what varies. Let’s look at a few examples of functions that immediately bail once they encounter an error.
First, we need to account for the error being returned in any position in the result list of an inner function call:
We also need to account for functions that bail-early while returning multiple values:
We also might want to replace or wrap the error in some fashion before returning:
From the cases above, we can see that the constant in all these variations is the extraction and testing of
err
. Thereturn
statement can vary and so it can't be abstracted away.The core problem seems to be reducing the ceremony around extracting and testing for an
error
, while keeping the semantics of thereturn
statement.Proposed Solution
My proposal is an
abort?
statement that immediately bails from the function whenever it encounters a non-nil error, returning a specified list of values.Code before
abort?
:Code with
abort?
:From the above code, we can see that there are 2 variants of
abort?
. A two clause version and a single-clause version.Two clause version
The first clause of
abort
expects an expression list that it uses to test for non-nil errors. In the above we're using the left-hand side of the assignment (t, err
) as the expression list. If any of these is a non-nilerror
, then the last clause is returned. As a result, the last clause is an expression list that MUST match the return signature of the function.The assigned values are available to the surrounding scope. This strays slightly from how other statements with an assignment preamble work (e.g.,
if
andswitch
) since they constrain assigned values to the following block. Sinceabort?
doesn't have a block, I think this behavior remains intuitive.One clause version
The single-clause version is used when the first expression list used to test also matches the return signature of the function. We can separate the assignment to its own line and write the 2 clauses in 2 steps:
But since the first list matches the second list we can just merge them into one:
That's what's happening for the
abort? uo.UnmarshalNext(...)
lines:Definition
Formally, it can be defined as:
The
abort?
statement tests its firstExpressionList
for the presence of anerror
and checks if the error is notnil
. If it isnil
then execution of the current function continues. If the error is notnil
, it returns the lastExpressionList
.In the case of an
Assignment
, the left-handExpressionList
of theAssignment
is used for testing for a non-nilerror
.The last
ExpressionList
MUST match the return signature of the function. In the case of the single-clause version, the first and lastExpressionList
are the same thing.It's illegal for the first
ExpressionList
to not contain anerror
type. We could check for this at compile time. This limits the blast radius of this change and makes it strictly for unwinding the stack without too much ceremony.Examples
I think it's worth showing a couple more examples to get a feel for how code might look with
abort?
. These are random functions from the standard library and kubernetes that contained instances ofif err != nil { return ... }
:The last example is particularly interesting in that it highlights when you might want to skip putting things on a single line and instead use a separate assignment statement.
abort?
follows the usage of theif
statement in that regard.Closing Thoughts
This idea came to me randomly and fully-formed for the most part so I hope I've done it justice. Please let me know if there's anything unclear or any edge-cases I didn't cover.
Costs
There's certainly a cost for adding a new keyword in both language and tooling. The compile-time and runtime costs should be roughly similar to the
if err != nil { return ... }
pattern since this is mostly an abstraction of that pattern rather than a new construct. I'm not familiar with the tooling implementation so I can't speak to the cost of that, but hopefully I've provided enough information for those that are to deem whether or not this is worth the cost.There's also the cost of learning a new statement for both experienced Go programmers and those new to the language. I've been careful not to introduce any constructs that don't already exist in the language. I think there is a cost here to be sure, but it's a minimal one since this is mostly giving a name to a frequently used pattern.
Benefits
The core benefit of this change would be to reduce boilerplate and increase information density. It saves 2-3 lines of code each time it’s used. That's not a lot in any singular case, but it compounds over multiple cases. As long as we're not losing information when condensing the code then the abstraction is worth.
It’s important not just to reduce boilerplate, but to communicate the intention behind that boilerplate. I think
abort?
does a good job of being lossless in that regard.Alternative Names Considered
My first version was
abort
to match the look of the other keywords in Go. That name was likely to cause issues becauseabort
as a package or variable name.Adding the
?
suffix solved both problems. It's not a breaking change since?
is illegal in package/variable names. It also captures the optional nature of bailing early only when an error is exists.return?
was considered, but while it captures the same sentiment asabort?
, it hurts the ability to quickly scan code because it’s only a single character away fromreturn
and it does nearly the same thing which would probably cause confusion and mistakes.error?
andtry
were also considered but they signal checking for errors rather than returning when one is found.try
also has the added baggage oftry/catch
in other languages and teaching a new meaning on top of that would be fighting an unnecessary battle.Comparison with Other Languages
I intentionally stayed away from discussing other languages since the design of
abort?
is based mostly on thereturn
statement and I wanted to keep this focused on that. I mentioned earlier that Go is most similar to languages that have result types. If you’re curious howabort?
compares with other "fail early" constructs, I think it’s an interesting exercise to rewrite code with the Result type in Go usingabort?
.The text was updated successfully, but these errors were encountered: