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

Handle RemoteDisconnected exception from http.client and retry request. #1911

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 31 additions & 1 deletion src/urllib3/connectionpool.py
Expand Up @@ -8,6 +8,12 @@
from socket import error as SocketError
from socket import timeout as SocketTimeout

try:
from http.client import BadStatusLine, RemoteDisconnected
except ImportError:
# Py2
from httplib import BadStatusLine

from .connection import (
BaseSSLError,
BrokenPipeError,
Expand All @@ -30,6 +36,7 @@
ProtocolError,
ProxyError,
ReadTimeoutError,
RemoteDisconnectedError,
SSLError,
TimeoutError,
)
Expand Down Expand Up @@ -354,6 +361,23 @@ def _raise_timeout(self, err, url, timeout_value):
self, url, "Read timed out. (read timeout=%s)" % timeout_value
)

def _check_if_raise_remote_disconnected(self, err):
"""Did remote closed a connection before request?"""

if six.PY2 and isinstance(err, BadStatusLine):
if sys.version_info < (2, 7, 16):
line = ""
else:
line = "No status line received - the server has closed the connection"
if err.args[0] == line:
return True
else:
return False
elif six.PY3 and isinstance(err, RemoteDisconnected):
return True
else:
return False

def _make_request(
self, conn, method, url, timeout=_Default, chunked=False, **httplib_request_kw
):
Expand Down Expand Up @@ -736,6 +760,7 @@ def urlopen(
HTTPException,
SocketError,
ProtocolError,
RemoteDisconnectedError,
BaseSSLError,
SSLError,
CertificateError,
Expand All @@ -748,7 +773,12 @@ def urlopen(
elif isinstance(e, (SocketError, NewConnectionError)) and self.proxy:
e = ProxyError("Cannot connect to proxy.", e)
elif isinstance(e, (SocketError, HTTPException)):
e = ProtocolError("Connection aborted.", e)
if self._check_if_raise_remote_disconnected(e):
e = RemoteDisconnectedError(
"Remote end has closed the connection", e
)
else:
e = ProtocolError("Connection aborted.", e)

retries = retries.increment(
method, url, error=e, _pool=self, _stacktrace=sys.exc_info()[2]
Expand Down
8 changes: 8 additions & 0 deletions src/urllib3/exceptions.py
Expand Up @@ -67,6 +67,14 @@ class ProtocolError(HTTPError):
pass


class RemoteDisconnectedError(HTTPError):
"Raised when the connection to a server fails."

def __init__(self, message, error, *args):
super(RemoteDisconnectedError, self).__init__(message, error, *args)
self.original_error = error


#: Renamed to ProtocolError but aliased for backwards compatibility.
ConnectionError = ProtocolError

Expand Down
3 changes: 3 additions & 0 deletions src/urllib3/util/retry.py
Expand Up @@ -15,6 +15,7 @@
ProtocolError,
ProxyError,
ReadTimeoutError,
RemoteDisconnectedError,
ResponseError,
)
from ..packages import six
Expand Down Expand Up @@ -418,6 +419,8 @@ def _is_connection_error(self, err):
"""
if isinstance(err, ProxyError):
err = err.original_error
if isinstance(err, RemoteDisconnectedError):
err = err.original_error
return isinstance(err, ConnectTimeoutError)

def _is_read_error(self, err):
Expand Down
70 changes: 70 additions & 0 deletions test/with_dummyserver/test_socketlevel.py
Expand Up @@ -411,6 +411,76 @@ def socket_handler(listener):
assert response.status == 200
assert response.data == b"Response 1"

def test_server_closed_connection_right_before_request(self):
# This test is simulating situation, when request is sent
# to server, but in same time server already closed connection.
#
# This also means that request wil never reach destination, because
# already closed connection.
#
# Test scenario:
#
# 1. Request should raise MaxRetryError, because max_retries=0
#
# https://bugs.python.org/issue41345
# https://github.com/psf/requests/issues/4664
# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=899406
# https://bugs.python.org/issue3566
# https://hg.python.org/cpython/rev/eba80326ba53
#
# Note from https://bugs.python.org/issue3566
#
# If the server closed the connection,
# by calling close() or shutdown(SHUT_WR),
# before receiving a short request (<= 1 MB),
# the "http.client" ("httlib" in py2) module raises:
# BadStatusLine exception for py2.
# RemoteDisconnected exception for py3.

fin = Event()

def socket_handler(listener):
sock = listener.accept()[0]

# Read data
buf = b""
while not buf.endswith(b"\r\n\r\n"):
buf = sock.recv(65536)
# Send reply
body = "OK"
sock.send(
(
"HTTP/1.1 200 OK\r\n"
"Content-Type: text/plain\r\n"
"Content-Length: %d\r\n"
"\r\n"
"%s" % (len(body), body)
).encode("utf-8")
)

# Shut down the connection.
# Further sends and receives are disallowed.
sock.shutdown(2) # simulate a server sent a FIN
fin.set() # let the test know it can proceed
sock.close() # close connection

data = "I'm in ur multipart form-data, hazing a cheezburgr"
fields = {
"upload_param": "filefield",
"upload_filename": "lolcat.txt",
"upload_size": len(data),
"filefield": ("lolcat.txt", data),
}

self._start_server(socket_handler)
with HTTPConnectionPool(self.host, self.port) as pool:
response = pool.request("POST", "/upload", fields=fields,
retries=0)
assert response.status == 200
fin.wait()
with pytest.raises(MaxRetryError):
response = pool.request("POST", "/upload", fields=fields, retries=0)

def test_connection_refused(self):
# Does the pool retry if there is no listener on the port?
host, port = get_unreachable_address()
Expand Down