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.