You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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.
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"
)
funcmain() {
// Create client/server pair of connections over abstract Unix sockets.listener, err:=net.Listen("unix", "")
iferr!=nil {
log.Fatalf("net.Listen(): %v", err)
}
address:=listener.Addr().String()
client, err:=net.Dial("unix", address)
iferr!=nil {
log.Fatalf("net.Dial(): %v", err)
}
server, err:=listener.Accept()
iferr!=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.
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.
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.
Activity
aperezg commentedon Jan 12, 2020
Could you show a complete example to try to reproduce the behaviour that you say, because the current behaviour in the
pkg/errorswith methodcauseis the same unless of the newWrapway 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 commentedon Jan 12, 2020
Hey,
if you run this program it will produce different outputs, depending if you use
pkg/errorsmaster or v0.8.1:In master you'll get:
while with v0.8.1:
If people use
errors.Wrapagainst stdlib errors, and then look at the type oferrors.Cause()then they might be hit by this change.jayschwa commentedon Jan 13, 2020
net.OpErrorprovides anUnwrapmethod in Go 1.13 thatCausenow follows. Maybe #215 should be reverted.Bak-Jin-Hyeong commentedon Jan 13, 2020
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 implementinterface { Cause() error }.dcormier commentedon Jan 14, 2020
This is a frustrating problem. There are places where I'd like to be able to take advantage of the added functionality from the
Unwrapmethod being added inv0.9.0, but this change toCausebreaks too many places.jayschwa commentedon Jan 14, 2020
@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 commentedon Jan 14, 2020
Perfect so, I will revert this PR and created a new v0.9.1 tag with the old behaviour
aperezg commentedon Jan 14, 2020
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 commentedon Jan 15, 2020
Yeah, reverting was the right choice. The
errors.Unwrapfunction 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
Causethat will not unwrap all the way, while also unwrappinggo1.13errors, without also breaking a bunch of stuff that unwraps not just net errors, but almost certainly*os.PathErroras well.james-johnston-thumbtack commentedon Mar 3, 2020
Given that this was reverted, does it make sense to mark the
Causefunction 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 commentedon Mar 3, 2020
Deprecating
Causeis not without merit.