-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: spec: add "must" operator # to check and return error #18721
Comments
You probably want to read the discussion at #16225 first.
|
Thanks @minux, it's good to get more background. :) This is different from the |
There might be some overlap here with previous proposals, but this is the most comprehensive proposal I've seen about this, and it's actually structured as a proposal, so I'm inclined to keep this open instead of closing it as a dup. Thanks. |
The reason other proposals are rejected is not because of those fixable
drawbacks.
The reason is fundamental:
the proposed simplification encourages only those two kinds of error
handling, but in fact, there is another: augmenting the error before
returning to provide more contextual information. (You might say the
proposal doesn't preclude the other error handling strategy, but if it
makes the first two so much easier to use, it will definitely reduce the
incentive for people to even think about using the 3rd.)
Anyway, I think it's possible to write a code transformer for this and see
what happens.
|
I agree @minux. That was one of the listed cons. I've thought about this a bit, and while I didn't add this to the proposal (to keep it simple). There could also be a binary form of the operator which calls a function, adding the error to the argument list before returning. Example: result := UserDeleteError(user)#DeleteUser(user)
// would be equivalent to:
result, err := DeleteUser(user)
if err != nil {
return UserDeleteError(user, err)
}
// -- or -- (depending on the error handling strategy mentioned above)
result, err := DeleteUser(user)
if err != nil {
panic(UserDeleteError(user, err))
} |
@jargv, now you're getting into the |
@bradfitz I would love to learn more about that rationale, because lack of ternary is another pain point for me. :) Makes me wonder if I'm missing some of the zeitgeist of Go (though I'm still a big fan!). Lack of ternary is especially painful when I'm creating nested struct literals and I've got to put a conditional and a variable in order to conditionally populate some field, but now the declaration of the variable and its usage are potentially quit far away, whereas the ternary allows me to elide even creating a variable. Of course, I can change from declaratively creating the struct to imperatively creating it. Actually come to think of it, declarative struct literals feel a bit like precedent for declarative ternary and declarative error handling. Is it idiomatic to avoid struct literals in go? |
For what it's worth, in the HotSpot JVM we had introduced (almost 20 years ago...) the C++ TRAPS and CHECK macros, trying to address a similar issue using the existing C++ language at that time (and trying to avoid C++ exception handling because compiler performance around functions with exception handling was unclear). The similarity with this proposal (though not notation) is striking. It's interesting to see the same ideas come up again: Every function that could possibly "raise an exception" (cause an error) ended with the parameter TRAPS in the signature; see e.g., http://hg.openjdk.java.net/jdk7/jdk7/hotspot/file/tip/src/share/vm/prims/jvm.cpp, line 116. Every function call that invoked such a function, ended with a last argument CHECK in the function call (see the same file, line 127). TRAPS essentially amounted to an extra parameter passed that was used to communicate an error condition. The CHECK macro translated, well, into the following glorious macro...
(http://hg.openjdk.java.net/jdk7/jdk7/hotspot/file/9b0ca45cd756/src/share/vm/utilities/exceptions.hpp, line 183). I leave it up to the reader to decipher the macros and judge their beauty... With respect to this proposal, I think it's actually more cryptic than the C++ approach above. Bike shedding aside, |
Instead of an operator, it could be a builtin generic function with a signature like:
So calling it just looks like:
(Edit: To be clear, I'm not expressing an opinion either way on the overall proposal. I'm just suggesting a less-intrusive syntax to the originally proposed |
Ha! That's awesome @griesemer, thanks for sharing that. I've done something similar-ish in go (sans macros) to try and invert the error handling responsibility: func foo1(errp *error, arg1, arg2 int) {
if *errp != nil {
return
}
// do something useful
}
func foo2(errp *error, arg2, arg2 string) {
if *errp != nil {
return
}
// signal failure
*errp = fmt.Errorf("failed because reasons")
}
func main() {
errp := new(error)
foo1(errp, 3, 7)
foo2(errp, "3", "7")
// ...
// fooN(errp, ...)
// call N functions for the price of one error handling block!
if errp != nil {
log.Printf("*errp: %#v", *errp)
}
} |
If we're having func fn() (..., error) {}
a := must(fn(), "extra error info") // append/prepend string to error returned by fn
b := must(fn(), func(err error) error { /* return the "augmented" error */ }) IMO, this is much more readable than |
Having written some examples in the documentation I tend to approach this proposal from the perspective of both using it in examples and impact on new gophers while reading them. This would become, in my opinion, a very contentious operator/built-in, raising always a flamatory question on whether a such use case is more appropriate using it or not. Frankly, I do not believe this should be a choice of the developer but of the package. If I, as a developer, must make such call, I can perfectly ending up with a divergent interpretation or decision from a fellow Gopher.
It is no coincidence we don't see In other words, I think this is a valid idea, but not for the language, only for specific packages that it might make sense. |
how about adding a keyword, roe (return on error), which like the go and defer keyword, being used as prefixes of function callings? |
This sort of minimization can obviously be performed without changing the language (https://play.golang.org/p/F-ZZySWZ5P).
vs
In my opinion, reducing a handful of lines of code is not worth introducing the incongruity of a function which can return a varying amount of values. |
But most[1] of the error handling deals with augmenting the error with contextual info and propogating it back to the caller. i.e. f, err := os.Open(p)
if err != nil {
return fmt.Errorf("can't open file %q: %v", p, err)
} This is the case I am more interested in. I don't see how individual packages can achieve that, without any language change. [1]: According this visualization, most common words in Go are |
@golang101 that would only work if you don't care about non-error returns |
This is much leaving the issue of "must". It does not require a built-in, it would require addition(s) to the |
My concern with "Must" would be that new developers would stop handling errors. They would just use the panic feature since it's less typing. I already see tutorials trying to do this:
Where
Making it easier to not correctly handle errors seems like it would be a big detriment to the language. |
While I don't do this, I don't have much of a problem with it if the intent is to bomb out in a main func. Also, naming plays a large role as "checkErr" is unclear and even makes the example a bit of a straw-man. "errTrip", "tripOnErr", etc.; Naming can bring enough clarity and separation from regular error handling that this sort of minimization doesn't become a DRY pedant's misplaced habit. Again, I don't do this. I'm content with explicit error handling when bombing out, even if it adds a few lines. |
Sorry, I don't understand. How can changing error interface avoid |
I like the idea, but share @xeoncross concerns about making it easier to not handle errors properly That aside, if we get it, I'd prefer |
I'm sorry, I misunderstood your statement. I had thought you were moving away from the issue of control flow operators and strictly concerned with easing the modification of errors with prefixes/suffixes. Nonetheless, your digression still takes us away from "must". Again, in my opinion, reducing a handful of lines of code is not worth introducing the incongruity of a function which can return a varying amount of values. My suggested and currently possible approach is only good for usage of Edit to add - It is probably worth pointing out that the only builtin which is directly related to control flow is |
I admit when looking at word counts and scanning over large blocks of code without reading it the error checks seem repetitious. They require at least a one code block with a conditional and the assignment operation always directly precedes it. However I also believe this contributes a great deal to Go's readability, at least for me. The cognitive load of both handling and identifying operations that can fail is very minimal. I can type 3 lines of code very fast and it's so mundane I have no chance of losing any train of thought. I haven't written any must() style functions in a long time in library code, but I do write chk() must() etc style functions in unit tests constantly and certainly find if { t.Error|Fatal() } a bit excessive at times. But I also don't need to find tests super readable, they prove your program correct once and you don't change them until you change your program. Not saying I am against this or anything, but I think it would be nice to see a entire library written / converted in the proposed style so people may get a good idea of the mental cost when reading code. Might be worth looking at a middle ground here, just to compare against. Really this feature has two parts, the implicit checking for non-nil error interface and returning control flow to the caller with the implicit error assignment. Might be nice to see how they look individually and together, to be honest I think the below would be a nice quality of life improvement without sacrificing any readability:
I do find having to line-item out my zero values over and over can be a bit tedious, or dealing with values where I can just mash 0 and nil, need to stop and think about the flow of the program, defining a zero value var T or using named return params, etc. Just food for thought, nicely structured proposal though! |
How about introducing two keywords, package main
import (
"github.com/juju/errors"
)
func ParseEthernetHeader(data []byte) (mac *EthHeader, read int, err error) {
fail (err error, reason string, args ...interface{}) {
// first argument of a failure block is always the error
return nil, 0, errors.Annotatef(err, reason, args...)
}
if len(data) < 8+6+6+3+4 {
// one way to explicitly call the failure handler so we concentrate
// error handling in a single place without relying on 'goto'
fail(ErrPacketTooSmall, "packet with %d bytes too small to contain an ethernet frame", len(data))
}
mac = &EthHeader{}
read += 8 // eth header preamble
// parses a MAC address, if it fails, the fail() block is called automatically,
// where `err` will be the error returned by ParseMAC and everything else is passed
// as extra arguments (for annotations, etc)
mac.Destination = must(ParseMAC(data[read:]), "invalid MAC address on destination field")
read += 6 // source MAC
// ...
return mac, read, nil
}
func ParsePacket(ethernetFrame []byte) (*EthHeader, *IpHeader, *TcpHeader, *UdpHeader, []byte, error) {
var ethernetHeader *EthHeader
var ipHeader *IpHeader
var tcpHeader *TcpHeader
var udpHeader *UdpHeader
var read int
var totalRead int
var payload []byte
fail (err error) {
// first argument of a failure block is always the error
return nil, nil, nil, nil, []byte{}, err
}
ethernetHeader, read = must(ParseEthernetHeader(ethernetFrame[:]))
totalRead += read
ipHeader, read = must(ParseIPHeader(ethernetFrame[totalRead:]))
totalRead += read
switch ipHeader.Protocol {
case 6:
tcpHeader, read = must(ParseTcpHeader(ethernetFrame[totalRead:]))
totalRead += read
case 17:
udpHeader, read = must(ParseUdpHeader(ethernetFrame[totalRead:]))
totalRead += read
default:
fail(ErrProtocolUnsupported)
}
payload = ethernetFrame[totalRead:]
payload = payload[:len(payload)-4]
return ethernetHeader, ipHeader, tcpHeader, udpHeader, payload, nil
} |
Just to note: it's not possible to add new keywords to Go 1. It will break programs where a new keyword is used as an identifier. |
Most of the proposals above suggest a keyword in front of the function to be wrapped with error handling. I tend to disagree that this makes a nicer look or better readability than the I like how Perl's
(Where An equivalent in Go might be something like
Compared to:
this approach provides some advantages:
|
Hi, I wonder if this change should be in the language at all or not. I know that many people think that the We praise Go for the clarity and simplicity it brings to programming and we love it for being able to link articles that were written sometimes before 1.0 that are still relevant and apply to the language (for example: https://blog.golang.org/profiling-go-programs ) To this extent, I would propose this as change to the tooling surrounding Go rather than changing the language itself. For example, for this example: func test() (interface{}, error) {
res, err := test()
if err != nil {
return nil, err
}
if res == nil {
panic("Invalid resource")
}
return res, nil
} this could look something like this: This way, the users that want to make this "more" readable will be able to do so, while those who don't will be able to keep the display settings as they are. The other aspect of the proposal is already covered by most editors that support code templating as you could for example a template One could also argue that the single line return / panic handling are not that common, depending on the software / libraries being used and that having people to actually think of error handling instead of bubbling this up is a feature not a problem. Or, as others shown, sometimes a comment is necessary to make it clear why a panic is needed instead of just returns. As such I think this is much better suited for the tools to be change not the language and everyone gets what they need. |
As I've forgot to mention it in my previous reply, but I think others did it already. By changing the way the editors display the code but not the language we also gain the benefit of not having to change any other tool built for the language itself, adding extra knowledge and responsibilities to them in order to satisfy this. |
A very good idea! Especially as this approach definitely does not break the Go 1 compatibility promise. There is, however, a downside to this: Every IDE and editor plugin that implements this will implement this differently. Some of them won't implement this at all, and numerous tools (cat, less,...) will never become "Go error aware". Most web pages will continue to display plain code without any reformatting. So the same piece of code may look very different depending on the tool used to examine the code. |
I guess this could either be agreed or not on how the editors will choose to display this. But if you are a user of a particular editor chances are that you are rarely jumping editors on a high frequency for this to be so bad. But as I've said, editors could agree on this for the display output.
I agree that non-Go tools and say Github / Bitbucket or other places would display this maybe in a different manner but my suggestion is that the editors that are used for Go already know they "talk" Go. It also has the advantage that it can be turned off if people don't like it, it doesn't change the language by introducing a keyword (which would be a compatibility break) as others suggested, it doesn't introduce a special syntax that would make it harder to understand what's happening. With the current proposal the following things will happen when reading the code:
This is definitely going to hinder readability, where as if an editor can just tell you in a one line that you are returning / panicking then you can move on. We also have to approach this from a reasonable point of view: do I ever read code using cat? what will |
I was intrigued by @dlsniper's suggestion, so I started a vim plugin to try it out: jargv/vim-go-error-folds. I've only used this very briefly, but so far I kind of like it. |
To be clear, this not my idea, I've not named the source of inspiration as I'd rather keep this focused on the proposal itself rather than have a debate of merits of IDE / editors / whatnot which could be polarizing and biased. |
The "or..." syntax does not trigger any of these questions. This said, editor plugins might indeed be the way to go. After all, they are a solution that can be applied now, whereas all language changes have to wait for Go 2. I particularly like the way how @jargv's Vim plugin turns the error handling statements into a readable and concise To ensure the same appearance across all editors and IDE's, the Go team might want to define mandatory syntax rules for the folded expression. |
@xeoncross said:
As an example of this exact issue, I saw this in Rust when I dabbled in it. In Rust, errors are returned as part of a tagged union called As annoying and repetitive as Go error handling can get, I actually like that it forces you to handle errors when there could be one. It could certainly use some tweaking, such as to the difference between functions that just have an |
I think this is not needed, both from the editors and from the Go team. There's no need for the Go team to specify how code is rendered and there's no need to have this consistent across the editors either. If someone comes up along the way with a different format then everyone likes it the editors should be able to cope with having options for how to handle it. |
I'm inclined to close this proposal but remember it for a longer error-related discussion later in the year (or just put it on hold until then). As I wrote at https://research.swtch.com/go2017#error, the big disappointment I have with the way most code uses errors is exactly that most code doesn't add important context. It justs 'return err'. Making that mistake even easier seems like a step in the wrong direction. (And making it hard to see that whether code panics or returns an error is even worse, but that's a relatively minor point.) |
There's no need for the Go team to specify how code is rendered
The did it already by providing `go fmt`. Creating a tool that ensures consistent source code look-and-feel for everyone was a genius move IMHO. No more arguing about how code should be formatted. This should not be diluted by a *laissez-faire* attitude towards code representation in editors.
… Am 23.01.2017 um 21:08 schrieb Florin Pățan ***@***.***>:
To ensure the same appearance across all editors and IDE's, the Go team might want to define mandatory syntax rules for the folded expression.
I think this is not needed, both from the editors and from the Go team. There's no need for the Go team to specify how code is rendered and there's no need to have this consistent across the editors either. If someone comes up along the way with a different format then everyone likes it the editors should be able to cope with having options for how to handle it.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#18721 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADqH8g51xBWvq06_iRtxnUUKyoUaHjqlks5rVQixgaJpZM4LosQX>.
|
@rsc at the very least, I think a callstack with file:line should always be part of error types... printing an Adding extra context to errors as a key-value would be ideal too (like context.Context) - so if inserting a row into a database table fails, I can add that row itself as part of the error (and my non-stdlib logging/metrics component saves that to elasticsearch, including a JSON representation of the column). |
@christophberger lets agree to disagree. Personally I dislike the Go font so the last thing that I'd like to do is have the font option taken away because the Go team decides that that is the only way we must display the code. Same with my proposal. @dansouza I'd say lets hold on on this until such time is appropriate and meanwhile focus on the proposal at hand. |
lets agree to disagree.
Agreed. Especially, let’s not forget that the original proposal is about a language change, not about editor plugins. So we have been moving quite off topic by now anyway. Editor-based error folding *will* come anyway, as this requires no language change and the editor/plugin/IDE authors are free to do so.
… Am 23.01.2017 um 21:36 schrieb Florin Pățan ***@***.***>:
@christophberger <https://github.com/christophberger> lets agree to disagree. Personally I dislike the Go font so the last thing that I'd like to do is have the font option taken away because the Go team decides that that is the only way we must display the code. Same with my proposal. gofmt is a different matter altogether.
@dansouza <https://github.com/dansouza> I'd say lets hold on on this until such time is appropriate and meanwhile focus on the proposal at hand.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#18721 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADqH8qTK8bMvN7McF2T65J299ysLjDGzks5rVQ87gaJpZM4LosQX>.
|
@rsc what's wrong about just returning an error when you know that this error has already a context ? (we have this problem with pkg/errors when we add the stack at each return) |
We're going to rethink errors in Go 2, so let's leave the general topic for then. We're very unlikely to add something this substantial before then. |
This is not a fully-fleshed out proposal, but here's an observation on the minimal required language change: If the "must" operator 1. stripped off the err value if applicable and 2. returned the zero value of all non-err return values and passed the error through, you could get an awful lot of mileage out of:
This pattern 1. allows wrapping return values and 2. allows The core desire for "capturing" the error on the way out and doing something to it can be met with named parameters and defer already; in fact you can already use this technique now if you manually spell out the |
@rsc Is there a discussion somewhere related to errors in version 2 somewhere? |
Not yet. I'd like to open a discussion about errors later this year. |
Taking this example, err := f()
if err != nil {
return nil, errors.New("f failed: " + err)
}
if err := g(); err != nil {
log.Println(f failed:", err)
} with a package main
import (
"fmt"
"log"
)
func f() error { return nil }
func g() error { return nil }
func work() error[, params...] {
// the err is completely hidden
// must [, ...vars :=] f() or mustReturn [...args]
must f() or mustReturn
// the err is copied from work via must
// if [vars...,] err := must g() {
must err := g() {
return err[, vars...]
} else {
fmt.Println("ok")
}
return nil[, vars...]
}
func main() {
// the err is completely hidden
must work() or mustPanic
// the err is copied from work via must
must err := work() {
panic(err)
}
fmt.Println("hello world")
} Its a keyword because go language itself can t handle the requirements of such // alike signature of must
func must( [interface{}..., ]err error ) ( [interface{}..., ]error?bool? ) {
if err != nil {
return err
}
return false //|nil ?
} Its an impossible signature, Also, that would surely help with the current practices The shorter versions such as As an example, error augmentation, tracing and probably more, In case of multiple output arguments, The Corresponding to such signature, // alike signature of must
func mustReturn( [interface{}..., ] [format string, ] err error ) ( [interface{}..., ]error ) {
if format=="" {
format = "err is nto nil: %v"
}
return [interface{}..., ] augmentErr(format, err)
} yet, many impossible things into this signature. One variation of this might be, must notFail() or
must errors.New("some arguments") or
must fmt.Println then
return x, y[, err] or, a goish version, must x, y, err := notFail() or
must err = errors.New("some arguments %v", err) or
must fmt.Println(err) {
return x, y, err
} or, a goish version, with a must x, y, err := notFail() or
should err = errors.New("some arguments %v", err) or
should fmt.Println(err) {
return x, y, err
} The less shorter version must err := work() {
panic(err)
} Is somehow magical, yes, but i believe |
Builtin functions can have any signature necessary. For example, We also need to continue accepting Go 1 code that uses |
The must operator is to error handling what the go keyword is to concurrency, and the defer keyword is to resource cleanup. It turns any function into a Must*-style function with smart error handling.
NOTE: this isn't a change to the way error handling works, only to the way error handling looks.
The Must operator (#) is a unary prefix operator that can be applied to a function call. The last return value of the called function must be of type
error
. The must operator "peels off" the returned error from the result list, and handles that error - if it is not nil - according to the error handling strategy of the surrounding function.There are 2 possible error handling strategies:
The error handling strategy is determined by the surrounding function's return types: if the last return value is of type
error
, it's error handling strategy is case 1, otherwise it's case 2.Example:
Some Pros:
Some Cons:
Conclusion
I believe there's precedent for the Must operator in golang. The
defer
andgo
keywords transform the way function calls work. Thego
keyword hides a load of complexity from the user, but always does the right thing. Thedefer
keyword makes code less error-prone by ensuring the cleanup code will be called on every path. The Must operator is similar: it does the right thing based on the surrounding function, and it ensures that the error is handled.The text was updated successfully, but these errors were encountered: