Skip to content
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

feat: another improved error handling proposal #1018

Open
peter7891 opened this issue Aug 2, 2023 · 10 comments
Open

feat: another improved error handling proposal #1018

peter7891 opened this issue Aug 2, 2023 · 10 comments
Labels
gno-proposal Gnolang change proposals

Comments

@peter7891
Copy link
Contributor

I propose introducing a new keyword try to tackle the problem of if err !=nil spam.

Curretnly we have

func foo() error {
    f, err := openFile()
    if err != nil {
        .....
    }
    
    n, err := operateOnFile(f)
    
        if err != nil {
        .....
    }
}

This can quickly get out of hand.

I propose the code to look something like

func foo() error {
    f := try openFile()    
    n := try operateOnFile(f)
}

The way it will work is, try will be replaced by the normal error handling we have now, during preprocessing.
Many details are to be defined. This is just a conversation starter.

@mvertes
Copy link
Contributor

mvertes commented Aug 2, 2023

Assuming that you propose a variation of golang/go#32437, try being a keyword here instead of a Go builtin. And the code transformation being:

f := try openFile()

replaced by:

f, err := openFile()
if err != nil {
    return err    // or return ..., err where ... are func output parameters
}

with OpenFile having a signature like func openFile() (File, err), and the function calling try must return an error in last parameter ?

Could it be used to handle chained calls as well:

try a().b().c()

where all a(), b() and c() return an error as last output parameter, then reducing even more the amount of code and simply allowing chaining any functions which return a tuple (result, error). Here that would be so great but a bit more complex to implement.

@peter7891
Copy link
Contributor Author

peter7891 commented Aug 2, 2023

Assuming that you propose a variation of golang/go#32437, try being a keyword here instead of a Go builtin. And the code transformation being:

f := try openFile()

replaced by:

f, err := openFile()
if err != nil {
    return err    // or return ..., err where ... are func output parameters
}

with OpenFile having a signature like func openFile() (File, err), and the function calling try must return an error in last parameter ?

Could it be used to handle chained calls as well:

try a().b().c()

where all a(), b() and c() return an error as last output parameter, then reducing even more the amount of code and simply allowing chaining any functions which return a tuple (result, error). Here that would be so great but a bit more complex to implement.

Robert's proposal and mine are kind of based on Zig's error handling.
https://ziglang.org/documentation/master/#try
Chaining is not allowed, i believe.

Yes, try will return the last parameter.

@mvertes
Copy link
Contributor

mvertes commented Aug 2, 2023

Applying try to chained calls is a game changer here. It is still consistent with the rest of the Go language (using a keyword instead of a '?' postfix operator for flow control). The distribution of try to the elements of the call chain is natural and intuitive for the user. The control flow stills occurs in the function where try is used.

It was not considered before because "chained calls are not so common in Go" (in the FAQ discussion of Griesemer proposal), but having error processing here would be an enabler for this pattern, very common in data processing environments.

To me it is a good synthesis between functional language philosophy, emphasising function composition, and system programming, where solid error processing is often mandatory at all steps. In this context, there is a multiplying effect on the conciseness, worth to be considered:

Going from:

r1, err := a()
if err != nil {
   return err
}
r2, err := r1.b()
if err != nil {
   return err
}
r3, err := r2.c()
if err != nil {
   return err
}

to:

r3 := try a().b().c()

@moul
Copy link
Member

moul commented Aug 3, 2023

Do we agree that the target of a failing try is to panic, not to return anything, right?

@mvertes
Copy link
Contributor

mvertes commented Aug 3, 2023

It's not and never has been about panic in Go, in the original discussion and proposal by Robert and now this one from Petar. Just about improving returned error handling.

I understand the concern, where in other languages the pattern try and catch is used to handle exceptions for example in Java, closer to how panic is used for in Go, thus maybe causing confusion.

To me, the confusion quickly disappears because the form would be entirely different between Go and Java. In Go, try would be an expression keyword (returning value) followed by an expression. In Java, try is a statement (no value returned) followed by a statement block, which can not be confused with an expression.

The keyword try could also be substituted by another one if necessary, but I don't think it is the main concern for the adoption.

@mvertes
Copy link
Contributor

mvertes commented Aug 3, 2023

If we want to panic instead of returning an error using try:

func f() (err error) {
   r := try g()    // f() will return an error if try fails

   defer func() {
      if err != nil {
         panic(err)
      }
   }()

   r := try g()    // f() will now panic if try fails
}

@thehowl
Copy link
Member

thehowl commented Aug 4, 2023

My 2c; I think that any syntax-level language proposal, which has already been proposed & rejected in Go, needs to satisfactorily answer the following question: why does the feature make sense in Gno, going against the decision in Go?

The reason why I think this is worth asking for all similar proposals is not that I think the Go team is always right, but I think that any syntax-level change we add that deviates from Go creates learning curve for a developer jumping from one language to the other. The reason for the change must be solid and heavily justified, especially when it adds whole new constructs like this one.

