Thanks for writing up this list! (And sorry for taking so long to respond.)

My current plan is to add os.ErrTimeout and make every value which returns true for Timeout() also return true for errors.Is(err, os.ErrTimeout). (And the same for Temporary.)

Taking each point in turn, my personal opinion:

For fmt.Errorf the only question is whether to turn a %s/%v to a %w.

Existing fmt.Errorf calls didn't wrap, so leaving them as-is is always correct.

• should they have an Unwrap method?

Wrapper error types should have Unwrap methods. This means, in particular, all types which forward IsTimeout and IsTemporary to a wrapped type should also have an Unwrap method.

• should they collect stack frames?

In general, yes, although there might be exceptions where location information adds no useful context and can be omitted.

• should they have a FormatError method?

In general, yes.

• Do any require an Is method?

Types with an IsTimeout or IsTemporary method that returns true should have an Is method declaring them as equivalent to os.ErrTimeout/os.ErrTemporary.

• An As Method?

Not in any cases that I've seen yet.

If one of the above obviates an older approach (like an Err error field on a struct) should the older approach be marked deprecated?

I'm ambivalent on whether we should mark the older approach as deprecated or not.

I wrote a program to analyze the 1.12 release's use of errors. It inspects each .go file, regardless of build tags, skipping testdata and cmd directories. It collects custom error types, use of Errorf that may wrap an error, direct calls to an Error method.

The report and program are in this gist: https://gist.github.com/jimmyfrasche/51b83022f83ccbe240092dcd884605cd

While I plan to, I have yet to explore the larger packages: crypto/{tls, x509}, encoding/{json, xml}, or anything under go, internal/x, net, os, or syscall

I went through everything else in the stdlib, though. I'm sure a lot of this on the radar and I'm even surer that I've missed things as this was a quick and superficial exploration, but another pair of eyes never hurts. Here's what I've found so far:

Types with an Err field that could use an Unwrap method:

• compress/flate.{Read,Write}Error
• encoding/csv.ParseError
• strconv.NumError
• text/template.ExecError

Types with Temporary()/Timeout() methods:

• context.deadlineExceededError
• internal/poll.TimeoutError (assuming that ever makes it way outside of internal/)

For errors that get wrapped without a way to get to the underlying error (mostly via fmt.Printf, but there is one exception below), most seem benign but there are a number that wrap IO errors. Arguably that's correct, but I note them regardless.

In image/png an IO error is wrapped in a FormatError but that type's represented as a string so I suspect there's little to be done about that.

Packages that wrap IO errors by calls to fmt.Errorf:

• internal/trace
• debug/pe
• image/gif
• mime/multipart

There's one other case where I found errors wrapped with fmt.Errorf: database/sql conversions. These wrap errors provided by the database driver. It might be a good idea to change these to %w. For example, postgres has a rich type system and data type conversion errors can contain user defined messages so being able to get to a driver defined error type could be a win.

Change https://golang.org/cl/163058 mentions this issue: os: make errors.Is work with ErrPermission et al.

Change https://golang.org/cl/170037 mentions this issue: all: add Unwrap and Is methods to various error types

Change https://golang.org/cl/194563 mentions this issue: strconv: add Unwrap to custom error types

Change https://golang.org/cl/194599 mentions this issue: cmd/go/internal/modfetch/codehost: add Unwrap to custom error types

Change https://golang.org/cl/194817 mentions this issue: cmd/go/internal/modload: add an Unwrap method on ImportMissingError

x509.CertificateInvalidError doesn't appear to have an Unwrap() method either, nor do the other Error types in the x509/verify.go file.

x509.CertificateInvalidError doesn't wrap an error:

type CertificateInvalidError struct {
        Cert   *Certificate
        Reason InvalidReason
        Detail string
}

InvalidReason is an enum of error causes. It would be possible to make InvalidReason implement error and make CertificateInvalidError unwrap to it, permitting:

if errors.Is(err, x509.Expired) { ... }

as an alternative to:

if e, ok := err.(*x509.CertificateInvalidError); ok && e.Reason == x509.Expired { ... }

That actually looks rather nice.

Change https://golang.org/cl/217132 mentions this issue: doc/go1.14: mention new method strconv.NumError.Unwrap

Change https://golang.org/cl/262343 mentions this issue: crypto/x509: add Unwrap to SystemRootsError