golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.95k stars 17.66k forks source link

proposal: Go 2: Error handling that is compact and composeable #55026

Closed gregwebs closed 2 years ago

gregwebs commented 2 years ago

Author background

Related proposals

Proposal

Existing proposals to improve Go errors taught us that our solution must provide 2 things:

I believe existing solutions handle the first point well but have failed on the second. With a slight variation on existing error handling proposals, we can provide the second.

Additionally, adding a compiler-recognized way of returning errors creates the opportunity to add low-overhead error tracing and thus dramatically improve debugging.

Motivation: Go's biggest problem

Recently the Go Developer Survey 2022 Q2 results were released. Now that Generics have been released, the biggest challenge for Go developers is listed as "Error handling / working with stack traces".

Exising Go code

A few years ago, Go developers described the issue with error handling and proposed a solution. As a short review, existing Go code looks like this:

f, err := os.Open(filename)
if err != nil {
        return …, err  // zero values for other results, if any
}

Existing solutions are a great starting point

New proposals allow the Go programmer to write the exact same code, but more tersely:

f := try os.Open(filename)

Where try is a new keyword or builtin that can be though of like a macro that generates the previous Go code with an early return of the error. The problem with this example is that it is trivial. Often we want to annotate the error with additional information, at least an additional string. Adding this code that modifies the error before it is returned is what I will refer to as adding an "error handler".

The original draft proposal solution used stacked error handlers, but this has difficulties around composition due to the automatic stacking and code readability since the error handler is invoked implicitly. A second proposal was put forth not long after which simply implemented try as above without any support for (stacked) error handlers. This proposal had extensive discussion that the author attempted to summarize. I believe the main problem with that second proposal is that it did not create any new affordances for error handling and instead suggested using defer blocks, which are clumsy to use with error handling.

Extending existing solutions with function-based error handling

Composing error handlers can be solved by adding a 2nd parameter to try. The second parameter is an errorhandler of type func(error) error or more precisely with generics: type ErrorHandler[E error, F error] func(E) F.

func(args...) (returnTypes..., error) {
        x1, x2, … xN = try f(), handler
        ...
}

turns into the following (in-lined) code:

func(args...) (rtype1, ... rtypeN, error) { // rtype1, ... rtypeN are the return types
        x1, … xN, errTry := f() // errTry is a new variable that is invisible or uniquely named
        if errTry != nil {
                return reflect.Zero(rtype1), ..., reflect.Zero(rtypeN), handler(errTry)
        }
        ...
}

Now we can cleanly write the following code given from the original problem statement:

func CopyFile(src, dst string) error {
        handler := func(err error) error {
                return fmt.Errorf("copy %s %s: %w", src, dst, err)
        }

        r := try os.Open(src), handler
        defer r.Close()

        w := try os.Create(dst), handler.ThenErr(func(err error) error {
                os.Remove(dst) // only if Create fails
                return fmt.Errorf("dir %s: %w", dst, err)
        })
        defer w.Close()

        try io.Copy(w, r), handler
        try w.Close(), handler
        return nil
}

ThenErr would be just a standard library function for chaining error handlers. The new example dramatically reduces verbosity. Once the reader understands that try performs an early return of the error, it increases readability and reliability. The increased readability and reliability comes from defining the error handler code in one place to avoid de-duping it in your mind at each usage site.

The error handler parameter is optional. If no error handler is given, the error is returned unaltered, or alternative mental model is that a default error handler is used which is the identity function type ErrorId[E error] func(err E) E { return err }

Supporting quick string annotation

We can support an error handler in the form of a format string.

x = try os.Create(), "os.Create annotation %w"
// The format string is translated to:
func(err error) error { return fmt.Errorf("os.Create annotation %w", err) }

The format string would be required to have %w or %v that the error will be applied to. It would also be required to be a string literal: if string re-use is desired, an error handling function can be used. Since this proposal just uses functions, there may be a way to try to support this use case with normal functions, something like:

func wrapErr(format string, v ...interface{}) func(error) error {
    return func(e error) error {
        v = append(v, e)
        return fmt.Errorf(format + ": %w", ...v)
    }
}

An issue with the function helper approach is the potential for extra overhead: try would need to be able to lazily invoke such an error handler generator.

Try an error directly

Sometimes the right-hand side is large. It may be preferable in some cases to assign the error value and then try it with an annotation.

x, err := valueAndError()
try err, fmt.ErrorHandler("valueAndError %w") // where ErrorHandler returns an error handler function
// or with direct support for a format string
try err, "valueAndError %w"

Similar to today, this style will require the usage of a linter to ensure that the error is checked, and it pollutes the existing scope with a variable that otherwise isn't needed.

Supporting prefix and suffix handlers

It is possible to support error handlers in the prefix position as well. Although this is sometimes convenient, a Go programmer knows that it isn't always better to have multiple ways of doing the same thing- this might be a bad idea.

x = try "valueAndError annotation %w", valueAndError()
x = try handler, valueAndError()

Conclusion

This proposal allows for:

Please keep discussions on this Github issue focused on this proposal rather than hashing out alternative ideas.

Appendix: Citations

Appendix: keyword or builtin

I personally have a preference for making try a keyword, but I would be happy as well it were a builtin. The difference in new code is just a pair of parenetheses. The builtin approach does have the advantage of avoiding breaking existing code that uses try as a variable name, which does occur in the Go compiler code base. However, go fix should be able to perform automatic renames to allow for smoother upgrades.

Appendix: prior art

The err2 library implements try as a library function! Error handlers however, are created with defer: this disconnects the error handlers from their actual error. So this library requires code review to ensure that errors are actually being handled correctly or special linter rules that understand the library.

I forked err2 to make it more similar to this proposal. I am calling the result err3.

There have been proposals for dispatching on or after an error value assignment. These are quite similar to this proposal but suffer from requiring assignment and not being usable as an expression. This proposal is still open and may be the clearest one. Other closed proposals:

There has been a proposal for a postfix catch, but it also includes an error assignment on the LHS.

There was a proposal for a postfix handle that used a prefix try and an implicit `err variable. And other proposals for a postfix error handler:

There has been a proposal similar to this one with what I would consider the right idea. Unfortunately the proposal was very light on detail and otherwise rushed with mistakes in the examples given: https://github.com/golang/go/issues/49091. It had a different syntax: instead of a comma, it used a with keyword. I would be fine with introducing a second keyword, but it does cause more backwards compatibility issues.

I made a similar proposal with a different syntax before there was a process for submitting proposals for changes to the language. The github issue was closed suggesting that I create a blog entry instead. That proposal confused many initial readers by showing problems with existing proposals in code instead of just creating a new proposal (it was later edited). Since then, the additional concept of format strings and error traces has been added (in addition to using try instead of ?).

Appendix: generic enumerations

Now that Go has Generics, we might hope for this to get extended to enumerations and have a Result type like Rust has. I believe that when that day comes we can adapt try to work on that Result type as well just as Rust has its ? operator.

Appendix: Learning from other languages

The try proposed here is very similar to Zig and Swift.

In addition to Swift using try similar to this proposal, Swift also adds try! (panic) and try? (return nil) variants which Go could consider doing adding in the future as well.

Zig has try and also a second keyword catch that is the same as this proposal. try is used with no error handler and catch is for adding an error handler. I avoid this approach in favor of a single try keyword with a 2nd parameter to avoid introducing an additional keyword. Additionally, the catch keyword in Zig comes with a special syntax for anonymous functions. I believe it is best to use existing Golang syntax for anonymous functions (Zig has no such construct). Having a terser syntax for anonymous functions would be great, but I believe it should be universally applicable. I do think it is possible to allow error handlers to not require type annotations.

It could make sense to use a catch postfix like Zig instead of a try prefix, just as Rust uses a ? as a postfix operator. Personally I would be fine with this option (either catch or ?). I think though that when a line is very long it is useful to see the try at the beginning instead of hunting for it at the end.

Zig is very Go-like in its philosophy and could be a great source of ideas of language evolution. It has an error tracing feature that would solve a major pain point in Go error handling: tracing errors. Also an interesting feature called Error Sets that Go could evaluate. Outside of error handling, but related to a common source of crashes in Go, Zig's implementation of Optionals is something that Go should evaluate as well.

Appendix: returning a type other than error

In Zig, catch allows the return of a type other than an error, for returning a default value in case of an error.

const number = parseU64(str, 10) catch 13;

My feeling is that this use case is not common enough to justify support for it. Most of the time I would want to at least log something before discarding an error.

Appendix: code coverage

There are concerns about code coverage. It may be a significant burden for line-oriented code coverage tools to figure out how to tell users if the error paths are getting exercised. I would hate for a helpful tool to hold back back language progress: it is worth it for the community to undertake the effort to have code coverage tools that can determine whether try is getting exercised.

Appendix: in-lining try

This issue is addressed in a comment reply.

Appendix: examples

import (
        "fmt"
)

func existingGo (int, error) {
        // The existing way of returning errors in Go:
        // 3 additional lines to return an error.
        // If we need to do more than just return the error, this is a pretty good system.
        // However, in the simple case of just returning the error, it makes for a lot of ceremony.
        x, err := valAndError()
        if err != nil {
                return 0, nil
        }
        return x + 1
}

func valAndError() (int, error) {
        return 1, fmt.Errorf("make error")
}

func newGo() (int, error) {
        x := try valAndError()

        // Add a handler inline, by itself not much better than the existing way
        x = try valAndError(), func(err error) error {
                return fmt.Errorf("valAndError %w", err)
        }

        // declare and compose handlers separately
        handler := func(err error) error {
                return fmt.Errorf("valAndError %w", err)
        }

        // handler function annotation
        x = try valAndError(), handler

        handler2 := func(err error) error {
                return fmt.Errorf("part 2 %w", err)
        }

        // compose handler function annotations
        // requires a ThenErr library function
        x = try valAndError(), handler.ThenErr(handler2)

        // We can support putting a handler on either side.
        x = try handler, valAndError()

        // The common case wraps an error with a string annotation.
        // So we can support giving a format string as the handler.
        // Wrap an error with a format string. %w or %v is required.
        x = try valAndError(), "valAndError %w"

        // We can support putting it on either side.
        x = try "valAndError %w", valAndError()

        // Try can also be used directly on an error.
        x, err = valAndError()
        try errors.Wrap(err, "valueAndError)"
        // However, that has overhead that using a builtin format string argument could eliminate
        try err, "valueAndError %w"

       // Format string annotations with additional values should use a handler
        i := 2
        x = try valAndError(), func(err error) error {
                return fmt.Sprintf("custom Error %d %w", i, err)
        }

        // Using a custom error type
        // For convenience the error type can define a method for wrapping the error.
        x = try valAndError(), theError{ num: i }.Wrap
}

type theError struct{
        num int
        err error
}

func (theError) Error() String {
        return fmt.Sprintf("theError %d %v", theError.num, theError.err)
}

func (theError) Wrap(err error) theError {
        theError.err = err
        return theError
}

func (theError) Unwrap() theError {
        return theError.err
}

Costs

Harder to learn the language spec because users must learn a new keyword. However, when verbose error handling code is removed, beginners will be able to read and evaluate Go code more quickly and learn Go faster.

The costs are discussed in detail elsewhere

All, but those that re-use Go libraries may just need to upgrade their library usage? I think these tools would then have less source code to analyze and thus run more quickly. It will mostly remove the need for linters that check for proper error handling.

Without handlers I would think it could be reduced because the compiler can generate code that previously would have been hand-written- the generated code does can be optimized in some ways, for example by not needing to be typechecked. It will reduce the error linting time (see above) for those of us that run linters right after compilation since checking for proper error handling will be easier. Supporting format strings and validating them to contain %w or %v at compile time would have some cost, but we could forego that validation. I am not familiar with the compile time costs of using functions for error handling.

None without handlers. The potential cost of supporting format strings is discussed in the proposal. I would think that the overhead of the error handlers can be removed by in-lining the functions? Normally once a function is returning an error, runtime performance is not a concern, so the most important thing is reducing the runtime cost of the happy path. It seems like the happy path should be undisturbed if handlers associated with the error path can be evaluated lazily.

I started a branch that gives some idea of some of the changes required, but keep in mind that it is incomplete and already making implementation mistakes.

This can be mostly implemented as a library, this is done here. However, it has several limitations that can only be solved with compiler modifications.

Try(f(), handler) with no return is fine but with a return things get awkward due to how Go's multiple return values are not programmable and generic varags are not available. The result is : x := Try1(f())(handler).

DeedleFake commented 2 years ago

First of all, I don't think that go fix is a valid way to avoid the Go 1 Compatibility Guarantee. The guarantee is that existing working code will simply continue to work. Users shouldn't need to run go fix, and, worse expect all of their dependencies to run go fix, just to keep things working, especially not when there are other solutions that don't have that issue. The problem would be mitigated to a fair extent by the go directive in go.mod, but if a solution can avoid the problem in the first place, I think that's better.

From what I've seen, most issues with error handling seem to revolve around an inability to apply DRY to error handling mostly itself stemming from an inability to return from the calling function from an inner function easily. A lot of languages avoid this by using an exception system that, much like panic(), causes the entire stack to return continuously until a catch, or a recover() in the case of a panic(), is encountered. This has massive issues, but it does solve that particular problem.

You mention that try is similar to a macro, and I wonder if the problem would simply disappear on its own if Go had its own hygenic macro system? A macro can return from the calling function, since it's not really 'called', and regular usage of it to remove the error return from a function would result in effectively the same thing as an exception system but without all of the hidden magic. There could be a series of standard error-handling macros such as try!() (Yields all but its last argument unless the final argument was a non-nil error in which case it would return from the caller with that error. Other returns would need to be possible via the macro system.) and explain!() (Same thing as try!() but automatically wraps non-nil errors using fmt.Errorf().) and maybe even catch!() (Does what your Zig example from above does.), and then if someone needed repeated custom error handling, they could just write their own. Complicated cases could just keep using manual ifs the way existing code does. Introducing macros and using them for error handling would also avoid the backwards compatibility problem of a keyword.

ianlancetaylor commented 2 years ago

The biggest concern about the earlier try proposal was the unexpected flow of control change in the middle of an expression. That is try f1() + try f2() can return in the middle, a potential return signaled only by the presence of try. This proposal seems to have the same issue.

DeedleFake commented 2 years ago

What if try was only possible at the outermost part of an expression? In other words, it would only be available as the top of a lone expression or the first thing on the right-hand side of an assignment or channel send? However, if a statement contained a try expression, the expression would also be able to use some indicator inside of the expression to pass inner parts back up. That way the presence of try would always be prominent, making it easy to see that an expression might return.

I'm not a huge fan of this particular syntax, but here's a few possible examples of legal and illegal statements:

x := try { f1() } // Legal.
x := f1() + try { f2() } // Illegal.
x := try { f1() } + try { f2() } // Illegal.
x := try { f1() + f2() } // Legal.
x := try { check f1() + check f2() } // Legal.

This might just be too confusing, though.

gregwebs commented 2 years ago

The biggest concern about the earlier try proposal was the unexpected flow of control change in the middle of an expression. That is try f1() + try f2() can return in the middle, a potential return signaled only by the presence of try. This proposal seems to have the same issue.

@ianlancetaylor I tried to follow the previous discussions and address all relevant points. This actually caught me off-guard, so thanks for bringing it up. At least, I read the summaries and this wasn't explicitly mentioned as one of the main concerns. I do see now that one of the summaries linked to a discussion that contained this as part of the text of the #2 point.

I think this concern is a red herring, let me explain why.

Discomfort with inlining try is quite similar to discomfort with reading lines with nested function calls. What happened is that someone was quite comfortable with inlining try (and probably nesting function calls) and created examples of transforming existing Go code and maximalized inlining of try. For example the following was given:

        mimeHeader, err := tp.ReadMIMEHeader()
        if err != nil {
                return nil, err
        }
        req.Header = Header(mimeHeader)

transformed to:

        req.Header = Header(try(tp.ReadMIMEHeader())

However, this is just gratuitous inling- a direct translation is:

        mimeHeader := try tp.ReadMIMEHeader()
        req.Header = Header(mimeHeader)

That is, try does not create any new function nesting issues: users that like function nesting create them. It is true that with try there will be more opportunities for nesting function calls for those that want to. In many cases the biggest blocker of direct function chaining now is having to handle errors in between function calls. With a smooth error handling option in place there are more opportunities for chaining function calls. This is actually a good sign: it indicates that Go code is easier to compose. The community will have to figure out how to best deal with easier function composition, but making function composition readable is not actually a new problem for Go code.

Today the problem of nesting function calls is an issue that is handled by not nesting function calls when it impairs readability. When it improves readability, function calls are placed on separate lines and an intermediate variable is declared. try won't force anyone to stop doing this.

In summary, this is just an issue for a style guide to mention if it does not already. I would be happy to create some linter rules to warn users about gratuitous function chaining, particularly in the context of using try, if existing rules don't work well enough.

gregwebs commented 2 years ago

I have made some minor edits to the proposal text just for making the text a little clearer and adding a few more links, including a link to my prior comment from the proposal. There is no change in substance, but if you are reading it for the first time from an email it would be preferable to come to Github to read it instead.

Thanks to all that have taken time to read the proposal so far! Please try to keep discussions focused on this specific proposal. @DeedleFake I would love a macro system that I could write try with, but that's a whole different proposal! Perhaps you could start putting together a proposal to at least provide compelling reasons for macros (note that go generate is effectively a very limited macro system) starting with a wiki page, or mail list posting, etc and tag me on it.

atdiar commented 2 years ago

Unfortunately, I don't find the examples very compelling. Currently, the consistency of the if err! = nil makes it easily recognizable at a glance.

I am more inclined to keeping things simple and consistent if a bit boilerplate-y. Correctness over stylistic considerations.

apparentlymart commented 2 years ago

While reading this proposal I developed a (probably incomplete) mental model of what you were describing which I wanted to repeat here in case I misunderstood (please let me know!) and in case it's helpful to others who prefer to think in "rules" rather than in examples.

What I understood:

I think the above covers the main proposal text at the time I'm writing this. There's subsequent discussion about the possibility of making try instead behave as if it were a part of the assignment and channel send operators, or in other words that this keyword would be permitted only after :=, =, and <-. As far as I can tell, that modification doesn't affect the rules for try I described above, and instead just restricts where the keyword try would be accepted at all.


With my understanding above in mind, I have some thoughts and follow-up questions (which are likely to be invalid if my understanding above was incorrect):

I'm sorry for asking a bunch of unrelated questions in one comment; I know that GitHub's comment UI doesn't really work very well for this sort of multi-threaded discussion but I also don't want to spam the issue with lots of separate comments. :grimacing:

gregwebs commented 2 years ago

@apparentlymart good questions. See my next comments for a formalization of the transformation.

It's not clear to me from the proposal if the intent was to require V to be specifically a string literal

Good question. If someone wants to re-use a string variable then it is probably becoming worth the effort of using a function, so we may not need to support a string variable. I think it would simplify the first implementation to only allow a literal, and then we could consider relaxing that in the future.

There's subsequent discussion about the possibility of making try instead behave as if it were a part of the assignment and channel send operator

This is good for the simple case, and I would gladly take it over the status quo, but adding error handling to this is difficult.

If I have a weird function func foo() (error, error), am I allowed to write try try foo() to shorthand handling both of those errors in a single statement?

That should be allowed according to how try operates. I haven't ever seen a Go function that returns two errors though. A better example might be to try to pair the try keyword with a different Go keyword. I am glad you are trying to poke holes in this- I agree that if we think through all possible use cases we might consider it necessary to use parentheses, in which case we can implement it as a builtin.

With that said: if this ends up being syntactically similar to a panicking function

A Go panic is equivalent to an exception. An exception is very different because it unwinds the stack to find a handler up the call chain. This try returns directly to the call site.

Might it be better to select a different keyword

I personally don't care what word we use. We could tentatively accept a proposal with the provision that there will be a further decision as to the exact word that is used and whether it is a builtin or a keyword.

gregwebs commented 2 years ago

... proposing that try would annotate the errors it returns with some sort of callsite location information. Using the naming scheme I employed for my description above, would you think of that as being an extra wrapper around the function given in T which implicitly adds another level of wrapping to capture the trace information? Does that then mask the true error type returned by T?

That is what some error packages do now, and it works fine since we have standardized on wrapping now. However, it isn't necessary in this case because we are implementing this in the compiler itself. You can follow the link to read how Zig implements it: a function that returns an error will have the caller pass in a hidden stack trace variable that try will write to before returning.

Thanks again for the good questions. I will try to roll up some of these Q&A into the proposal.

gregwebs commented 2 years ago

Sorry, I wasn't thinking straight when I wrote down the translation. Also, I don't think we should avoid assignment to non-error variables when there is an error. So the translation would be.

func(args...) (returnTypes..., error) {
        x1, x2, … xN = try f(), handler
        ...
}

turns into the following in-lined code:

func(args...) (rtype1, ... rtypeN, error) { // rtype1, ... rtypeN are the return types
        x1, … xN, errTry := f() // errTry should be invisible or have a unique name
        if errTry != nil {
                return reflect.Zero(rtype1), ..., reflect.Zero(rtypeN), handler(errTry)
        }
        ...
}
apparentlymart commented 2 years ago

Thanks for the nudge to read the Zig documentation! I'm sorry I didn't follow that link on my first read; I was taking a broader look then and so it wasn't clear to me quite how important that link was to the subsequent discussion.

I don't yet have any experience with Zig so I did a bit of homework to understand how Zig thinks about errors and how that is similar or different from Go. Here are my notes in case they are useful to others reading this proposal who are also not Zig-experienced:

My intention in pointing out these differences is not to argue that Zig's design is not applicable to Go, but instead just to identify some areas where the design might need to be altered to fit into Go's type system and conventions.

The need to generate different code for fallible vs. infallible does seem to conflict with the fact that "fallible" exists only as a convention in Go. It does seem possible in principle for the compiler to treat functions where the last return value is of type error as special, but subjectively it feels to me like that goes against the grain of Go's language design so far. Perhaps it's reasonable to argue that any sort of first-class error handling requires treating error-returning functions as special, rather than just a convention. What do you think?

DeedleFake commented 2 years ago

It does seem possible in principle for the compiler to treat functions where the last return value is of type error as special,

Would that mean that something like errors.New() or fmt.Errorf() would be fallible?

gregwebs commented 2 years ago

The need to generate different code for fallible vs. infallible does seem to conflict with the fact that "fallible" exists only as a convention in Go. It does seem possible in principle for the compiler to treat functions where the last return value is of type error as special ...

Yes,try requires this convention. So doing this with error tracing as well isn't an issue. This convention is extremely universal in Go code, to the point that I would expect future proposals that attempt to formalize this convention. But as mentioned in this proposal, I think the right time for this is when generics support enumerations and we can have a Result type like Rust.

Would that mean that something like errors.New() or fmt.Errorf() would be fallible?

I think the practical implication of this question is: will the error trace go down all the way into a function such as errors.New(). The answer is yes for a simple implementation. However, I think the implementation can be developed to eliminate that from the trace. A function that returns just an error (and can never return a nil) can probably be automatically removed from the trace.

gregwebs commented 2 years ago

There are 2 approaches for instrumenting error tracing. The one I originally had in mind was to only use try to instrument tracing, in which case errors.New would certainly not be instrumented. But the other approach we just discussed would be to automatically instrument functions even without the usage of try. It does seem that error tracing could be made automatic based on the established Go convention of returning error in the last position, in which case there would be no dependency on try and it could be removed from this proposal and made into an entirely separate proposal.

apparentlymart commented 2 years ago

@gregwebs I was trying to focus on the Zig-like approach in my last comment because I was responding to your callout that I hadn't properly read it the first time. :grinning:

But indeed there do seem to be at least two approaches here, and possibly three if you count the two variants I mentioned in my original long comment here. Here's a summary of those three in the hope of making it clearer for other readers:

  1. Zig-style: the compiler generates additional code to capture error traces out of band of the actual error values, using hidden arguments to any fallible functions.

    Although not explicitly stated yet, I'm assuming this would include allowing func main() to be a fallible function and making the runtime print out the error trace if that function does indeed return a non-nil error.

    If user code needs to interact with the error trace then it must do so via some intrinsic-function-based API that returns the current function's out-of-band trace object, because that information would not be directly accessible through the error value itself.

  2. try keyword automatic error wrapping: as a hidden extra step after evaluating the error transformation function, try would then wrap that result in a special built-in error type which captures some sort of trace information.

    That special trace error type would implement the existing error unwrapping interfaces so that calling code can peel off the wrapper in the conventional way to obtain the original error value inside it.

    Presumably if a particular error is returned through multiple levels of try then either each try adds another level of wrapping or the wrapping code notices that the error is already of the special error type and shallow-replaces the wrapper with a new wrapper that aggregates the two levels of tracing together. Either way, the error that emerges at the other end allows enumerating all of the try operators that the error returned through.

  3. try keyword with explicit error wrapping: the error transformation function can take an optional extra argument which represents trace information generated by the try expression.

    A transformation function which takes that extra argument can therefore incorporate the information from it into the error value it returns in whatever way makes sense for that error type. The result can therefore be of any dynamic type, without any implicitly-added wrapper that callers must peel off to recover the original error.

I suppose there is also an implied additional option of not trying to support "error tracing" as a first-class feature at all, and instead leaving that as something to be handled by library code using existing runtime mechanisms. I've not thought too hard about what the implications of that would be, but at first glance it does seem like an automatic tracing mechanism is not really a crucial part of the proposal, and that having the shorter syntax without the explicit control flow constructs is the main goal.

(I imagine this discussion should aim to reduce this to a single specific proposal -- whether one of the three above or another approach -- but it'll probably take a little more discussion to figure out which of these has the best combination of pros and cons.)

gregwebs commented 2 years ago

I favor the Zig out-of-band approach because

I removed error tracing from this proposal. Automatic error tracing would be an amazing new feature for Go. Let's not discuss it anymore on this proposal though other than to facilitate creating a new proposal where it can be discussed. Link your new proposal here or contact myself or us on this issue if you want help making a new proposal.

gregwebs commented 2 years ago

I forked the err2 package to implement this as a library that I am calling err3. However, it has several limitations that can only be solved with compiler modifications.

As a result, the CheckX api is okay, but the TryX api cannot take a handler as an argument. This is solved by returning a function that handlers that are applied to, which is not an ideal API. Try(f(), handler) with no return is fine but with a return the API is: x := Try1(f())(handler).

bitfield commented 2 years ago

Now that Generics have been released, the biggest challenge for Go developers is listed as "Error handling / working with stack traces".

“Biggest” here might need a little qualification. 11% of those responding to the ”biggest challenge” question mentioned error handling, but only 665 people responded to that question. So in terms of actual numbers, roughly 73 people out of the 5,752 surveyed find error handling (in some way) their biggest challenge in using Go. That is, again roughly, 1% of respondents.

So another, I accept less persuasive, way to interpret this data would be to say that around 99% of survey respondents do not find error handling a significant challenge in using Go.

This has no bearing on the merits (or otherwise) of this specific proposal, of course, but I suggest it has some bearing on whether it's worth anyone's while to make any proposals about changing error handling. To say ”it's the biggest challenge” is to run the risk of being misinterpreted as saying “it's a challenge for a large proportion of Go users“, which the survey data suggests very strongly is not the case.

steeling commented 2 years ago

+1 to removing the auto tracing from this proposal. I'd also recommend dropping ThenErr, as it doesn't seem critical to the core concepts here, and can be added at a future date if desired. My fear is that it detracts from the core of the proposal if people take issue to that specific component. Maybe framing it as a "potential future direction" could help.

steeling commented 2 years ago

In similar proposals the concern was raised that by "sandwiching" the function call with a keyword (try), and a handler function, it detracts from the readability of the program, ie:

w := try os.Create(dst), func(err error) error {
        os.Remove(dst) // only if Create fails
        return fmt.Errorf("dir %s: %w", dst, err)
}

makes the os.Create call a bit harder to see. One thought to improve this would be to make the try keyword implicit if a handler is provided such that the above becomes

w := os.Create(dst), func(err error) error {
        os.Remove(dst) // only if Create fails
        return fmt.Errorf("dir %s: %w", dst, err)
}

However that would have some syntax collision with multiple assignments, which would necessitate the need for a syntax change. A lot of symbols/keywords would work, but I'll use the @ symbol for the sake of the argument, so we now have:

w := os.Create(dst) @ func(err error) error {
        os.Remove(dst) // only if Create fails
        return fmt.Errorf("dir %s: %w", dst, err)
}
ianlancetaylor commented 2 years ago

Thanks. This proposal doesn't address one of the biggest concerns about the try proposal (#32437) which is that the flow of control can be buried inside a complex expression. It does address other concerns about try, but not that one. Therefore, this is a likely decline. Leaving open for four weeks for final comments.

steeling commented 2 years ago

"Buried flow control" feels sort of subjective IMO. At its core, it's a commentary on readability that claims having explicit flow control is more readable than cluttered, albeit explicit if err != nil {...} statements. I'd argue that it's up to the user to write readable code, and certainly more readable code can be produced by introducing try as a tool. If a user decides to write difficult to read code, they can already do so without the use of try.

Plus as commented in that proposal, panic affects flow control already.

Lastly, go-vet, or go-lint, or even the widely used golangci-lint can add checks to recommend/restrict usage of buried try calls for users that want to protect against it.

ianlancetaylor commented 2 years ago

Understood. Still, that was one of the primary reasons for rejecting the try proposal, and this proposal has the same concern. We aren't going to revisit that decision unless there is new information.

apparentlymart commented 2 years ago

The subsequent discussion included the possibility of making try be allowed only in some specific contexts: normal assignment, channel writes, and as the first token in a statement.

try EXPR
foo := try EXPR
ch <- try EXPR

Would that be a sufficient constraint to ensure that the control flow implied by this feature is not buried inside complex expressions?

(I don't really have strong feelings either way about this variant of the proposal, so I'm raising it only in the hope of moving discussion forward towards something that might qualify as "new information", and not as a vote for or against it.)

gregwebs commented 2 years ago

Thanks. This proposal doesn't address one of the biggest concerns about the try proposal (https://github.com/golang/go/issues/32437) which is that the flow of control can be buried inside a complex expression. It does address other concerns about try, but not that one. Therefore, this is a likely decline. Leaving open for four weeks for final comments.

@ianlancetaylor I believe you already raised this concern and I responded to it here. And another user made the same point.

But I haven't seen any reply to this point (solve it with a style guide) it in this discussion or elsewhere. I may have missed it- there are hundreds of comments. Rather than not responding to this point and closing the proposal, I was hoping for a thoughtful response as to why this cannot be handled by a style guide, gofmt, and linters.

Let me re-phrase the point again here: It is up to programmers (with the aid of style guides, formatters, and linters) to write their code in a readable way. try does not cause things to be buried in expressions, programmers do.

You can write code the exact same as now but just turn the if expression into 1 line instead of 3.

x, err := f()
try err, "f failed"

Sometimes this is the best because the code is already indented and f() is a larger expression that takes up a lot of space.

I agree with @apparentlymart that the usage of try can be limited to good style, but I think as a base case that could be done with gofmt or golangci-lint rather than putting restrictions into the compiler itself.

BTW, I updated my library implementation of this to be named try. It is quite a productivity booster around error handling once you get used to it. Unfortunately because it is a library that cannot implement try properly there are some awkward things to get used to.

I am beginning to use this code in the real world. I have < 100 usages and so far, but I haven't found a single opportunity to do anything other than have it be the first expression on the RHS of an assignment or the leading statement of the line.

I need to make a tool to transform error handling in a code base and am hoping to leverage the go fix tooling. I think this logic around recognizing error handling could be ported to gofmt, but even if not, re-formatting to use an intermediate variable seems like it should be possible?

ianlancetaylor commented 2 years ago

@apparentlymart Restricting try to only certain uses makes the language more complicated and is non-orthogonal. Go aims to be simple. Adding extra rules that everybody most learn is not simple.

@gregwebs Sure, people could use a style guide. But not everybody will. Somebody reading unfamiliar Go code must always be aware of the possibility that a try appears embedded in a complex expression.

I want to emphasize again that we have to be able to make decisions and then move on. If we must reconsider these decisions over and over, we can't make progress. We've made a decision here.

apparentlymart commented 2 years ago

The decision I understood as having been made, and was aiming to respond to, is that it's not appropriate introduce any language constructs which have an effect resembling return in an expression position, because in Go we are accustomed to only statements affecting control flow. (With the notable exception of the panic function, which is exceptional for various reasons already discussed at length in #32437 and elsewhere.)

Is it true that the there has been a more general decision that values of type error should never receive special treatment in the language, and should instead remain purely a matter of convention and idiom?

I can understand the (assumed) objection that any construct which is used only to deal with values of type error (or is otherwise focused only on error handling) is inherently non-orthogonal in a language where errors are just values and all of the patterns around them are a matter of convention. I just want to confirm whether that is indeed a specific objection.

My goal is to help those who wish to make future proposals in this area understand what bar they need to meet to be considered new information rather than a rehash of previous discussion, or to be clear that such proposals are simply inconsistent with the language design goals and so not worth making in the first place. I find it upsetting to see so many people spending time proposing, counter-proposing, and discussing in a direction which is (as far as I can tell) not a desired language design direction; I would like to help folks making these proposals to "move on" if this is a settled matter already.

I acknowledge that language design is a hard problem requiring consideration of many different conflicting goals. I'm not meaning to imply that it's possible to rule definitively against all solutions that might hypothetically be proposed, but I hope that "avoid any language features that treat error-typed values as special" is narrow enough to be answerable as a potential design principle that could help divert effort to proposals which take a different approach, along with considering the already-understood "no new control flow inside expressions" principle.

Thanks!

ianlancetaylor commented 2 years ago

The decision I understood as having been made, and was aiming to respond to, is that it's not appropriate introduce any language constructs which have an effect resembling return in an expression position, because in Go we are accustomed to only statements affecting control flow.

Yes.

I would add that it's not going to be OK to introduce a language construct that can change flow control in an expression, and then say "people should only use it in these ways, as a style guideline." And it's not going to be OK to introduce a language construct that is parsed as an expression, and then say "but this thing that looks like an expression is not permitted where we otherwise permit any expression."

Thanks.

apparentlymart commented 2 years ago

Thanks for the additional clarification about the objection, @ianlancetaylor.

My conclusion from this is that it might still be acceptable to propose new statement-level syntax for error-specific control flow. It is specifically anything "expression-like" that is ruled out by previous decisions. I think that's a helpful constraint for future proposers to consider, though I do now wonder whether there's a better place to record that information than in this specific issue's discussion thread! Is there anything like a central "decision log" for proposals team decisions related to particular common subjects that have accumulated many separate proposals?

I suspect that part of why these seemingly-redundant proposals keep coming up is that it's often hard to understand exactly what has been decided before, because the decisions are a relatively small detail in otherwise-long discussion threads for specific proposals.

ianlancetaylor commented 2 years ago

I agree that new statement level syntax is a possibility.

There is some general discussion in #40432.

gregwebs commented 2 years ago

I suspect that part of why these seemingly-redundant proposals keep coming up is that it's often hard to understand exactly what has been decided before, because the decisions are a relatively small detail in otherwise-long discussion threads for specific proposals.

This is exactly what happened to me. I read through hundreds of comments and many other proposals. There was one thread (among many others) with this complaint, and one summary that mentioned the thread. The summaries indicated that it was a collection of issues as a whole that lead to the proposal being rejected rather than there being any single issue that had been decided on.

If these decisions, or any other hard constraints were listed somewhere, then we might be able to have a well-functioning community proposal process. This is the second time I have tried to navigate the process. The first time My proposal was closed- I found out there was no process and that you had to be a member of the Go team to make a proposal. This time there is a process- but it still seems that in at least some cases you have to be a member of the Go team to understand what things have been decided on.

Every project needs help with staying organized and writing things down. I think the community could help. I would gladly submit documentation of this decision if I knew where. Maybe here? https://github.com/golang/proposal/blob/master/design/go2draft-error-handling-overview.md

gregwebs commented 2 years ago

Okay, I see the issue now that @ianlancetaylor linked to about error handling mentions this decision as "major". Unfortunately, I missed that issue. Perhaps we can edit the other issues/proposals and link to it at the top of the description?

ianlancetaylor commented 2 years ago

Error handling in particular has unfortunately become an enormous mess, as can be seen by looking at https://github.com/golang/go/issues?q=label%3Aerror-handling+ . It's an area where there are a vast number of variants of approaches, and nobody can keep up with all the details of all the earlier discussions. I wrote #40432 as an attempt to put a bit more order on the topic, but I don't know that it has helped much in practice.

I have no objection to adding a mention of #40432 on existing issues, but it sounds like a tedious process. Perhaps I will try to remember to mention the issue when adding the error-handling label to issues.

gregwebs commented 2 years ago

So it is normally poor form to discuss alternatives on a proposal. However, this proposal was apparently dead on arrival so I will attempt to explain the minimal adaptations to adapt it to the constraints that we now understand. If a new Phoenix arises from the ashes of this discussion, a new proposal will be needed.

An intermediate err variable

The easy way to satisfy the constraint of not being an expression is to remove the ability for try to return anything (that is what an expression does). So what you have left is this proposal, but only the cases where try is only given an error. So this proposal proposed:

x := try f(), "f failed"

but instead we could only allow for:

x, err := f()
try err, "f failed"

After using my library a bit, I actually prefer this approach the most. And to automatically translate existing code, it is often necessary. A lot of the reason why it was nice to hide the err variable in the original proposal just comes down to the Golang's issues around variable reuse during assignment. However, these issues are already present and can be solved with linters.

Error handler functions

Above I show the special string to error handler version mentioned in the proposal. But let's discuss error handler functions. We need to choose between having the second argument be a lazy expression of type error or be a handler function of type func(error) error. lazy:

// concretely
x, err := f()
try err, fmt.Errorf("f failed: %w", err)
// generally
x, err := f()
try err, handler(err)

It's not quite DRY to use this same variable multiple times. With go's scoping issues, I could see this ending up creating a defect where the variable used in the handler is different that the one given to try. If we support an error handler function func(error) error, then we have just:

x, err := f()
try err, handler

The lazy form can be used this way by currying the function so that it needs error to be applied.

It is also possible that the keyword can support both a lazy error expression and a handler function func(error) error.

Assignment

Even if we support the intermediate variable form, it isn't necessary for a function that returns only an error to assign it to an intermediate variable.

try f(), "f failed"

It would be weird to not support this. But supporting this means that when f() is changed to returns a result in addition to an error, we will have to create a second line. This type of workflow already happens in Golang with respect to errors, but if we are going to be consistent we would need to require assignment. The original proposal suggests this form:

x := try f(), "f failed"

This makes try look and behave like an expression, which has been decided against. Is there a way instead to make the concept of a "try assignment" statement?

try x := f(), "f failed"

But we still must support the intermediate error variable form as well, because that is best for readability in many cases, so we will end up with this keyword accepting 2 very different things as its first argument.

Assignment and Lazy handlers

There is a way to force users to use try in only one way. That is to make the error handler lazy (as mentioned above) and require an error handler argument. This way err must be assigned. To fully commit to this approach, we would need to not have the special string error handler and require an error handler. So the simplest form of just returning the error would be:

err := f()
try err, err

This looks strange, although it is the same strangeness of today with if err != nil { return err }. But it's kind of strange that a new, better language construct would look like this.

A way forward

I think we have to allow for the single line try f(), ... when the function only returns an error. Adding a new return variable to f is painful, but it is more rare that a function returns only an error and then adds a non-error result. Normally a function has 1 non-error result and then adds more.

// just error can avoid assignment
try f(), "f failed"

// if there is a non-error result, all the function returns must be assigned to variables
x, err := f()
try err, "f failed"

For the error handler, I would personally prefer not supporting the lazy form and having curried functions. The equivalent of the special string form can be written as a curried function:

func Fmtw(msg string) func(error) error {
  return func(err error) error { return fmt.Errorf(msg + ": %w", err }
}

try f(), Fmtw("f failed")

Error handlers then support both the single line without assignment and the 2 line with assignment forms.

gregwebs commented 2 years ago

Okay, thank you all for your feedback on this. I made a new proposal with try as a statement. I think this is the solution that addresses all the community feedback on error handling in a Go-like way and that if you look at the proposal is what the community really already converged on, but it just hasn't been written down coherently with all the pieces together yet.