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

proposal: try...or for right-shifting error handling #52175

Closed
jtlapp opened this issue Apr 6, 2022 · 39 comments
Closed

proposal: try...or for right-shifting error handling #52175

jtlapp opened this issue Apr 6, 2022 · 39 comments
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Milestone

Comments

@jtlapp
Copy link

jtlapp commented Apr 6, 2022

Here's yet another error handling proposal. I'm warming up to the idea of keeping error handling with the erroring code, but I'm still hoping we can find a way to better emphasize the primary code path.

I noticed that a particular try syntax yielded 40 positive emoticons and no negative emoticons, but a far more popular assessment of the try syntax pointed out that it was hiding error handling behavior.

I thought to employ the try syntax without hiding error handling. Under this proposal, the following code:

w, err := os.Create(dst)
if err != nil {
    return err
}
defer w.Close()

if _, err := io.Copy(w, r); err != nil {
    // additional cleanup goes here
    return err
}

... would take the following form:

w := try os.Create(dst) or return _err
defer w.Close()

try io.Copy(w, r) or {
    // additional cleanup goes here
    return _err
}

If the or keyword isn't appealing, maybe someone could suggest an alternative keyword or alternative syntax. Same goes for the proposed auto-assigned _err identifier, which would hold the last return value.

This approach moves all error-handling code to the right, so the primary code path is more visible — less cluttered — along the left. The try keyword puts some clutter on the left, but because its syntactically simple and always the same, I'm thinking we would easily gloss over it, especially with 4-space tabs when there is no other return value.

UPDATE 1: Any statement or bracketed code block can follow or, and _err would be scoped to that statement or code block.

UPDATE 2: Scroll down a few comments to see simpler variations of this syntax that drop the word try.

@jtlapp jtlapp added the Proposal label Apr 6, 2022
@gopherbot gopherbot added this to the Proposal milestone Apr 6, 2022
@jtlapp jtlapp changed the title proposal: try... or for right-shifting error handling proposal: try...or for right-shifting error handling Apr 6, 2022
@seankhliao
Copy link
Member

Please fill out https://github.com/golang/proposal/blob/master/go2-language-changes.md when proposing language changes

@ianlancetaylor ianlancetaylor added LanguageChange v2 A language change or incompatible library change error-handling Language & library change proposals that are about error handling. labels Apr 6, 2022
@ianlancetaylor
Copy link
Contributor

What is permitted to follow or?

@jtlapp
Copy link
Author

jtlapp commented Apr 6, 2022

What is permitted to follow or?

A single statement or a bracketed code block, with _err scoped to just that statement or code block.

Feel free to help me out. I'm proposing the outline of a solution and need help with the specifics.

@jtlapp
Copy link
Author

jtlapp commented Apr 6, 2022

Has the following variation been considered? We could further emphasize the primary code path by dropping the try keyword and instead suffixing a ? to tell the compiler that error handling follows:

w := os.Create(dst)? or return _err
defer w.Close()

io.Copy(w, r)? or {
    // additional cleanup goes here
    return _err
}

@jtlapp
Copy link
Author

jtlapp commented Apr 6, 2022

else might make this variation more readable and spare us any additional keywords:

w := os.Create(dst)? else return _err
defer w.Close()

io.Copy(w, r)? else {
    // additional cleanup goes here
    return _err
}

Compare with the original code:

w, err := os.Create(dst)
if err != nil {
    return err
}
defer w.Close()

if _, err := io.Copy(w, r); err != nil {
    // additional cleanup goes here
    return err
}

@andig
Copy link
Contributor

andig commented Apr 6, 2022

I find the implicitness of err or _err little transparent.

w, err := try os.Create(dst) or return err

feels clearer for me (as long as try can only address a single statement). It remains to be defined what scope err would have.

If the additional declaration or err need be saved and the often-used case of just returning the error is concerned you might shorten that to

w := failif os.Create(dst)

That would need an additional rule of returning zero values for all other return values.

@DeedleFake
Copy link

That would need an additional rule of returning zero values for all other return values.

But it also removes the ability to manipulate the value before returning. Since errors.Unwrap() was added, I've tried to make it a habit to almost always wrap errors with information about where it happened and what it was doing. I very, very rarely just directly return an error anymore.

I like the idea of reusing else. It might give me a reason to actually use that keyword now. ? would make it impossible to use that operator for nil-safe access operations, though.

I find the implicitness of err or _err little transparent.

I don't like that, either. Maybe an explicit declaration?

w := os.Create(dst) else (err error) return fmt.Errorf("create destination: %w", err)

If you wanted to reuse an existing variable, you could just do it explicitly still:

w, err = os.Create(dst)

@andig
Copy link
Contributor

andig commented Apr 6, 2022

But it also removes the ability to manipulate the value before returning.

It doesn't. It offers an additional way of expressing things when that manipulation is not needed. I've presented both together, not either or.

@andig
Copy link
Contributor

andig commented Apr 6, 2022

I don't like that, either. Maybe an explicit declaration?

I'd still keep the try around and declaring err of type error should not be necessary:

w := try os.Create(dst) else(err) return fmt.Errorf("create destination: %w", err)

try...else/fail may be followed by a statement or block. Instead of

w := failif os.Create(dst) 

that would match with

w := try os.Create(dst) 

where a try statement without else/fail/with block returns on error.

@jtlapp
Copy link
Author

jtlapp commented Apr 6, 2022

You might read this critique of the following approaches:

w := failif os.Create(dst) 

w := try os.Create(dst) 

@ianlancetaylor
Copy link
Contributor

This looks pretty similar to #33029.

@ianlancetaylor
Copy link
Contributor

Using a token like ? has been suggested many times. See #40432 for some links.

@jtlapp
Copy link
Author

jtlapp commented Apr 6, 2022

@ianlancetaylor, Yes, I saw both of those before posting this proposal, but thanks for including the links. It was the popularity of this comment that made me wonder if a slight twist on prior solutions would make people happy. So far, that doesn't seem to be the case.

My suspicion is that this community isn't concerned with the issues identified in the original RFP. Maybe a poll would ascertain what error handling issues are worth focusing on, if any. If most select, "error handling is fine as it is," then we needn't put another ounce of effort into it.

@ianlancetaylor
Copy link
Contributor

What about try f1() or return try f2() or return _err? Which error gets returned?

@jtlapp
Copy link
Author

jtlapp commented Apr 6, 2022

What about try f1() or return try f2() or return _err? Which error gets returned?

Eek! That's confusing. I think you've killed the proposal, as written.

@jtlapp
Copy link
Author

jtlapp commented Apr 6, 2022

Perhaps the way to save it is to disallow or chaining, but that's an odd constraint for an otherwise-generative language.

@jtlapp
Copy link
Author

jtlapp commented Apr 7, 2022

Hey @ianlancetaylor. Maybe your team has already considered this, but when you decide what issues to fix according to the polling on this repo, you are only appealing to your current programmer base and not making an effort to expand that base. It is possible to keep the current crowd happy while drawing in others with features they're looking for.

For myself, I keep revisiting Go when I need to do heavy lifting, but each time I'm hesitant to commit the next project because I value writing code that can be understood at a glance, without having to mentally filter out clutter. IMO, hard to read code is hard to maintain in the long run and more prone to problems, although Go certainly offsets this in other regards.