For this reason, while I'm not necessarily outright against all language changes that deviate from Go, I do think a strong argument needs to be made relating to "why Gno is different". I think in this case, the burden of proof would lie on @peter7891, and I can see this as some potential for sense in Gno (as compared to, say, #919 -- where the only good talking point for the question I could think of was for global error variables).

To me, the reason this could make sense discussing is that in Gno, panics are a fundamental feature for smart contracts to abort the transaction; so we have a slightly different error-handling system and conventions, compared to Go. Then again, to me writing the if err != nil { panic(err) } clause is something that has grown on me and I don't mind, which is why at the time being I would still lean towards not supporting this feature; but I'm very willing to listen for any further explanations and arguments in favour.

@peter7891
Copy link
Contributor Author

peter7891 commented Aug 4, 2023

My 2c; I think that any syntax-level language proposal, which has already been proposed & rejected in Go, needs to satisfactorily answer the following question: why does the feature make sense in Gno, going against the decision in Go?

The reason why I think this is worth asking for all similar proposals is not that I think the Go team is always right, but I think that any syntax-level change we add that deviates from Go creates learning curve for a developer jumping from one language to the other. The reason for the change must be solid and heavily justified, especially when it adds whole new constructs like this one.

For this reason, while I'm not necessarily outright against all language changes that deviate from Go, I do think a strong argument needs to be made relating to "why Gno is different". I think in this case, the burden of proof would lie on @peter7891, and I can see this as some potential for sense in Gno (as compared to, say, #919 -- where the only good talking point for the question I could think of was for global error variables).

To me, the reason this could make sense discussing is that in Gno, panics are a fundamental feature for smart contracts to abort the transaction; so we have a slightly different error-handling system and conventions, compared to Go. Then again, to me writing the if err != nil { panic(err) } clause is something that has grown on me and I don't mind, which is why at the time being I would still lean towards not supporting this feature; but I'm very willing to listen for any further explanations and arguments in favour.

To be fair, the proposal that was rejected in Go was not on the syntax-level but a function call.
On the surface, it is a small difference but actually it is a big one.

@mvertes
Copy link
Contributor

mvertes commented Aug 4, 2023

My perception is that there may be a chance for Petar's proposal to Go:

The original golang/go#32437 was about a builtin, not a keyword as proposed in Petar's one. I'm sure that adding try as a keyword has also been considered by Griesemer during its study, and maybe discarded too quickly:

  • he favoured a builtin because introducing a new builtin is less intrusive to the Go spec than a new keyword (my take: doesn't matter, both are intrusive).
  • the interesting use case described in my above comments where try applies to all successive calls in a chained call expression may not have been considered before,
  • and the reason being chained calls are not a common enough pattern in Go (in the FAQ section of the Griesemer proposal)
  • here I think on the contrary that it would enable and broaden the use of such patterns: it would allow functions returning a value and an error to be chained, not only functions returning a value, and I think intuitively that it's a major improvement.

I may be wrong on the last one, and it needs to be looked at carefully.

Another proposal golang/go#60720 also introduces try as a keyword, and was quickly rejected, but for other flaws than Petar's one: not enough novelty (we have some), another extra keyword handle, that we don't have and do not think is necessary. defer already exists and may be the right solution to handle error decoration, or even panic as described above, demonstrating also orthogonality.

You may think this ship has sailed, but better error handling is still the #1 issue today in Go (and therefore in Gno too), and we are at the perfect place to experiment a bit more and go further than just exercise thought.

@mvertes
Copy link
Contributor

mvertes commented Aug 7, 2023

I noticed that in addition to distributing over chained calls, i.e. applying try to multiple function calls at once in the same composite expression like in

d := try a(x).b(y).c(z)

try also distributes over successive calls, as for closure generators where the generated function returns an error as last output parameter:

d := try a(x)(y)(z)

Also, by being idempotent for functions where the last output parameter is not an error (so in that case, function call results are returned by try, with no control flow), il allows to combine in the single try expression some functions returning errors and others not. The error handling will be done where applicable.

In the case of nested calls, try doesn't distribute and instead we have

d := try c(try b(try a(x)))

Note that the absence of additional parenthesis (as opposed to try() builtin from Griesemer) improves readability, and that the peculiar form of the expression keyword makes apparent (at least to me) that some control flow takes place in that expression.

Consider that the above single line examples replace original code blocks of 12 lines each (depending on the number of calls), and allow new functional programming patterns while retaining the error handling already present in Go. All of this was absent from the original proposal, and others that I've seen so far. To me it makes the Peter proposal even more compelling and worth being submitted to the Golang team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gno-proposal Gnolang change proposals
Projects
Status: 🔵 Not Needed for Launch
Status: Coffee Slot
Development

No branches or pull requests

4 participants