proposal: errors: configurable Handle(error) function

[mvndaai]

Problem

Most of the time in Go if an error occurs it can be returned. There are certain cases when this is not possible, usually go functions.

go func(){
    if err := foo(); err != nil {
        // cannot 'return err' because the function has already returned
    }
}

Packages shouldn't choose how to handle errors so they end up either squashing the error or making configurable error logging like ErrorLog in httputil.ReverseProxy or http.Server.

Proposal

I propose adding a function errors.Handle(error) that can be called in cases where it is impossible to return an error.

There should be a second function errors.SetHandler(func(error)) that can be called, usually in main, that lets an application choose how to handle an error. (i.e. log, increment a metrics, ...)

[ianlancetaylor]

One of the most common ways to handle an error is to return it. Am I correct in thinking that that is not supported here?

It's very common for different functions to want to handle errors differently, but it seems that errors.SetHandler is global, and would only be called once per program. While that might be workable within a single application, I'm skeptical that library code would be able to use errors.Handler.

[mvndaai]

@ianlancetaylor thanks for taking the time to review this!

As a clarification, returning an error does not handle it, it allows the parent function to handle it instead. This would not change that flow at all, it would only change the moments when errors are actually handled.

You are correct that errors.SetHandler would be global. It is not meant to be used by the library function but instead let applications configure it if they want.

The point of this change is that the library code would use error.Handle in cases when an error is handled instead of returned. Currently, code like defer f.Close() just ignores that an error could exist. Having error.Handle would mean that the library packages do not need to be opinionated on how errors are handled.

[mvndaai]

To clarify

library packages do not need to be opinionated on how errors are handled.

There are many ways to handle an error and all are valid depending on the application. Here are some examples:

log.Println(err)

fmt.Println(err)

https://cloud.google.com/error-reporting/docs/setup/go

errorClient.Report(errorreporting.Entry{
    Error: err,
})
log.Print(err)

https://cloud.google.com/logging/docs/setup/go

logger := client.Logger(logName).StandardLogger(logging.Error)
logger.Println(err)

This change means that library packages and all packages just call errors.Handle and don't have to care about preference, but instead leave it up to the application.

[ianlancetaylor]

It seems to me that a library can let an application handle an error by simply returning the error.

[mvndaai]

@ianlancetaylor thank you for taking time on this. This proposal is for instances when it impossible to

simply return the error

Since that was unclear I went back and edited the proposal to hopefully be more clear. Thanks again!

[earthboundkid]

It used to be very common to write programs that work by setting global values, rather than returning things. You could still do this today with:

package globalerr

var Error error

func IsSet() bool {
    return Error != nil
}

And then just set this when you want to in your functions and methods. I don't think this is a good idea, however. This style of programming turns out to compose poorly and be quite brittle. I don't think there's any reason for the language to encourage this approach to errors.

[mvndaai]

@carlmjohnson this isn't about having global errors. This is giving a way for packages to handle ones that are currently getting ignored.

In the function ioutil.ReadFile there is an ignored error in defer f.Close()

