-
Notifications
You must be signed in to change notification settings - Fork 17.4k
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 Paradigm #60720
Comments
The introduction of a new keyword makes this not backwards compatible. This looks (almost?) identical to the error handling draft, feedback. |
Okay, genuinely I did not know anything about this draft and I feel bad about this. However, after reading it, the only things that differ are:
For the first difference, I am sure that the chaining possibility has more value than being able to change the handler, considering that it is still possible by adding to the chain an handler with a return statement, essentially shadowing every previous handler. But for the second difference, I think that would be sort of a limitation to have to return at the end of the chain: One last thing, and it is for my better understanding: i thought that "being backwards compatible" meant that in the new version, every type of code written in previous versions would just work without any difference, but obviously any newer code written with the new statements and constructs would not work on older versions. So adding a keyword would break backwards compatibility only because someone in his code maybe have used the same work for a variable (for example) right? Or there is another aspect that I am not aware of? I'm again sorry for not being aware of this draft. |
Yes, as you say, adding a keyword is pretty much always incompatible, because it breaks code that has variables/types/functions/constants whose name is that keyword. That doesn't mean that we can never add a new keyword. After all, it's not hard to automatically rename all such objects. But it does impose a significant cost. All language changes have costs and benefits, and whether to adopt the language change requires weighing those costs and benefits. |
Thanks for the insight. But going back to the error handling, I am curious about the reasons why the catch statement would result in the enclosing function to exit (if the error is not nil), without having the possibility to handle the error in some way (maybe, like in my proposal, just by logging it) and let the function continue. I also think that having a built-in implicit default handler is a really "nice to have", but maybe would encourage people to not handle errors in the most reasonable why. Normally is just enough to wrap that error inside a new error with additional description of the context: in fact, I don't think that this code handle err {
err = fmt.Errorf("this is the context: %w", err)
} is so different and more verbose than handle err {
return zero_value_1, ..., zero_value_n-1, fmt.Errorf("this is the context: %w", err)
} also because you have circumstances where the zero value for an operation is different from the zero value of the type (I am thinking about searching the index of something in a string, array, slice, ecc.) |
No okay, as @seankhliao pointed out, now we are talking about changes that, at least for me, should be made to the original draft about error handling. Maybe this proposal should have a different title now and I should add at the top of my proposal a note that guides to what we are talking about now, leaving the rest of the message intact to keep the entire context.
So, given that we take everything as it is in the draft mentioned above, that has all the same principle as mine (just more polished and with some syntax differences), the only problem that I have with the original draft is that if you call a function with a check, the enclosing function at the end will always return, there is no way you can keep the function alive (quote taken from the draft:)
For dealing with this problem, in my previous post, I proposed the fact that the default handler (which is the cause for inevitable return statement) should be there until the first handle statement was declared in the function explicitly. Maybe another way to handle this (sorry for the word) is to add the possibility to control the chain flow with |
All of these examples in this proposal are leaking a file descriptor, by not closing a file on error. I might have missed it, but I don't see a way to register cleanup functions, which could have helped here. |
I you are talking about the examples in my code, you kind of right, I missed the However, for the sake of correctness, I will update the code snippets. |
No, it won't. For a simple cases, yes. But for some resources you want to close them as soon as you are done with them. Defering every close by default would make you to double-close such resources. Defer is not a solution for the problem I mentioned.
It would be greatly appreciated, a proposal should have if not real-world use-cases, then at least correct examples. |
I think to have understood what you mean and you are completely right. I'll try to explain with some Old and wrong way to do it: func heavyWorkload() error {
resource, err := loadHeavyResource()
if err != nil {
return err
}
defer resource.release()
data, err := resource.useResource()
if err != nil {
return err
}
// some other long procedures that use data
return nil
} In this case the heavy resource will be released only at the end of the enclosing function. func heavyWorkload() error {
resource, err := loadHeavyResource()
if err != nil {
return err
}
data, err := useResource(resource)
resource.release()
if err != nil {
return err
}
// some other long procedures that use data from the resource
return nil
} One way to handle this would be to wrap a portion of the code in another function func heavyWorkload() error {
handle err {
return fmt.Errorf("heavy workload error: %w", err)
}
data := catch func() ([]byte, error) {
resource := catch loadHeavyResource()
defer resource.release()
data := catch useResource(resource)
return data, nil
}()
// some other long procedures that use data from the resource
return nil
} In this case, I admit, it's a little bit clunky, but would also add the benefit Or another way of addressing this would be to use the func heavyWorkload() error {
handle err {
return fmt.Errorf("heavy workload error: %w", err)
}
resource := catch loadHeavyResource()
data, err := useResource(resource)
resource.release()
catch err
// some other long procedures that use data from the resource
return nil
} Probably a lot more readable, but would not work if you have to use the resource more func heavyWorkload() error {
handle err {
return fmt.Errorf("heavy workload error: %w", err)
}
resource := catch loadHeavyResource()
var resourceReleased bool // set to false
releaseResourceFunc := func() {
if !resourceReleased {
resource.release()
resourceReleased = true
}
}
handle err {
releaseResourceFunc()
}
data1 := catch useResourceFirstTime(resource)
// use data1 ...
data2 := catch useResourceSecondTime(resource)
releaseResourceFunc()
// some other long procedures that use data2 from the resource
return nil
} Again, not a huge fan of this implementation ... Maybe the best way to handle this is:
I think sometimes one feature can't be used for every use case, and will only One last thing I did not have understoop from your comment (and don't get me wrong ... |
@rjeczalik, I just thought about a less clunky way to handle the cleanup with a flag that would extend to other use cases (not only flags). func heavyWorkload() error {
handle err {
return fmt.Errorf("heavy workload error: %w", err)
}
resource := catch loadHeavyResource()
var resourceReleased bool
handle err if !resourceReleased {
resource.release()
}
data1 := catch useResourceFirstTime(resource)
// use data1 ...
data2 := catch useResourceSecondTime(resource)
resourceReleased = true
resource.release()
// some other long procedures that use data2 from the resource
return nil
} So after the second handle declaration, when a function is called with the check statement, the resulting control statement underneath would be |
This hurts readability too much, and holds too much overlap with just using a function in my opinion. func LoadJSONFile(filePath string) (*MyStruct, error) {
var err error
errf := func() (*MyStruct, error) {
return nil, fmt.Errorf("could not load json file <%s>: %w", filePath, err)
}
f, err := os.Open(filePath)
if err != nil {
return errf()
}
defer f.Close()
data, err := io.ReadAll(f)
if err != nil {
return errf()
}
var result *MyStruct
err = json.Unmarshal(data, result)
if err != nil {
return errf()
}
return result, nil
} |
Thanks for the effort you've put into this proposal! Considering your first example using func LoadJSONFile2(filePath string) (result *MyStruct, err error) {
defer func() {
if err != nil {
err = fmt.Errorf("could not load json file <%s>: %w", filePath, err)
}
}()
f, err := os.Open(filePath)
if err != nil {
return err
}
defer f.Close()
data := io.ReadAll(f)
if err != nil {
return err
}
err = json.Unmarshal(data, result)
if err != nil {
return err
}
return result, nil
} This variant preserves your goal of writing the error-wrapping call with its common prefix only once, but it does still require an The other notable drawback of my variant is that it requires naming the return values so that it's possible for the defer to mutate I don't really have a strong opinion on this proposal yet, but when thinking about proposals I like to try to imagine ways to achieve something as similar as possible using the current language, because that helps to consider what value the new language features are bringing. To me the above seems like a reasonable solution to the problem as stated, but I must admit I would be unlikely to actually write code like this because I tend not to make functions generate errors with information that the caller of the function already knows, and instead focus on describing the aspects of the problem that the caller cannot determine from its own context about what it was calling and what it passed in. The problem statement therefore doesn't really match a problem I have, but I acknowledge that others have different preferences for error message content. |
hi, I also have a proposal for Go 2: Error-Handling, can you also comment it? thanks for your see |
Can go support no func LoadJSONFile(filePath string) (*MyStruct, error) {//error must define
f := os.Open(filePath)
defer f.Close()
data:= io.ReadAll(f)
var result *MyStruct
json.Unmarshal(data, result)
return result, nil
} |
@webbongithub, thanks for your opinion. Personally, I know this is a subjective topic don' get me wrong, I don't think if hurts readability that much. I agree that there is a significant overlap with the alternative solution you proposed, but adding a new way to do the same things with a slightly different syntax it's not always a bad thing.
If someone told me that they would remove for ranges in Go 2, I would be really mad. func LoadJSONFile(filePath string) (*MyStruct, error) {
handle err {
return nil, fmt.Errorf("could not load json file <%s>: %w", filePath, err)
}
f := catch os.Open(filePath)
defer f.Close()
data := catch io.ReadAll(f)
var result *MyStruct
catch json.Unmarshal(data, result)
return result, nil
} I honestly don't see why this would be so much unreadable. Just my honest opinion. |
@y1rn, The problem with this is that you are actually ignoring the error, and this is really a bad practice, in general. |
I mean func LoadJSONFile(filePath string) (result *MyStruct, err error) {//error must define
defer func(){
if errors.Is(err, BaseErr) {
//...
}
}()
f := os.Open(filePath)
defer f.Close()
data:= io.ReadAll(f)
err = json.Unmarshal(data, result)
if err != nil {
return nil, fmt.Errorf("fail to unmarshal json data: %w", err)
}
return result, nil
} This looks do not need to change go syntax and good to write code, Problem is reviewer doesn't know where may return err. |
@y1rn sorry but I can't understand if you are proposing another feature or if you are providing an example that would do the job in the current version of Go. Because if this is the second case, this code will never compile. |
@apparentlymart I completely understand your approach on things and, as I also replied to another person, I understand that there are already ways to do the same thing with nothing more, nothing less. |
Thanks for the proposal and discussion. As pointed out above, this is fairly similar to the earlier Therefore, this is a likely decline. Leaving open for three weeks for final comments. |
I am not sure that i understand what you mean by I personally think this is verbose and doesn't fix things much. func heavyWorkload() error {
res1 := somethingThatCanFail()?
res2 := somethingThatCanFail2()?
return nil
} To anotate errors, the question mark operator should generate code that calls a method on an interface that does the conversion. Just as they do in Rust. |
@peter7891 Please see #32437. It was spelled |
i don't see any better solution than this. From what i see, people complained it wasn't clear or it was hard to see where functions returned. Something like Zig const number = try parseU64(str, 10); Although, i do prefer the question mark, after the function call, like Rust. let number: i64 = "1".parse()?; I think its easier to spot stuff at the end of the line than in the middle. |
How about something like this: func LoadJSONFile(filePath string) (*MyStruct, error) {
f := os.Open(filePath) catch ("could not open json file")
defer f.Close()
data := io.ReadAll(f) catch ("could not read json file") {
// This is an optional code block to close something if needed.
// After running this block it jumps to the catcher label below.
}
var result *MyStruct
json.Unmarshal(data, result) catch ("could not unmarshal json file")
return result, nil
catcher(err error, msg string):
return nil, fmt.Errorf("%s <%s>: %w", msg, filePath, err)
}
|
No change in consensus. |
Author background
I'm a student, but i'm mainly using go from the first day I started programming and now I'm using
it for 3 years. I coded a bunch of libraries, some targeting specific needs for different
OSes, the largest one is a TCP/HTTPs server library (Server v2).
I use regularly Javascript, Java, SQL databases (HTML and CSS if you consider them programming languages).
Related proposals
Yes, there have been a lot of proposals related to this topic.
First of all, this proposal matches the official guidelines related to this topic.
Then it will be completely backwards compatible and will not introduce any weird
constructs that will preserve the readability and simplicity of the Go code.
Yes, the topic is the error handling.
If so, how does this differ from previous error handling proposals?
This does not to implement anything like a try-catch, considering that this is already
too similar to the panic-recover already present.
This proposal has more of a macro-like behaviour, reducing the possibility of overworking
on a new mechanic, with the risk of diverging from the Go principle of "error checking".
Error Handling Proposal
This proposal addresses the problem that affects every project written in Go
that has to deal with a lot of error handling, resulting in a very verbose
code, without removing the principle that you should always handle every
error returned by a function in the best way.
This would benefit every go programmer, considering it's about error handling, and
is a widely discussed topic among every programmer, even non-go ones.
Changes
The proposal, by adding only a new keyword to the language, the
catch
one, will addto the language 2 new strictly related construct (more like 1 construct and 1 prefix):
the function it's declared in; this construct will be presented below
this prefix will be presented below
The backwards compatibility will be discussed below along with code snippets showing the changes.
We will discuss also interaction between other language features, like goroutine functions ran with
the
go
keyword (this proposal has a similar concept, that is adding a prefix to a function call toexplicitly say that you want to tread that function differently from normal).
This proposal is not about performance improvements, but (with my knowledge) should not even affect it
in a bad way.
Introduction
Here is a good example of a code that would benefit from the introduction
of this concept:
It's obvious that there is a lot of code that is redundant and verbose: this could be
fixed if we could tell the compiler what to do every time a function returns as the
last return-value an error and it is not nil, like a macro. Here is an example:
First Usage
The first usage of the
catch
keyword is to tell the compiler that fromthis point until the end of the function, without inheritance
from other function calls inside, if you call other functions
with the
catch prefix
, it should implicitly call thecatch function
only
if err != nil
.To set the
catch function
you must use this syntax only:without using
func() { ... }
or any pre-declared function (like withdefer
orgo
)to underline that this
catch function
should be explicitly madefor this function errors and should not be handled by a function
that is used for every error in the program/package.
However once inside the catch function body, it's always possible
to call other functions as a normal function.
The
catch function
must not have a return signature (like a 'void'function) because in reality it's implicitly matching the one
of the calling function (in this case
(*MyStruct, error)
).This is also why the catch function must be declared inline.
Second Usage
The second use case of the
catch
keyword is by placing itin front of a function call that has as the last return type an error.
By doing this you can avoid taking this last error and
tell the compiler to handle it automatically with the catch
function if the error is not nil (removing the need constantly use of the
construct
if err != nil { ... }
).Function with named parameters in the return statement
The catch argument can have whatever name you desire, but it
must be an error. In this case if you named it
err
, theone declared in the function signature would be shadowed.
Illegal Usage and Skip error check
Manually trigger the catch function
From now we are going to change the context a little bit
for the next sections
We will be using the same MyStruct with a message and a timestamp
and two functions, one private and one public with different
arguments but the same return signature, both used to create a new
instance of the struct.
This is widely used in packages that has a private method that exposes
more options and a public one that has less arguments and maybe also
does some sanity check first
The private method just checks if the message is not empty and asks
for an explicit timestamp that will be directly injected in the returned
struct.
The public method instead only asks for a message, the time will be
time.Now(), and also checks that the message is not on multiple lines.
The
catch function
can be manually triggered by using thecatch
keyword followed by an error value, this being the onlyreturned value of a function or a pre-existing variable.
In reality this is no so much different from what we have already
discussed above, but has a slightly different purpose, so here it is.
In fact the only thing that is doing is calling a function that returns only
an error (like the json.Unmarshal(...) before), but we surely know that the
returned error will not be nil.
Discussion
Functions returning other functions
Now let's see when a function returns in the end another function
with the same return signature. Here is the original code snippet:
In this case this is what the last return statement
will be equal to with the
catch
:And if you think that you should not modify the returned
values, you can always directly use
without the
catch prefix
: the compiler will know that you do not wantto execute the
catch function
on the error.Inheritance
As previously mentioned, this is not like a defer-recover construct:
any error, even in the function called, is not handled by the most
recent version of the catch version, but only by the one inside the same
function body. In this way it be easier for an LSP (go vet, staticcheck
or others similar) to detect an improper use of the
catch function
.One thing to be pointed out is that the
catch function
should notbe treated as a function variable, because there could be problems with
inline-declared function, especially used with goroutines:
A function like that does not have to match the "parent" function return
statement in which is declared, and normally doesn't (goroutines
function usually does not return anything): this breaks the principle behind the
catch
macro style and also goes against the reason why thecatch function
must be declared for each distinct function,that is to discourage the usage of a universal function handler for every
function in the program/package.
So:
The return statement inside the catch function
It was not previously mentioned, but the return statement
inside the
catch function
is not mandatory, in fact if youwould not use the return statement, the
catch function
willbe executed normally, doing everything it contains, and then
continuing the function execution.
Again, one thing is what the language lets you do, one thing
is what is actually correct to do in a program.
An example that demonstrates that this kind of logic is already
possible is the following:
Change of the catch function
One last thing that can be discussed is the possibility to change the
catch function
behaviour inside the same function: honestly, I thing thiswould be kind of overkill, if you have the need to constantly changing the
catch function
, you would probably be better controlling each errorlike we are used to. But an example would be like this:
How the compiler should behave (conceptually)
At compile time, considering the
catch function
is valid, the compiler shouldliterally insert that function's code in the right places as can be evinced from
the differences between the old and the new code.
Conclusions
I really think that this would make people save a lot of lines of code,
while not compromising readability of the code and backwards
compatibility. I think this could be even implemented, without considering
development time, in a
go v1.21
orgo v1.22
.This change will not make Go easier to learn, but not even harder. This should
make writing Go code, both for production and for personal use, more pleasing
and less tedious.
As stated before, this should not have any performance impact on programs, thanks
to the nature of this feature, that is being a like a macro. This obviously will
put more strain on the compiler and any LSP, but I don't think this will be huge.
This is my first proposal I do in my life so sorry if I missed some
aspects and I remain available for discussions.
Hope you like this idea. Thanks for the time dedicated to this proposal
The text was updated successfully, but these errors were encountered: