Closed freeekanayaka closed 4 years ago
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.
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.
net.OpError
provides an Unwrap
method in Go 1.13 that Cause
now follows. Maybe #215 should be reverted.
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 }
.
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.
@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.
Perfect so, I will revert this PR and created a new v0.9.1 tag with the old behaviour
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 :)
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.
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.
Deprecating Cause
is not without merit.
Hello,
after merging #215, I got some test failures of code that calls
errors.Cause(err)
whereerr
is returned by some stdlib API: in my caseerr
is returned bynet.Conn.Write()
, anderrors.Cause(err)
used to return anet.OpError
instance, whereas it now returns asyscall.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.