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

Timeout makes the next request to the same domain fail #18

Closed
honux opened this issue Jul 28, 2016 · 13 comments
Closed

Timeout makes the next request to the same domain fail #18

honux opened this issue Jul 28, 2016 · 13 comments

Comments

@honux
Copy link

honux commented Jul 28, 2016

Heya.

Whenever a request fail with a socket.timeout, the following request to the same domain fails. Example:

import httplib2
h = httplib2.Http(timeout=3)
h.force_exception_to_status_code = True

head_1, cont_1 = h.request('https://httpbin.org/delay/60', headers={'cache-control':'no-cache'})
# head_1: {'status': '408', 'content-type': 'text/plain', 'content-length': 15}
# cont_1: b'Request Timeout'

# right after the timeout, request anything to the same domain
head_2, cont_2 = h.request('https://httpbin.org/delay/2', headers={'cache-control':'no-cache'})
# Instead of a 2 second delay, it will reuse the connection made on the previous request, making the response timeout / have the response of the previous request.

I'm using the latest httplib2 and python3


If this is expected, is there any way to really 'drop' the connection? Forcing a new connection instead.

@justinfx
Copy link
Contributor

Out of curiosity, if I am going to handle this case myself, is it sufficient to catch the socket.timeout exception, and then try and pop and close the connection from h.connections?

@temoto
Copy link
Member

temoto commented Aug 13, 2018

@justinfx sounds good. I'd start from a test looking like this:

  def handler(request):
    if request.number == 1:
      time.sleep(0.6)
      return tests.http_response_bytes(status=500)
    return tests.http_response_bytes(status=200)

  http = httplib2.Http(timeout=0.5)
  with tests.server_request(handler, request_count=2) as uri:
    with tests.assert_raises(...):
      http.request(uri)
    response, _ = http.request(uri)
    assert response.status == 200

@justinfx
Copy link
Contributor

@temoto I have a fix and a test locally. The fix works when I manually verify it, but I am having some trouble getting it to replicate the broken behaviour in the test because of the way it seems a new connection is being swapped in already by the test behaviour. I will end up submitting a wip MR for this, but thought in the meantime I would ask to see if you know off the top of your head why the test would be side stepping the standard logic of the same connection being reused on the second request?

@temoto
Copy link
Member

temoto commented Aug 14, 2018

@justinfx I don't see your code.

@thehesiod
Copy link

nice! I was just about to fix this myself ;)

@ClamsTheCat
Copy link

@justinfx commits fixed my issue as well

@temoto
Copy link
Member

temoto commented Nov 15, 2018

Please try latest release v0.12.0 with Justin fix.

@justinfx
Copy link
Contributor

@temoto I didn't see that this was merged. There is no mention of it in the changelog and this issue is still open.
Are you referring to this other thing that came from a suggestion I had made?
211d6f0

@temoto
Copy link
Member

temoto commented Nov 16, 2018

@justinfx yes I confused things up, sorry.

temoto pushed a commit that referenced this issue Nov 19, 2018
#18
#111

Co-Authored-By: justinfx <justinisrael@gmail.com>
@temoto
Copy link
Member

temoto commented Nov 19, 2018

Fix by Justin is merged in master now.

@justinfx
Copy link
Contributor

Thanks!

@max-sixty
Copy link

Can this be closed now?

@temoto
Copy link
Member

temoto commented Oct 7, 2020

@max-sixty thanks

@temoto temoto closed this as completed Oct 7, 2020
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

6 participants