go: errors: Is and As make it impossible to maintain error hygiene

@rogpeppe

Consider:

var MyEOF = fmt.Errorf("my EOF: %w", io.EOF)

This satisfies both errors.Is(MyEOF) and errors.Is(io.EOF) but there’s no way to preserve the former without also preserving the latter.

I struggle to see a situation where you want non-transitive Is: x is MyEOF, MyEOF is io.EOF, but x is not io.EOF.

The similar case with As seems equally confusing: a can be converted to b, b can be converted to c, but a cannot be converted to c?

A concrete example would help here, I think.

I don’t actually believe that it’s necessary to expose the concept of an error chain at all.

We address this in our draft design: “We feel that a single error is too limited a view into the error chain. More than one error might be worth examining.”

Why should an error need to be able to pretend to be something else?

So Is can be used in place of error predicates. For example, to ask if err is temporary, you write errors.Is(err, os.ErrTemporary). This is used in several places in 1.13, notably the net package. For example, see the net.DNSError.Is method.

I think the current design’s reliance on reflect isn’t a good idea, and the need for reflectlite to exist is a good example.

It’s unfortunate that we needed reflectlite, but that’s an implementation detail. You need reflection for As, and we think As is important because it generalizes the common practice of type-asserting errors to the case of error chains.

• you may want to know about all errors in a chain, for logging
• you may only want to know about some errors in a chain, for handling …

The current Is/As functions tie the first two kinds of errors chains together, when conceptually they are different.

The intent of Unwrap/Is/As is to provide error information to programs, for handling (the second bullet). The Error method returns the string for logging. It should include information for the entire list of errors, even those not returned by Unwrap.

The two cases correspond to two Errorf verbs:

• fmt.Errorf("...: %v", err): display err for logging, but not handling
• fmt.Errrof("...: %w", err): display err for logging, and also make it available for handling

I believe this is an implementation of KeepType:

func KeepType(err error, targetType interface{}) error {
    return &keepError{err, reflect.PtrTo(reflect.TypeOf(targetType))}
}

type keepError struct {
    err        error
    targetType reflect.Type
}

func (k *keepError) Error() string {
    return k.err.Error()
}

func (k *keepError) As(target interface{}) bool {
    if reflect.TypeOf(target) != k.targetType {
        return false
    }
    return errors.As(k.err, target)
}

Proof: KeepType should "return true if and only if As(e, new(T)) is true and U is identical to T. "

If direction: if U is identical to T, we skip past the initial if statement, execute As(e, new(T)) (since U is identical to T) and return its value, so if U is identical to T and As(e, new(T)) returns true, so does KeepType.

Only if direction: we need to show that if U is not identical to T or if As(e, new(T)) is false, then KeepType returns false. The initial if handles the first disjunct, and the subsequent call to errors.As handles the second.

Think about io.ErrUnexpectedEOF. You don’t want errgo.Is(err, io.EOF) to return true when err is io.ErrUnexpectedEOF. They’re different kinds of error, even though io.ErrUnexpectedEOF is directly related to io.EOF.

This example confuses me, because errors.Is(io.ErrUnexpectedEOF, io.EOF) does return false. They may be related, but ErrUnexpectedEOF is not EOF.

It’s unfortunate that we needed reflectlite, but that’s an implementation detail. You need reflection for As, and we think As is important because it generalizes the common practice of type-asserting errors to the case of error chains.

Go already has a type assertion statement. The fact that such a low level package as errors needs to resort to interface{} and reflect-based magic is surely not ideal. Beginners will need to be exposed to this from a very early stage, and it’s really not easy to explain.

One thing that was concerning to me about the current proposal is the amount of mechanism involved.

Just want to add a big +1 to this point in particular. I think the current design’s reliance on reflect isn’t a good idea, and the need for reflectlite to exist is a good example.

That’s true, but I think that only reinforces my point: it is up to the author of the error (or error type) — not the user of the error — to decide how much information that type or value is capable of masking.

This seems odd to me. Why should the power of hiding errors be up to the creator of the error rather than the code that wants to hide it?

There is an alternative to walking the chain. Each time we wrap an error, instead of recording a link in the chain, we instead decide whether to keep or drop the underlying error value. I don’t actually believe that it’s necessary to expose the concept of an error chain at all.

Now that the printing and source location concerns have been removed from the errors package, it has become clear to me that all we really need here is a way of distinguishing error metadata from the actual error itself. By metadata, I mean extra information such as message annotation and source location info.

One thing that was concerning to me about the current proposal is the amount of mechanism involved. Not only do we have an Unwrap type, we also have other interface conventions, such as interface{Is(error)bool} and interface{As(interface{}) bool}, and the As implementation requires the non-trivial reflectlite package as a dependency.

To me, this feels a bit wrong. Error handling is one of the lowest levels of the language. I would like an errors package that feels irreducibly simple, and the current proposal doesn’t currently make me feel that way.

How about something like this instead, as the entire implementation of the errors package?

package errors

// New returns an error that formats as the given text.
//
// The returned error's E-value is itself.
func New(s string) error {
	return &errorString{s}
}

// errorString is a trivial implementation of error.
type errorString struct {
	s string
}

func (e *errorString) Error() string {
	return e.s
}

// Error may be implemented by an error value to signify that the error
// value is adding metadata to some underlying error (the "E-value").
type Error interface {
	error

	// E returns the underlying error, or E-value. If it returns
	// nil, the receiver itself should be treated as the
	// E-value.
	E() error
}

// E returns the "E-value" of an error - the part of the error that
// should be used for error diagnosis.
//
// When writing code that makes a decision based on an error, the E
// value should always be used in preference to the error value itself,
// because that allows functions to add metadata to the error, such as
// extra message annotations or source location information, without
// obscuring the actual error.
func E(err error) error {
	for {
		err1, ok := err.(Error)
		if !ok {
			return err
		}
		e := err1.E()
		if e == nil {
			return err
		}
		err = e
	}
}

We don’t need the Is or As methods any more, because they’re trivially implemented in normal Go:

 if errors.E(err) == io.EOF { .... }

 if err, ok := errors.E(err).(*os.PathError); ok { ... }

Although the E method looks superficially similar to Unwrap, it’s not the same, because error wrappers don’t need to preserve the error chain - they can just keep the most recent E-value of the error that’s being wrapped. This means that error inspection is O(1). The reason for the loop inside the E function is to keep error implementations honest and to avoid confusion: errors.E(err) will always be the same as errors.E(errors.E(err)).

Once we have this, error annotation and other error metadata can be left to other packages. The %v and %w verbs of the fmt package can retain their current semantics.

Here’s a simple error-wrapping example:

// Notef returns err prefixed by the given formatted message.
// The returned error's E-value is left unchanged.
// If err is nil, Notef returns nil.
func Notef(err error, msg string, args ...interface{}) error {
	if err == nil {
		return nil
	}
	if m := fmt.Sprintf(msg, args...); m != "" {
		return noteError{
			msg: m,
			err: errors.E(err),
		}
	}
	return err
}

type noteError struct {
	msg string
	err error
}

func (err noteError) Error() string {
	return err.msg + ": " + err.err.Error()
}

func (err noteError) E() error {
	return err.err
}