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

4 x connectandsend() retries in call to HTTP.request(..., retries = 0) #115

Closed
samoconnor opened this issue Nov 15, 2017 · 2 comments
Closed

Comments

@samoconnor
Copy link
Contributor

conn = @retryif ClosedError 4 connectandsend(client, sch, host, ifelse(p == "", "80", p), req, opts, verbose)

Should be @retryif ClosedError retry connectandsend(... ?

Also, there are other places in client.jl where network connect, read, write etc is retried despite retries = 0:

ip = @retry Base.getaddrinfo(hostname)

tcp = @retryif Base.UVError @timeout(opts.connecttimeout::Float64,

@retryif Base.UVError write(conn.socket, reqstr)

buffer = @retry @timeout(tm, readavailable(socket), error("read timeout"))

@samoconnor
Copy link
Contributor Author

@quinnj, I started on a PR for this, but the problem is that there are nested retries.
The doc says: "retries::Int: # of times a request will be tried before throwing an error; default = 3".
If I just use the retry::Int arg of request() for the @retryif ClosedError retry parameter we might end up with more than the requested number of retries. i.e. there could be 3 connectandsend() attempts and another 3 attempts at whatever comes after that that is already governed by retry::Int.

Perhaps it would be cleaner to take out all the low-level retries and have a single top-level retry loop governed by retry::Int.

@quinnj
Copy link
Member

quinnj commented Nov 26, 2017

I'm not sure I really see the issue; I don't see a problem w/ hard-coding a retry for Base.getaddrinfo; is it really terrible if that gets auto-retried? What's the use-case for not allowing that retry? I imagine your use-case comes more from wanting complete control over all retrying, but in this case, I feel like it's just a no-brainer and can't hurt to just do the retry in HTTP.jl itself. I'd like to hear more specific issues/use-cases around having this kind of super granular retry control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants