errors: inconsistency between os.IsNotExist and errors.Is with wrapped error

[orlangure]

What version of Go are you using (go version)?

$ go version
go version go1.14.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/yury/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"

What did you do?

Playground

package main

import (
	"errors"
	"fmt"
	"io/ioutil"
	"os"
)

func main() {
	_, err := ioutil.ReadFile("foobar")
	fmt.Println("original error:", err)

	fmt.Println("os.IsNotExist", os.IsNotExist(err))
	fmt.Println("errors.Is", errors.Is(err, os.ErrNotExist))
	
	fmt.Println("wrapping with Errorf")
	
	err = fmt.Errorf("can't read file: %w", err)

	fmt.Println("os.IsNotExist", os.IsNotExist(err))
	fmt.Println("errors.Is", errors.Is(err, os.ErrNotExist))
}

// Output:
// original error: open foobar: No such file or directory
// os.IsNotExist true
// errors.Is true
// wrapping with Errorf
// os.IsNotExist false
// errors.Is true

What did you expect to see?

Consistent reporting in both errors.Is and os.IsNotExist with both wrapped and not wrapped no such file or directory error.

What did you see instead?

The original error returned by ioutil.ReadFile is recognized as ErrNotExist by both errors.Is and os.IsNotExist.
Error wrapped with fmt.Errorf("text: %w", err) is no longer recognized by os.IsNotExist, but still recognized by errors.Is.

[andybons]

@neild @jba

[neild]

For better or worse, we chose not to change the definition of os.IsNotExist and other preexisting predicate functions to unwrap errors. This is intentional behavior.

[orlangure]

Thank you. Does it mean the we should always use errors.Is, and consider os.IsNotExist deprecated?

[neild]

You should use errors.Is if you want the check to unwrap errors.

[harshavardhana]

Would it be helpful to update the os.IsNotExist documentation to ensure that users end up using errors.Is() instead of os.IsNotExist when applicable?

[ianlancetaylor]

@harshavardhana That has been done for the future 1.16 release for #41122 in https://golang.org/cl/268897.

There is nothing to do here, so closing this issue. Please comment if you disagree.

[harshavardhana]

There is nothing to do here, so closing this issue. Please comment if you disagree.

This looks good @ianlancetaylor - thank you