Cause() behavior has changed

[freeekanayaka]

Hello,

after merging #215, I got some test failures of code that calls errors.Cause(err) where err is returned by some stdlib API: in my case err is returned by net.Conn.Write(), and errors.Cause(err) used to return a net.OpError instance, whereas it now returns a syscall.Errno instance.

I understand the reason for the change, but just wanting to highlight this is introduces backward-incompatible behavior, at least for some consumers.

YMMV re fixing this issue. Other people might find it useful to read, if the stumble upon the same regression.

[aperezg]

Could you show a complete example to try to reproduce the behaviour that you say, because the current behaviour in the pkg/errors with method cause is the same unless of the new Wrap way be used.

Furthermore, You could continue using the version 0.8.1 instead of master or the latest version while you migrate to the new version.

[freeekanayaka]

Hey,

if you run this program it will produce different outputs, depending if you use pkg/errors master or v0.8.1:

package main

import (
	"fmt"
	"log"
	"net"

	"github.com/pkg/errors"
)

func main() {
	// Create client/server pair of connections over abstract Unix sockets.
	listener, err := net.Listen("unix", "")
	if err != nil {
		log.Fatalf("net.Listen(): %v", err)
	}
	address := listener.Addr().String()
	client, err := net.Dial("unix", address)
	if err != nil {
		log.Fatalf("net.Dial(): %v", err)
	}
	server, err := listener.Accept()
	if err != nil {
		log.Fatalf("net.Accept(): %v", err)
	}

	// Close the server end and try to perform a write with the client.
	server.Close()
	_, err = client.Write([]byte{1, 2, 3, 4})

	// Wrap the error and then get the cause.
	err = errors.Wrap(err, "connection failure")
	fmt.Printf("%T\n", errors.Cause(err))
}

In master you'll get:

$ go run main.go 
syscall.Errno

while with v0.8.1:

$ go run main.go 
*net.OpError

If people use errors.Wrap against stdlib errors, and then look at the type of errors.Cause() then they might be hit by this change.

[jayschwa]

net.OpError provides an Unwrap method in Go 1.13 that Cause now follows. Maybe #215 should be reverted.

[Bak-Jin-Hyeong]

How about informing users to recommend the use of errors.Is?

Similar backward compatibility issues will occur when user-defined error types implement interface { Unwrap() error } as well as when they implement interface { Cause() error }.

[dcormier]

This is a frustrating problem. There are places where I'd like to be able to take advantage of the added functionality from the Unwrap method being added in v0.9.0, but this change to Cause breaks too many places.

[jayschwa]

@aperezg, I recommend reverting PR #215 and tagging v0.9.1. The hypothetical benefit of #215 is not worth the breakage of real, existing code.

[aperezg]

Perfect so, I will revert this PR and created a new v0.9.1 tag with the old behaviour

[aperezg]

So the new v0.9.1 is tagged with the previous behaviour https://github.com/pkg/errors/releases/tag/v0.9.1

Thanks a lot for all information and help :)

[puellanivis]

Yeah, reverting was the right choice. The errors.Unwrap function does not unwrap an error until it has no unwraps left… it returns a single–iteration of unwrapping.

There is unfortunately, no way to provide Cause that will not unwrap all the way, while also unwrapping go1.13 errors, without also breaking a bunch of stuff that unwraps not just net errors, but almost certainly *os.PathError as well.

[james-johnston-thumbtack]

Given that this was reverted, does it make sense to mark the Cause function as deprecated given that it will not generally work properly with Go 1.13 wrapped errors? The replacement would be to use the new Go 1.13 runtime functions in "errors" package for unwrapping and inspecting.

[puellanivis]

Deprecating Cause is not without merit.