Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc/go1.15: add note about crypto/tls error wrapping change #40554

Closed
rolandshoemaker opened this issue Aug 3, 2020 · 4 comments
Closed

doc/go1.15: add note about crypto/tls error wrapping change #40554

rolandshoemaker opened this issue Aug 3, 2020 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@rolandshoemaker
Copy link
Member

After https://go-review.googlesource.com/c/go/+/227840 http.Client and tls.Client will now return an opaque tls.permanentError in place of what were previously public error types (i.e. net.OpErr). This means that in order to get the previously top level error users need to use errors.As (or errors.Unwrap) to get the useful error (best practice should presumably be to always use errors.As but directly casting to net.OpErr is definitely quite common).

cc @FiloSottile @katiehockman

@ALTree ALTree added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 4, 2020
@ALTree ALTree added this to the Go1.15 milestone Aug 4, 2020
@katiehockman katiehockman added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Aug 10, 2020
@katiehockman katiehockman self-assigned this Aug 10, 2020
@katiehockman
Copy link
Contributor

Yeah that sounds good. I'll take care of that. Thanks @rolandshoemaker.

@gopherbot
Copy link

Change https://golang.org/cl/247698 mentions this issue: docs/go1.15: document crypto/tls permanentError

@UladBohdan
Copy link

Hi @rolandshoemaker @katiehockman

I've noticed the original description of this issue refers to direct casting of errors to net.OpError type, I can see how it was affected and I can see how error check was modified in tests to handle new behaviour https://go-review.googlesource.com/c/go/+/227840/4/src/crypto/tls/handshake_server_test.go

However, I can't really see why the final description of the change https://go-review.googlesource.com/c/go/+/247698/4/doc/go1.15.html refers to assertion of net.Error type. From what I see, net.Error interface type was not really affected: new permanentError type also implements net.Error, therefore casting will not be broken https://go-review.googlesource.com/c/go/+/227840/4/src/crypto/tls/conn.go

Could you please explain this to me? Does go1.15 really affect net.Error but not other public error types from net package i.e. net.OpError?

@FiloSottile
Copy link
Contributor

Yeah the release note should probably say "the original error" rather than "the original net.Error". I'll send a CL later if no one beats me to it.

reneighbor pushed a commit to cloudfoundry/gorouter that referenced this issue Apr 14, 2021
* Do to golang 1.15 change, errors are now wrapped in a private
permanentError
* See: golang/go#40554

[#177498036](https://www.pivotaltracker.com/story/show/177498036)
@golang golang locked and limited conversation to collaborators Feb 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

7 participants