(And Go devs, I know you disagree with this assessment, but I also know you know Go turns off a good number of programmers. I'm speaking for those who are intrigued but too concerned to commit.)

@ianlancetaylor
Copy link
Contributor

@jtlapp Thanks. We try to avoid that by encouraging people who do not currently use Go, but are interested in it, to take our annual survey, where we ask questions like "why do you not use Go more?" See, for example, https://go.dev/blog/survey2020-results. Of course it's impossible to avoid all biasing. All we can do is try our best. And at the same time we recognize that the language is not for everyone.

@beoran
Copy link

beoran commented Apr 7, 2022

This proposal itself may not be it, but I think the use of the else keyword seems very interesting.

statement1 else statement2 else statement3 ...

This could have the semantics that statement 2 is executed only if statement 1 evaluates to an error, or to an assignment including exactly one error, and so on.

file, err := os.Create("foo.bar") else return err 

Versus...

if file, err := os.Create("foo.bar") {
    return err
}

@jtlapp
Copy link
Author

jtlapp commented Apr 7, 2022

@beoran, also this:

if _, err := io.Copy(w, r) else {
    // additional cleanup goes here
    return err
}

Instead of either of these:

_, err := io.Copy(w, r)
if err != nil {
    // additional cleanup goes here
    return err
}
if _, err := io.Copy(w, r); err != nil {
    // additional cleanup goes here
    return err
}

It does help a little.

@jtlapp
Copy link
Author

jtlapp commented Apr 7, 2022

It's interesting that, according to the 2020 survey, error handling is now the top remaining issue with adopting Go. For me, I'd word it differently, as I'm looking for making the primary code path clearer. I have enough of a background in C to understand that the error handling works fine.

Screen Shot 2022-04-07 at 07 28 09

@lpar
Copy link

lpar commented Apr 7, 2022

It is possible to keep the current crowd happy while drawing in others with features they're looking for.

For a while, but eventually you end up with C++.

There are very few programming languages that try as hard to keep simplicity as a feature of the language. I'm inclined to say that if you don't value simplicity over features, Go is probably the wrong language for you.

@jtlapp
Copy link
Author

jtlapp commented Apr 7, 2022

For a while, but eventually you end up with C++.

That's the slippery slope argument, which I think is more dismissive than constructive.

There are very few programming languages that try as hard to keep simplicity as a feature of the language. I'm inclined to say that if you don't value simplicity over features, Go is probably the wrong language for you.

I'm actually trying to remove syntactic features from the primary code path, in order to make the path more visible. Because of the way error handling was implemented, some new feature will be needed.

@lpar
Copy link

lpar commented Apr 7, 2022

(Quoting from the linked page) In a slippery slope argument, a course of action is rejected because, with little or no evidence, one insists that it will lead to a chain reaction resulting in an undesirable end or ends.

That's not what I'm suggesting. Nobody is saying that adding one new error handling feature will inevitably lead to lots of additional complexity. Rather, you suggested yourself that multiple new features could be added to the language to draw in new users without putting off existing Go programmers. I'm skeptical of that as a viable long term strategy. I think the experience of C++ and Perl supports the idea that repeatedly adding new features leads to a more complex language, even if each individual feature has a use case where it makes the code simpler.

I'm actually trying to remove syntactic features from the primary code path

No, you're trying to add syntactic features to the primary code path as an available alternative to other syntactic features that you don't like seeing there. But since no feature would be removed from the language, you would be increasing the language's overall complexity.

@jtlapp
Copy link
Author

jtlapp commented Apr 7, 2022

No, you're trying to add syntactic features to the primary code path as an available alternative to other syntactic features that you don't like seeing there. But since no feature would be removed from the language, you would be increasing the language's overall complexity.

You told me that I don't value simplicity, but we can talk about simplicity of the Go specification, and we can talk about simplicity of the code. I'm uncomfortable with Go exactly because it makes the primary code path too complex -- complexity that most Go devs are apparently comfortable with. There is too much boilerplate code obscuring the normally intended operation. Even the original RFP admits this undesirable complexity.

@jtlapp
Copy link
Author

jtlapp commented Apr 7, 2022

I've thought of a serious problem with the variants of this proposal. It's confusing to have both of these lines of code valid:

w := os.Create(dst) else return _err

w, err := os.Create(dst)

We really need a marker up front to tell the dev that we've skipped a return value, as otherwise the dev will have to look ahead to the end of the function call to decide what is being returned.

I'll close this issue, as both the original proposal and its variants are problematic.

@jtlapp jtlapp closed this as completed Apr 7, 2022
@jtlapp
Copy link
Author

jtlapp commented Apr 7, 2022

We'd need something like this to make the variants work, but this is so different from the original proposal that if there's any interest in it, it probably ought to go in its own thread:

w? := os.Create(dst) else return _err

w, err := os.Create(dst)

@jtlapp
Copy link
Author

jtlapp commented Apr 7, 2022

@beoran, I suspect your counterproposal is the simplest solution to the problem that this community could possibly accept. I think you ought to propose it in a new thread. If the community rejects it, I doubt there's any change to error handling the community would accept.

@simskij
Copy link

simskij commented Apr 19, 2022

For what it's worth, I think the current way of handling errors is one of the nicest out of any language I've touched, even factoring in its repetitiveness and verbosity. To me, it's not a problem worth solving.

@bronger
Copy link

bronger commented Dec 6, 2022

We really need a marker up front to tell the dev that we've skipped a return value, as otherwise the dev will have to look ahead to the end of the function call to decide what is being returned.

I do not agree. FWIW, I look at the variables left to the equal sign and know what is returned.

Moreover, I think that any proposal that covers the simplest cases is enough as we have always a decent default notation (i.e. the current way) for the more complex cases. Still, in Go the happy path is perpetually interrupted by the unhappy path, plus things like if err := myfunc(); err != nil which move the happy path (the function call) away from the start of the line.

Therefore, I’d like to point the focus on a simplified version of what is proposed above:

file := os.Create("foo.bar") else return

What this does: The last return value must be comparable to nil. If the last value is not nil its zero value, the current function returns, and the last value is returned as the last value of the current function. The other return values of the current function are their current values (if named) or zero values (if unnamed).

But if the last value is nil, all return values except the last one are assigned to the LHS.

I see the following nice properties:

  1. Covers the simplest and quite frequent use case: passing an error to the caller.
  2. Easy to explain: “Pass a non-nil error to the caller, and gobble a nil error”
  3. Does not introduce a new keyword.
  4. Is barely invasive (IMHO) and compact.
  5. Happy path in the front, unhappy path less prominent.
  6. Good to read.
  7. Proven in other languages with else return being ?, and might be attractive to people coming from such languages.
  8. The rule “every exit point is a return” still holds.
  9. The paradigm “errors are values” largely holds.
  10. Has the optional extension that one can give the returned things explicitly after the return. Personally, I’m indifferent here.

@DeedleFake
Copy link

DeedleFake commented Dec 6, 2022

The last return value must be comparable to nil.

What about functions that return a bool instead of an error? Also, proper error handling procedure these days is to return a new error that wraps the old one, usually with fmt.Errorf(), in the majority of cases, but this syntax does not allow for that. By and large I agree with your points but I don't think it covers the entirety of the problem.

@bronger
Copy link

bronger commented Dec 7, 2022

What about functions that return a bool instead of an error? Also, proper error handling procedure these days is to return a new error that wraps the old one, usually with fmt.Errorf(), in the majority of cases, but this syntax does not allow for that. By and large I agree with your points but I don't think it covers the entirety of the problem.

As I said: “[…], I think that any proposal that covers the simplest cases is enough as we have always a decent default notation (i.e. the current way) for the more complex cases.”

My takeaway from the 1000-postings issue of Robert Griesemer’s try() proposal (#32437) was (1) that the Go community is reluctant to adopt invasive changes and (2) that the current error handling is considered not really bad. Still, improvement in that area tops the wish list. Therefore I think a proposal should be cautious and not be afraid of quickly falling back to current idioms.

@bronger
Copy link

bronger commented Dec 7, 2022

@DeedleFake Your objection clearly shows, however, that my point (10.) should be deleted. If one wants to return something different than the naked err return value of the called function, it will highly probably base on err. So there is no benefit in having something after “else return”.

Your comment with bool values is also helpful: The constraint that it must be comparable to nil can be lifted by saying we just compare to non-zero and that the types of the two last return values must match. This even strengthen the paradigm “errors are values” by not special-casing any types.

I will update my main comment above accordingly.

@lpar
Copy link

lpar commented Dec 7, 2022

@bronger

I think that any proposal that covers the simplest cases is enough as we have always a decent default notation (i.e. the current way) for the more complex cases.

The problem is when a proposed new notation incentivizes bad coding practices by making them easier. In general, blindly returning error values without wrapping them or handling them is bad code, and in a good codebase it will be done rarely. If you give programmers a way to avoid doing anything with errors by adding else return to every call to pass them all the way up to main(), a lot of programmers will do just that. Just look at how many Java programmers took to catch (Exception e) {}.

That's why in previous discussions, people have repeatedly said that they want to see a proposal that makes good error handling easier, and not just something that makes if err != nil { return err } easier.

@bronger
Copy link

bronger commented Dec 7, 2022

I don’t see any evidence that returning a received error verbatim is considered bad programming. Besides, I don’t see why. There are use cases for wrapping an error, but there are other cases where the original error already says enough. Also note that each language has to be able to scale down, i.e., make simple things simple. Most Go projects are not big, yet their needs deserve being covered by syntax.

@jtlapp
Copy link
Author

jtlapp commented Dec 7, 2022

In general, blindly returning error values without wrapping them or handling them is bad code

Every function call would wrap its error before returning it? Is this something people are doing to get stack traces?

I can't say I've ever heard of this being standard practice in any programming language. Usually people provide distinct wrappings for logical boundaries that are apparent higher in the stack, and these boundaries need not correspond to each function call in the underlying implementation. Otherwise, small changes to the implementation could break callers.

I think it's good to look for a solution that invites good error handling practices, but that's only one of the problems I'd like to see addressed. I'd also like to see less error boilerplate so the main code path is clearer and more maintainable.

@lpar
Copy link

lpar commented Dec 13, 2022

Is this something people are doing to get stack traces?

No, it's something people are doing so that they don't need stack traces.

Every function call would wrap its error before returning it?

No, just most of them. Sometimes returning the error without wrapping is the right thing to do, but that's pretty rare in my experience. The majority of the time you want to provide more context about where the error occurred, what the code was trying to do, and the values that caused the error.

@bronger
Copy link

bronger commented Jan 10, 2023

I just learnt how Zig does error handling: https://ziglang.org/documentation/master/#Error-Return-Traces (BTW, ziplang also uses the mantra “errors are values”.)

Here, the try does some magic and embeds the calling history in the error value. This enables the Zig runtime to print a so-called “error return trace” which is supposed to contain more information than a stack trace.

Could it be helpful if else return did similar magic? Like, wrapping the error in a default way, e.g. adding contextual source code information.

This would provide enriched and loggable (albeit longish) error values with a succinct “else return”. And where you need more sophisticated wrapping, you can do so.

Disadvantage that comes into my mind: Error values are even more special-cased then (the error wrapping becomes part of the language).

@DeedleFake
Copy link

@bronger

Zig's error handling has been mentioned in a number of proposals before. It was discussed a fair bit in #55026.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

10 participants