func ReadFile(filename string) ([]byte, error) {
	f, err := os.Open(filename)
	if err != nil {
		return nil, err
	}
	defer f.Close()

Although closing does not affect the flow of the program the error shouldn't be swallowed. I should know that errors are happening. This could be a memory leak in my program.

Currently, if ioutil wanted to handle the error and not change the flow of the program it would have to be opinionated.

defer func(){
    if err := f.Close(); err != nil {
        log.Println("file was not closed", err) // Using `log` is being opinionated
    }
}()

Making a generic errors.Handle(err) function means ioutil does not need to be opinionated

defer func(){
    if err := f.Close(); err != nil {
        errors.Handle(err)
    }
}()

The default for errors.Handle could be log.Println(err.Error()), but if my application uses a different way of handling errors errors.SetHandler could be used.

[mvndaai]

In my applications, my error handler function is very complicated and opinionated. It has multiple steps:

1. Determine the severity of an error if I can
2. Log the error in Stackdriver using the determined severity
3. Increment a metric

I would not want ioutils or any other package that I use to determine how to handle errors. If they used log.Println it would be completely lost in my logs and not displayed in my metrics. If there was an errors.Handle function that those packages could use it would not get lost.

[mvndaai]

A much better example is ErrorLog in httputil.ReverseProxy{}

If nil, logging goes to os.Stderr via the log package's standard logger.

The httputil packaged created this function

func (p *ReverseProxy) logf(format string, args ...interface{}) {
	if p.ErrorLog != nil {
		p.ErrorLog.Printf(format, args...)
	} else {
		log.Printf(format, args...)
	}
}

I believe that log shouldn't be a dependency of httputil and calls to that function should be replaced by errors.Handle.

[mvndaai]

Based on the comments the proposal was unclear. I rewrote it to be more to the point. Thank you everyones for taking the time to review.

[rsc]

I think we should decline all the error handling proposals until we are ready to revisit the topic (perhaps not for a few years).

[mvndaai]

@rsc can you categorize this differently than all the other error handling ones. This is NOT a different approach to try, decoration, and return flow. This what to do with an error when it gets to the top of the return tree.

[rsc]

Moving to Go2 to be with all the other error handling proposals.

[mvndaai]

@rsc this isn't a change like try or the boilerplate of if err != nil {. There are no changes needed to the language. The paradigm of decorate and return doesn't change. If you have to label it Go2 to not close it, I guess that is better than closing this, but it isn't really necessary.

This is a fix for a missing feature of Go on how to make a decision on what to do with an error if it cannot be returned (go and defer func returns). It only requires adding 2 functions to the current errors package.

[ianlancetaylor]

I appreciate that you feel strongly about this issue.

I can't think of anything else in the Go standard library that works this way. The idea of a global handler just doesn't seem very Go like to me. This feature is only useful if the standard library itself uses it, but I think that for any specific case in the standard library we would look for some other way to return the error to the application.

What I learn from your example of ioutil.ReadFile is that we should fix that function, not that we should add a global error handling mechanism.

You suggest that shouldn't want to be opinionated, but in fact Go is an opinionated language, by choice. And the opinionated choice is to return errors. httputil.ReverseProxy is indeed an exception here.

[mvndaai]

@ianlancetaylor Thanks for the kind words and taking the time to respond. That being said I disagree that

This feature is only useful if the standard library itself uses it

It would be amazing if the standard library used it, but it would be useful for any imported package.

On the comment of

You suggest that shouldn't want to be opinionated, but in fact Go is an opinionated language, by choice. And the opinionated choice is to return errors

I agree that the Go choice is to return errors and I don't want to change that choice. Since Go has defer and go functions, that isn't always possible. I want Go to be opinionated on how to handle errors, I just don't want every other package I try to import from github to have their own opinions.

I went looking through a few lists of popular go library mostly searching for defer and go funcs to see how errors were handled. This code from the asw-sdk-go code is the best example of why this is needed.

// TODO: What to do with this error? Probably should just log
b, err := json.Marshal(m)
if err != nil {
	continue
}

Here are some other examples of how packages have chosen to handle errors were errors.Handle might be better.

• golang tools has ignored errors
• glog, a logging library, uses fmt.Fprintln(os.Stderr, ...)
• logrus, another logging library, uses fmt.Fprintln(os.Stderr,..) or it's own logging
• Goose has many unhanded errors defer db.Close() and a log.Println
• Authboss has unhandled errors defer resp.Body.Close() and uses their own logger.Errorf
• Gorm uses panic, which might be appropriate to stop flow, but is often more extreme than necessary.

[earthboundkid]

@mvndaai, I think the issue you're pointing to is real, but I'm not sure a global error handler is a good solution. Different errors require different responses—log, ignore, panic (or abort, retry, fail if you remember the days of DOS)—and one handler can't be expected to tell which case is which.

For deferred writes specifically, I and many other have a simple helper function. I think something like it should be considered for the standard library errors package. For problems with goroutines not reporting errors, I think a more systematic solution would be needed—possibly in an entirely new programming language.

[mvndaai]

@carlmjohnson thanks for understanding that this is a real issue.

Thanks for pointing out a way to handle deferred errors. Although that is helpful if people want to change how they are writing code and use named returns, that is still ignoring all go functions and other errors like those from channels.

I agree that different errors deserver different responses. I am not advocating that this be used instead of return error. That would be a horrible idea. I want errors.Handle to be configurable so I could use errors.Is to decide what to do differently if I need something different, only in situations where a returned error is not possible.

The issue is that are currently lots of places where errors cannot be returned. Imported packages, which I cannot control, have channels, go funcs, and defer funcs. Applications, which we can actually control, have others like main or http handler funcs.

The things in my control I can handle how I want. The ones that are not, have no standard way of being handled in go. They are usually a panic from inexperienced developers because that is what all the tutorials teach. All other examples I have seen either log using fmt/log or just ignore them.

[fwhezfwhez]

I'm using this code style to handle error:
./main.go

package main

import (
	"fmt"
	"src"
)

func main() {
	if e := src.AddUser(); e != nil {
		fmt.Println(e.Error())
		return
	}
}

./src/service.go

package src

import (
	"fmt"
	"github.com/fwhezfwhez/errorx"
)

func AddUser() error {
	if e := generateError(); e != nil {
		return errorx.Wrap(e)
	}
	return nil
}
func generateError() error {
	var e = fmt.Errorf("time out")
	return errorx.Wrap(e)
}

output:

2020-03-06 14:31:00 | G:/go_workspace/GOPATH/src/mapx/src/service.go: 16 | time out
2020-03-06 14:31:00 | G:/go_workspace/GOPATH/src/mapx/src/service.go: 10 | time out

[mvndaai]

@fwhezfwhez that package seems helpful, thanks for bringing it to my attention, but it does not solve a main issue of this proposal: What happens when a package you import has an error in a defer or a go func? In both of those cases errorx might not be used, so you would have logs that don't fit the rest of your logs if they package chooses to log it at all.

[fwhezfwhez]

@mvndaai errorx aims to return errors trace-wrapped and handle them in an unified place.It doesn't conflict whether handle errors in defer part or go func() part.

func service() error {
    defer func() {
        if e:= destroyObject(); e!=nil {
            handle(errorx.Wrap(e))
        }
        if e:= x.Close(); e!=nil {
            handle(errorx.Wrap(e))
        }
    }()
    e:= generateError("time out")
    return errorx.Wrap(e)
}

 go func(){
    if e:= service(); e!=nil {
        handle(errorx.Wrap(e))
    }
}()

Apparently in defer part, errors can't return, thus you might handle it straightly.
So does go part.

[mvndaai]

@fwhezfwhez I am confused. I agree that you can call whatever handle function in your own code. The point of this change is that there should be an errors.Handle to call instead, because in a package that you import and they used a go func, they don't have access to your handle function.

[fwhezfwhez]

@fwhezfwhez I am confused. I agree that you can call whatever handle function in your own code. The point of this change is that there should be an errors.Handle to call instead, because in a package that you import and they used a go func, they don't have access to your handle function.

It requires package itself provides xxx.SetErrorHandle(f func(error)) then you can handle error inner a package.
If a package handle error inside and do not return, it means this error is not exported as expected.I think no need to care about an error the author do not want you to handle.

[mvndaai]

@fwhezfwhez I would rather have an errors.SetErrorHandle(func(error)) instead of needing to call a function for every package I import. Then all authors can just call errors.Handle. I want to choose if I care about an error, not defer to the author of the package.

[ianlancetaylor]

Thanks, but this is not a path that Go is going to take. I'm not denying that there is an issue here, but the specific idea of introducing a global handler is not the solution. Closing this issue.