-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: Go 2: introduce a 'check' built-in function to reduce error handling boilerplate #33233
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
Comments
I think this proposal actually does address it: func CopyFile(src, dst string) (err error) {
r, err := os.Open(src)
check(errors.Wrap(err, "error opening copy source"))
defer r.Close()
w, err := os.Create(dst)
check(errors.Wrap(err, "error creating copy destination"))
defer func() {
w.Close()
if err != nil {
os.Remove(dst) // only if "check" fails
}
}()
_, err = io.Copy(w, r)
check(errors.Wrap(err, "error copying data"))
check(errors.Wrap(w.Close(), "error closing destination file"))
return nil
} Without this call-site annotation, it could potentially be very difficult to figure out if an error came from the I'm not really sure if I like this proposal more than |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Thanks for making the above point which is a good one. Although it had occurred to me that You could, of course, avoid this cost with the present way of doing things: _, err = io.Copy(w, r)
if err != nil {
return errors.Wrap(err, "error copying data")
}
err = w.Close()
if err != nil {
return errors.Wrap(err, "error closing destination file")
} On the other hand the cost might not be so great as probably the first thing a decorating function will do is to return Also this is something that |
I'm going to close this now as it's already clear there's little support for it and I don't want to clutter up the Go 2 proposals list with any more 'no hope' error handling proposals than there already are. Thanks to those who did support it or made a constructive comment. I was a supporter of the This proposal was even simpler than It would, of course, be easy enough to add a second optional argument (a string) to check(errors.Wrap(err, "error opening copy source")) you'd have: check(err, "error opening copy source") However, unless this were confined to just prepending the second argument to the string value of the first, it would inevitably involve calling some wrapping function under the hood (hardly ideal for a language feature) and you'd end up with the same problems which beset #32811. Frankly, I find it difficult to see how any alternative proposal could ever deal with in place error decoration in a better and more flexible way than |
Now that the
try
proposal (#32437) has been withdrawn, I have been thinking about whether there might be a middle-ground proposal which would still save significant error handling boilerplate but be more acceptable to those who dislikedtry
.After studying various alternative proposals and the factors which led to
try
's demise, I have come up with the following which is even simpler and might fit the bill. There have been many similar ideas (notably #32811 which was a major influence) though probably not in the exact form used here.The proposal.
I propose the introduction of a new built-in function called
check
. This takes a single parameter of a particular interface type (usuallyerror
) and does not return anything.check
can only be used inside a function with at least one result parameter where the last result is of the same interface type.If the argument to
check
isnil
, then it does nothing and control passes to the next statement.However, if the argument is not
nil
, then it triggers an immediate return from the enclosing function. The final return parameter of the function is set to the argument tocheck
and any other return parameters are given their zero values (or current values if named).So, this:
turns into the following inlined code:
where the x's are the zero values (or current values) of any other return parameters apart from the final interface parameter.
Aim of the proposal
The aim of this proposal is exactly the same as
try
, namely to cut down on boilerplate in a significant number of cases.As in the case of
try
, all errors can be decorated in the same way usingdefer
or the currentif err != nil
approach can and should be used in all other cases.As a new built-in function is being used, it would still be backwards-compatible and would still be open to possible future improvements by simply adding a further parameter or parameters to
check
.Example
The following example taken from the
try
proposal may help to make all this clear:Note that a function which only returns an interface value of the relevant type (such as
w.Close()
) can be passed directly tocheck()
.Advantages
Compared to the
try
proposal, the present one has the following advantages:It's simpler and should therefore be easier to implement, understand and document.
Custom error interfaces can now be coped with - though I believe
try
could have been altered to do the same.It will be easier in most cases to refactor between code using
check
and the present approach, which should make debugging easier. Also the former doesn't look out of place where both approaches are used in the same function.The word
check
is perhaps more suggestive that a change in flow control may be triggered thantry
was.As
check
doesn't return anything, this effectively rules out the possibility of there being several independent errors on the same line (i.e.try
expressions, nested or otherwise) which a lot of people disliked (albeit others saw as an advantage).Disadvantages
Compared to the
try
proposal, the present one has the following disadvantages:It doesn't save as much boilerplate.
check
is probably used more thantry
as an identifier in existing code and is longer.Finally
I should perhaps add that I'm not totally sold on this proposal (or for that matter any other alternative proposal) myself as it's questionable whether it saves enough boilerplate to be worth doing but it certainly looks more compact.
Incidentally, I've given up hope that
go fmt
will be changed to permit single statementif
's to be written on one line which would have made even the present approach look more compact.Also this proposal doesn't address (or even attempt to address) the problem of error decoration any better than
try
did because, quite frankly, I think this is best done with the existingif err != nil
approach unless all errors can be decorated in the same way whendefer
can be used.However, I think it's arguably clearer what's happening with this proposal and so I thought that I should at least put it forward to see whether there's any support for it. If not, then it's something else we can rule out once and for all.
The text was updated successfully, but these errors were encountered: