Skip to content

proposal: exp/xerrors: add try and handle #67913

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
anonld opened this issue Jun 10, 2024 · 7 comments
Closed

proposal: exp/xerrors: add try and handle #67913

anonld opened this issue Jun 10, 2024 · 7 comments
Labels
error-handling Language & library change proposals that are about error handling. Proposal

Comments

@anonld
Copy link

anonld commented Jun 10, 2024

Proposal Details

Disclaimer: The following was originally posted by u/Enough_Compote697 on r/golang. For greater reach, and especially for the attention from the Go team, re-post here after the deletion of the original post. Please keep in mind that I am NOT the OP of that Reddit thread, so please don’t tag me in this GitHub issue (unless reposting violates the rules here).

To encourage robust programming by reducing error handling boilerplates, I propose promoting the following functions from a third party library to exp/xerrors and eventually making them built-in:

func handle(h func (error)) {
    r := recover()
    if r == nil {
        return
    }
    if e, ok := r.(error); ok {
        h(e)
        return
    }
    panic(r)
}

func check(e error) {
    if e != nil {
        panic(e)
    }
}

func try[T any](v T, e error) T {
    check(e)
    return v
}

func try2[S, T any](u S, v T, e error) (S, T) {
    check(e)
    return u, v
}

Technically, a built-in variadic try() would eliminate the need of check, try2, try3, etc.

Before commenting/voting on this proposal, please make sure you are familiar with the Reddiquette, and please abide by it the best you can.

Examples


Ex. 1:

func main() {
    defer handle(func(e error) { log.Fatal(e) })
    os.Stdout.Close()
    try(fmt.Println(42))
}

Ex. 2: (see blog/errors-are-values)

func flush(h func(error)) {
    defer handle(h)
    try(fd.Write(p0[a:b]))
    try(fd.Write(p1[c:d]))
    try(fd.Write(p2[e:f]))
}

Note that with the try/handle pair, this version of flush is much simpler and cleaner than the errWriter example in the blog, and much less code is needed.

Ex. 3: (see issue/32437)

func sum(a, b string, h func(error)) int {
    defer handle(h)
    return try(myAtoi(a)) + try(myAtoi(b))
}

Ex. 4: (see play/p/bOUz7qx3urD and the corresponding r/golang post)

Ex. 5: (see design/go2draft-error-handling-overview)

