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.

Sign up for GitHub

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

Jump to bottom

net/url: url.Error should propagate the net.Error interface #12866

Closed
jum opened this issue Oct 7, 2015 · 10 comments
Closed

net/url: url.Error should propagate the net.Error interface #12866

jum opened this issue Oct 7, 2015 · 10 comments
Labels
FrozenDueToAge
Milestone
Go1.6

Comments

@jum
Copy link

jum commented Oct 7, 2015

At some points in a program it might interesting to find out if a network error is transient or a timeout. If the program uses Dial et. al. this is handled by calling the Temporary() or Timeout() interfaces on net.Error. But if the program uses http.Get or similar, the net.Error is hidden behind url.Error.Err. one has to use a logic like this check for transient errors:

            testErr := err
            if e, ok := testErr.(*url.Error); ok {
                testErr = e.Err
            }
            // As we are called regularly by cron, ignore some errors.
            type timeout interface {
                Timeout() bool
            }
            if e, ok := testErr.(timeout); ok {
                if e.Timeout() {
                    err = nil
                }
            }
            type temporary interface {
                Temporary() bool
            }
            if e, ok := testErr.(temporary); ok {
                if e.Temporary() {
                    err = nil
                }
            }

If url.Error would embed Err anonymously, any interfaces would be available without the type assertion to *url.Error.
@ianlancetaylor ianlancetaylor changed the title url.Error should propagate the net.Error interface net/url: url.Error should propagate the net.Error interface Oct 7, 2015
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Oct 7, 2015
@rakyll
Copy link
Contributor

rakyll commented Oct 7, 2015

/cc @bradfitz

@bradfitz
Copy link
Contributor

bradfitz commented Oct 8, 2015

Sure. If somebody wants to send a CL to make url.Error implement net.Error, that's fine with me at least.

@davecheney davecheney self-assigned this Oct 8, 2015
@davecheney davecheney modified the milestones: Go1.6, Unplanned Oct 8, 2015
@davecheney
Copy link
Contributor

I'll take a shot

@davecheney
Copy link
Contributor

@jum I started to look into this, and I cannot find a place where url.Error.Err is assigned something that conforms to net.Error. I think there is nothing to do on this ticket because url.Error never wraps a net.Error.

Can you please provide some sample code that shows a url.Error holding a net.Error.

Thanks

Dave

@davecheney
Copy link
Contributor

@jum sorry, I found it. There is one place in http/client.go, https://github.com/golang/go/blob/master/src/net/http/client.go#L417, where url.Error is used to record the url that failed in a redirect chain.

@davecheney
Copy link
Contributor

https://go-review.googlesource.com/15672

@gopherbot
Copy link

CL https://golang.org/cl/15672 mentions this issue.

@arvenil arvenil mentioned this issue Feb 23, 2016
Open
@arvenil
Copy link

arvenil commented Feb 23, 2016

I just wasted way more than 15 minutes trying to figure out why some network error was not catch by err.(net.Error) even though docs stated it should. Turned out url.Error implements net.Error since go 1.6.
I wonder if anyone thought/propose adding go version numbers to docs saying from which version given method is available?

@bradfitz
Copy link
Contributor

@arvenil, there is a separate bug open for that. I don't know its number off hand, though.

@arvenil
Copy link

arvenil commented Feb 23, 2016

@bradfitz I didn't found it before posting, but I've tried again now and I think it is this one #5778 ? Thank you!

@golang golang locked and limited conversation to collaborators Feb 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.6
Development

No branches or pull requests
7 participants
@bradfitz @davecheney @jum @rakyll @arvenil @ianlancetaylor @gopherbot