func CopyFile(src, dst string, h func(error)) {
    defer handle(func(err error) { h(fmt.Errorf("copy %s %s: %w", src, dst, err)) }
    r := try(os.Open(src))
    defer r.Close()
    w := try(os.Create(dst))
    defer handle(func(err error) {
        os.Remove(dst)
        panic(err)  // propagate to the handle above for formatting
    })
    defer w.Close() // to close b4 remove, as defer is last-in-fisrt-out
    try(io.Copy(w, r))
}

Ex. 6: (see design/go2draft-error-handling

func SortContents(w io.Writer, files []string, h func(error)) {
    defer handle(func(e error) { h(fmt.Errorf("process: %w", e)) })
    lines := make([][]string, len(files))
    for i, f := range files {
        defer handle(func(e error) { panic(fmt.Errorf("read %s: %w", f, e)) })
        r := try(os.Open(f))
        defer r.Close()
        lines[i] = mySplit(try(io.ReadAll(r)))
    }
    sorted := slices.Sort(slices.Concat(lines))
    for _, line := range sorted {
        try(io.WriteString(w, line))
    }
}

To prevent mishandling deeply nested error that is out of interest, one may limit the error type to a specific kind, and handle the error only after a type assertion, e.g.

defer handle(func(e error) {
    if pe, ok := xerrors.As[fs.PathError](e); ok {
        // error handling code
    } else {
        panic(e) // not interested, pretending not intercepted
    }
})

Ex. 7: (an alternate version of ex. 6, changes marked with ~)

func SortContents(w io.Writer, files []string~) ~error~ {
    defer handle(func(e error) { ~panic~(fmt.Errorf(...) }
    // ...
    return nil
}

Since both return and panic are for propagating error back to the main() function, they are nearly identical semantically. That said, if library authors dislike this kind of error propagation, they may just keep using the current idiomatic if err != nil to handle errors, or adopt the handler propagation style illustrated above.

In addition, even though this version of SortContents panics instead of returning an error, the return argument (error) in the function signature can be kept for documentation purpose by treating the signature as a kind of function annotation, but the cost is to add a corresponding trailing return nil statement. Although the compilation will fail if this final line is omitted, it seems like a gotcha with stinky code smell.

Ex. 8: (another alternative as in regexp and text/template)

func MustSortContents(…) {…}

Note the idiomatic Must prefix implies the function panics in case of error. Although it is up to callers to decide whether to ignore or handle the error in case the program panics, personally I am more inclined to proper error handling rather than turning a blind eye on it, as this proposal is all about encouraging robust programming.

Motivation


When there are too many trivial error handling surrounded by boilerplates, their non-trivial counterparts will just nearly disappear, so this proposal tries to promote the visibility of truly important red flags by reducing boilerplates.

Also, by reducing the burdens on programmers, they are less deterred from handle error robustly. (See the law of effect and operant conditioning for more on the topic.)

Scope


This proposal is only about adding 2 functions (try and handle) to the official repository. Although the introduction of this pair may sparkle debate on error handling style (handler propagation vs error propagation), this proposal is not intended to set a new convention. That is left for the community to discuss and experiment on how to use these 2 functions. If you prefer the idiomatic return err, it is fine, and you can just keep using it. This proposal is not aimed at replacing that idiomatic error handling style but just at reducing programmers’ burden by promoting to try/handle pair from a third party library to the official repository, so please keep in mind that the discussion here should focus on the cost and benefits of such promotion. Thanks for your cooperation.

(Expected) Benefits


  • By reducing the burden of programmers, they are less deterred from robust error handling.
  • By refactoring repeating error handling blocks, programmers are writing code less vulnerable to copy-pasting bug, which can be catastrophic. (Below are examples from Google Project Zero)
    1. Exception-oriented exploitation on iOS
    2. Oops, I missed it again!

(Expected) Cost


Basically, when it comes to programming proposal, it is all about trade-off in the following trilemma:

  • programmers’ time
  • compilation time
  • execution time

The latter two can be addressed with technical improvement. For example, go1.14 reduce the cost of defer with design/34481-opencoded-defers. Although the benchmark then might now become slightly outdated, the tax of try/handle still seems far from being prohibitive.

Regarding programmers’ time, it can be further broken into learning time and coding time. Reducing boilerplates means saving coding time in a recurring manner, so the one-time cost of learning looks like a worthy investment, not to mention the potential robustness enhancement.

Comparisons


  • vs design/go2draft-error-handling: change statements to functions to enhance composablility (see examples above)
  • vs design/32437-try-builtin: change implicit return to implicit panic to reduce surprises, as implicit panic already exists while implicit return is something new and surprising, violating the principle of least astonishment.
  • same: callers still need to write their error handling logic.

FAQ


Q: Why revisit the try built-in proposal?

A: To reduce boilerplates of error handling. It is a pain point of many Go developers according to past surveys.

Q: Why not the idiomatic if err != nil { return } flow but the foreign throw-catch model?

A: When most library routines encounter an error, they immediately abort and return a non-nil error coupled with 1/2 negligible zero value(s), so replacing return with panic means nearly semantically the same for the caller.

Moreover, return err can coexist with the panic-recover model, so library authors can choose the error handling flow that fit their needs the most.

Q: But that violates the Go philosophy. There should be one (and preferably only one) obvious way to do it.

A: That’s true, but there is nothing new. This proposal is not about adding a 3rd way of error handling to the language, but just about utilising the existing language constructs to reduce boilerplates.

Q: How about go try(f(…))?

A: Not clear. Before understanding how goroutines would impact defer/panic/recover, it seems prudent to forbid go try(f(…)) first by

  1. issuing a compilation error when try is called in a go statement, and
  2. adding a vet check for exp/xerrors during the transitional stage.

Library authors can keep using the good old idiomatic if err != nil check with goroutines as is.

Discussion


  1. Examples above simply convert return err to panic. More complex control flow like goto/break/continue may need extra care when switching the error handling flow to the panic-recover model. In case simple refactoring is not enough, perhaps just remaining as-is (if err != nil) suit such scenario the most. Without real world data, it is hard to tell whether the task of refactoring is easy or daunting. If there are real world examples demonstrating non-trivial refactoring, let’s see if we can write tool to automate the work as in the rangefunc experiment. For example, given a function:

    package foo

    //go:generate witherrhandle
    func f(v T) error {
    return g(v)
    }

Running go generate play.ground/foo yields:

  func fV2(v T) (err error) {
      fWithHandler(v, func(e error) { err = e })
      return
  }

  func fWithHandler(v T, h func(error)) {
      defer handle(h)
      try(g(v))
  }
  1. For the sake of compatibility, current stdlib routines would keep panicking or returning errors as is, but the internal may use the new mechanism as long as such changes are invisible to callers. When the timing becomes mature, signatures of newly added functions may form a new idiomatic flow of error handling as shown above.

  2. From a dependency management perspective, promoting handle from a third party library to the official repository, meaning establishing a mechanism for panic interception, may do more good than harm, but the pros and cons are not so clear, so it is left for the community to discuss. For example, if package main imports package foo, and the input sanitizer of foo.Bar panics like the MustXXX family in package regexp and text/template, the main function, especially that of critical server, would be better-off with panic recovery and graceful error handling.

Alternatives

  1. The try function can be written in a variadic fashion with some cost illustrated below:

    func try(vs ...any) []any {
    if e, ok := vs[len(vs)-1].(error); !ok {
    panic("try: last argument is not error")
    } else if e != nil {
    panic(e)
    }
    return vs[:len(vs)-1]
    }

Although this version of try is variadic, it is much less user-friendly. Since it returns []any instead of concrete types, type assertion is needed for devirtualisation. For example, to open a read-only file, one has to write

  f := try(os.Open(name)).(fs.File)

It will be even more clumsy when devirtualising more than one interface{}.

  1. While try is technically most suited to be a built-in function, handle is less constrained and it can be a function in the errors package instead. Although where will it land eventually is out of scope of this proposal, please feel free to discuss here about the pros and cons of adding one more reserved word. In particular, is handle frequently used as a custom variable/function name? If so, it means the built-in handle will be often shadowed, rendering it less useful to be a built-in. On the other hand, a nice side-effect of moving the handle function to the errors package is the possibility to define a visually pleasant errors.Handler interface.

Postscript

Although this proposal is just about including a pair of trivial functions in the official repository, it is surprisingly hilariously disproportionally lengthy, reminding me the Parkinson’s law of triviality. The reason is that much length is used for highlighting the differences between the new and the idiomatic error handling flow so as to address the Fredkin’s paradox, psychological inertia and path dependence. Nonetheless, if clearer explanation can help us reach consensus more easily, it is worth writing more words.

Acknowledgement

Thanks all for you thoughtful feedback, including but not limited to u/FullTimeSadBoi, u/justinisrael and u/drvd. Please accept my apology if I have forgotten to mention your name.

@anonld
Copy link
Author

anonld commented Jun 10, 2024

u/Enough_Compote697 has assigned me as his communication agent, and he has just sent me a supplementary note.

The appHandler in blog/error-handling-and-go can implement the potential errors.Handler() interface, but it seems not quite useful. Anyway, here is the rewritten version:

func init() {
	http.HandleFunc("/view", viewRecord)
}

func viewRecord(w http.ResponseWriter, r *http.Request) {
	h := &appHandler{c: NewContext(r), w: w}
	defer h.Handle()
	key := datastoreNewKey(h.c, "Record", r.FormValue("id"), 0, nil)
	record := Record{}
	h.Try(datastoreGet(h.c, key, record), "Record not found", http.StatusNotFound)
	h.Try(viewTemplateExecute(w, record), "Can't display record", http.StatusInternalServerError)
}

type appHandler struct {
	Message string
	Code    int
	c       Context
	w       http.ResponseWriter
}

func (h *appHandler) Try(e error, m string, c int) {
	if e != nil {
		h.Message, h.Code = m, c
		panic(e)
	}
}

func (h *appHandler) Handle() {
	r := recover()
	if r == nil {
		return
	}
	if e, ok := r.(error); ok {
		h.c.Error(e)
		http.Error(h.w, h.Message, h.Code)
		return
	}
	panic(r)
}

Update: u/Enough_Compote697 argues that Handle should instead be a method of custom error type, and gives the following example against a standard errors.Handler interface:

type appError struct {
    Error   error
    Message string
    Code    int
}

func (e *appError) Handle(c context.Context, w http.ResponseWriter) {
    if e != nil {
	c.Errorf("%w", e.Error)
	http.Error(w, e.Message, e.Code)
    }
}

func viewRecord(w http.ResponseWriter, r *http.Request) {
    c := appengine.NewContext(r)
    defer handle(func(e error) {
	if aerr, ok := e.(*appError); ok {
	    aerr.Handle(c, w)
	} else {
	    panic(e)
	}
    }
    key := datastore.NewKey(c, "Record", r.FormValue("id"), 0, nil)
    record := new(Record)
    try := func(err error, msg string, code int) {
	if err != nil {
	    panic(&appError{err, msg, code})
	}
    }
    try(datastore.Get(c, key, record), "Record not found", http.StatusNotFound)
    try(viewTemplate.Execute(w, record), "Can't display record", http.StatusInternalServerError)
}

func init() {
    http.HandleFunc("/view", viewRecord)
}

Update 2: u/Enough_Compote697 refactored the error handling code to a handling function generator like func appErrorHandler(...) func(*appError). See play/p/qUeVvOJaTLVe for more.

P.S. For other custom try functions that throw a custom error type, perhaps it is better to wrap the underlying error in the custom try function than to write a custom handle function.

@anonld
Copy link
Author

anonld commented Jun 10, 2024

For anybody interested in defining an ErrorHandler interface like

type ErrorHandler interface {
    error
    Handle(l *slog.Logger)
}

Please file another issue. It is out of the scope of this proposal. Thanks for your cooperation.

(In reality, in order to make type assertion like err.(ErrorHandler) possible in the errors.Handle function, functions and types in packages such as fmt and log need to be refactored to new packages like fmtlite and loglite that no longer import os. It is doable, but u/Enough_Compote697 remains opposed to defining such interface. See the revised version of appError example above for more.)

@anonld
Copy link
Author

anonld commented Jun 10, 2024

"Don't forget the cost of education and documentation as exemplified by the introduction of any as an alias of interface{}", u/Enough_Compote697 adds. "Regarding the 2nd point in the discussion session above, if I understand correctly, to avoid deadlock, one has to write go func(){ try(...) } instead, so the question now is whether the location of the defer/recover matters? What is the difference between defer handle(...); go func(){...} and go func(){ defer handle(...); try(...) }?"

u/Enough_Compote697 also kindly reminds prospective commenter to recap wiki/NoPlusOne.

@anonld
Copy link
Author

anonld commented Jun 14, 2024

I suggest changing the signature of handle to func(f inspector[error], h handler[error]).
To make it easier to call handle, I suggest adding InspectorIs(target error) inspector[error] and InspectorAs(target error) inspector[error] to the errors package, where the definitions of inspector and handler are func(T) bool and func(T) respectively.

Example: play/p/WoYEctU1d0P

func fatal(e error) { log.Fatal(e) }
func main() {
	defer handle(errors.InspectorIS(io.EOF), fatal)
	os.Stdout.Close()
	try(fmt.Println(42))
}

Update: the inspectAs implementation in the link above is erroneous. The fixed version is at play/p/yoQnlWES7qk.

The signature and implementation of the handle function is not final, and should be discussed here. For example, the revised version of handle looks like

func handle(f inspector[error], h handler[error]) {
	v := recover()
	if v == nil {
		return
	}
	if e, ok := v.(error); ok && errors.Inspect(e, f) {
		h(e)
		return
	}
	panic(v)
}

In this version of handle, the error inspection logic is decoupled from the error handling code, and I am more inclined to this style of programming. In a hypothetical case, the error handling code may need to traverse the error tree again, repeating the work in error inspection. Nonetheless, I believe decoupling is clearly a win for maintainers, and the optimization work can be left for the compiler. If you have any argument for/against such decoupling, please feel free to comment below. It would be even better if you can support your argument with experience working on real world projects. I will be glad to hear your voice.

@anonld
Copy link
Author

anonld commented Jun 16, 2024

Update: idea retracted (see the comment below)
The latest revision of handle is as follows:

func handle[E error](fmap map[error]inspector[error], fdefault inspector[E]) {
	v := recover()
	if v == nil {
		return
	}
	if e, ok := v.(error); !ok || !inspect(e, func(err error) bool {
		if inspectIs(err)(e) && fmap != nil && fmap[err] != nil {
			return fmap[err](e)
		}
		if err, ok := xinspectAs[E](err); ok && fdefault != nil {
			return fdefault(err)
		}
		return false
	}) {
		panic(v)
	}
}

"In my test, the fmap != nil check can be omitted without compilation nor runtime error, but I am not sure whether it is undefined behavior. That said, that guard is pretty cheap for me (only 15 characters for && fmap != nil), and I would rather keep it there as a defense-in-depth", u/Enough_Compote697 adds.

Note: In this revision, errors.inspect can remain unexported. If you want to expose that function as a public API, please file another proposal for that as it is out of the scope of this proposal. Thank you for your cooperation.

@anonld
Copy link
Author

anonld commented Jul 4, 2024

Comment from u/Enough_Compote697:

I believe we all should be aware of feature creep and keep in mind that this proposal is for the most common scenario of error handling, i.e. propagating the error to the handling function. Therefore, I suggest keeping the scope of this proposal as is. If anyone wants an extra conditional handling function, please write your own custom error handler or file a separate proposal for it. Thanks for your cooperation.

P.S. In my personal experience, when I need to to write error inspection code like errors.Is, my handling function in the if block is basically (1) normal termination (e.g. io.EOF) or (2) loop continuation (e.g. fs.SkipDir). I am happy with the present if err == foo way of error handling in such scenarios and doubt the utility of a conditional error handler. That said, I would like to know (1) real world examples of other kinds of error handling (if there are any) and (2) data showing the percentages of different sorts of error handling. Perhaps it is worth adding an extra function if the most basic error propagation case is less common than expected. For example, I envision that defer errors.Wrap("func: %w") may help people save the work of writing a custom error handler for just wrapping the encountered error. (In the CopyFile example, it would be defer errors.Wrap(fmt.Sprintf("copy %s %s: %%w", src, dst)))

Again, without data supporting such addition, I would like to keep the scope of this proposal as is. That said, I am open to such proposal, so just feel free to comment. Your feedback can help moving the discussion forward. :)

@seankhliao seankhliao added the error-handling Language & library change proposals that are about error handling. label Jul 10, 2024
@seankhliao
Copy link
Member

I don't think we need a mirror of a reddit discussion in the issue tracker.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
error-handling Language & library change proposals that are about error handling. Proposal
Projects
None yet
Development

No branches or pull requests

2 participants