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

Retry doesn't retry upon ConnectionResetError #2751

Open
vitaly-krugl opened this issue Oct 17, 2022 · 5 comments
Open

Retry doesn't retry upon ConnectionResetError #2751

vitaly-krugl opened this issue Oct 17, 2022 · 5 comments

Comments

@vitaly-krugl
Copy link

vitaly-krugl commented Oct 17, 2022

Subject

My application uses the urllib3 Retry class. When the socket connection fails with ConnectionResetError during ingestion of response, the Retry doesn't retry it.

Environment

Python 3.9 on OS X

In [1]: import platform
   ...: import urllib3
   ...: 
   ...: print("OS", platform.platform())
   ...: print("Python", platform.python_version())
   ...: print("urllib3", urllib3.__version__)
   ...: 
OS macOS-10.16-x86_64-i386-64bit
Python 3.9.10
urllib3 1.26.12

Steps to Reproduce

My app uses urllib3 via the requests 2.12.0 package. It appears that the connection reset occurs while requests is ingesting the response from the urllib3 response stream. Session and Retry are configured as follows:

import urllib3.util.retry as urllib3_retry

# Methods which are considered idempotent
DEFAULT_RETRIABLE_METHODS = frozenset(
    {'PUT', 'GET', 'OPTIONS', 'HEAD', 'DELETE', 'TRACE'})

DEFAULT_RETRIABLE_STATUSES = frozenset(
    {408, 429, 500, 501, 502, 503, 504, 506, 507, 509, 598})

DEFAULT_MAX_TOTAL_RETRIES = 5

DEFAULT_MAX_CONNECTION_RETRIES = 5

DEFAULT_MAX_READ_RETRIES = 5

DEFAULT_BACKOFF_FACTOR = 1

DEFAULT_CONNECTION_POOL_SIZE = 10

DEFAULT_TLS_PROTOCOL_VERSION = ssl.PROTOCOL_TLSv1_2


def create_default_retry():
    return urllib3_retry.Retry(
        total=DEFAULT_MAX_TOTAL_RETRIES,
        connect=DEFAULT_MAX_CONNECTION_RETRIES,
        read=DEFAULT_MAX_READ_RETRIES,
        other=5,
        redirect=0,
        method_whitelist=DEFAULT_RETRIABLE_METHODS,
        status_forcelist=DEFAULT_RETRIABLE_STATUSES,
        backoff_factor=DEFAULT_BACKOFF_FACTOR,
        raise_on_redirect=False,
        raise_on_status=False)

def create_default_ssl_adapter():
    adapter = requests.adapters.HTTPAdapter(max_retries=create_default_retry())

    adapter.init_poolmanager(connections=DEFAULT_CONNECTION_POOL_SIZE,
                             maxsize=DEFAULT_CONNECTION_POOL_SIZE,
                             block=False,
                             ssl_version=DEFAULT_TLS_PROTOCOL_VERSION)
    return adapter

class RetriableSslSession(requests.Session):
    def __init__(self):
        super(RetriableSslSession, self).__init__()

        self.mount('https://', create_default_ssl_adapter())

Expected Behavior

I expected the request to be retried automatically. Failure to do retries upon connection reset would appear to be a serious flaw in the Retry stack.

Actual Behavior

Request is not retried and fails with the following exception:

ERROR:mdt_inspect.utils.metrico.client:Failure processing <GET https://akab-5h5sgjdaayaccksm-nvigudyq54gcf3kl.luna.akamaiapis.net/metrico-services/v1/arl-basic-info headers={'Accept': 'application/json'} query={'fileIds': '268394'} timeouts=(5, 20) data='{"fileIds": ["268394"]}'>: MetricoMalformedResponseError('Malformed response: ChunkedEncodingError(ProtocolError("Connection broken: ConnectionResetError(54, \'Connection reset by peer\')", ConnectionResetError(54, \'Connection reset by peer\')))')
Traceback (most recent call last):
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 443, in _error_catcher
    yield
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 566, in read
    data = self._fp_read(amt) if not fp_closed else b""
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 532, in _fp_read
    return self._fp.read(amt) if amt is not None else self._fp.read()
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 463, in read
    n = self.readinto(b)
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 507, in readinto
    n = self.fp.readinto(b)
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/socket.py", line 704, in readinto
    return self._sock.recv_into(b)
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ssl.py", line 1241, in recv_into
    return self.read(nbytes, buffer)
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ssl.py", line 1099, in read
    return self._sslobj.read(len, buffer)
ConnectionResetError: [Errno 54] Connection reset by peer

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/requests/models.py", line 816, in generate
    yield from self.raw.stream(chunk_size, decode_content=True)
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 627, in stream
    data = self.read(amt=amt, decode_content=decode_content)
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 592, in read
    raise IncompleteRead(self._fp_bytes_read, self.length_remaining)
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/contextlib.py", line 137, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 460, in _error_catcher
    raise ProtocolError("Connection broken: %r" % e, e)
urllib3.exceptions.ProtocolError: ("Connection broken: ConnectionResetError(54, 'Connection reset by peer')", ConnectionResetError(54, 'Connection reset by peer'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/vkruglik/git/metanalysis-project/metadata-analysis-sandbox/bin/mdt-inspect/src/mdt_inspect/utils/metrico/client.py", line 371, in _dispatch_https_request
    response = self._SESSION.request(method=method,
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/requests/sessions.py", line 587, in request
    resp = self.send(prep, **send_kwargs)
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/requests/sessions.py", line 745, in send
    r.content
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/requests/models.py", line 899, in content
    self._content = b"".join(self.iter_content(CONTENT_CHUNK_SIZE)) or b""
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/requests/models.py", line 818, in generate
    raise ChunkedEncodingError(e)
requests.exceptions.ChunkedEncodingError: ("Connection broken: ConnectionResetError(54, 'Connection reset by peer')", ConnectionResetError(54, 'Connection reset by peer'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/vkruglik/git/metanalysis-project/metadata-analysis-sandbox/bin/mdt-inspect/src/mdt_inspect/utils/metrico/client.py", line 392, in _dispatch_https_request
    raise MetricoMalformedResponseError(
mdt_inspect.utils.metrico.client.MetricoMalformedResponseError: Malformed response: ChunkedEncodingError(ProtocolError("Connection broken: ConnectionResetError(54, 'Connection reset by peer')", ConnectionResetError(54, 'Connection reset by peer')))
Traceback (most recent call last):
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 443, in _error_catcher
    yield
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 566, in read
    data = self._fp_read(amt) if not fp_closed else b""
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 532, in _fp_read
    return self._fp.read(amt) if amt is not None else self._fp.read()
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 463, in read
    n = self.readinto(b)
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 507, in readinto
    n = self.fp.readinto(b)
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/socket.py", line 704, in readinto
    return self._sock.recv_into(b)
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ssl.py", line 1241, in recv_into
    return self.read(nbytes, buffer)
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ssl.py", line 1099, in read
    return self._sslobj.read(len, buffer)
ConnectionResetError: [Errno 54] Connection reset by peer

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/requests/models.py", line 816, in generate
    yield from self.raw.stream(chunk_size, decode_content=True)
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 627, in stream
    data = self.read(amt=amt, decode_content=decode_content)
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 592, in read
    raise IncompleteRead(self._fp_bytes_read, self.length_remaining)
  File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/contextlib.py", line 137, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/urllib3/response.py", line 460, in _error_catcher
    raise ProtocolError("Connection broken: %r" % e, e)
urllib3.exceptions.ProtocolError: ("Connection broken: ConnectionResetError(54, 'Connection reset by peer')", ConnectionResetError(54, 'Connection reset by peer'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/vkruglik/git/metanalysis-project/metadata-analysis-sandbox/bin/mdt-inspect/src/mdt_inspect/utils/metrico/client.py", line 371, in _dispatch_https_request
    response = self._SESSION.request(method=method,
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/requests/sessions.py", line 587, in request
    resp = self.send(prep, **send_kwargs)
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/requests/sessions.py", line 745, in send
    r.content
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/requests/models.py", line 899, in content
    self._content = b"".join(self.iter_content(CONTENT_CHUNK_SIZE)) or b""
  File "/Users/vkruglik/git/metanalysis-project/venv/lib/python3.9/site-packages/requests/models.py", line 818, in generate
    raise ChunkedEncodingError(e)
requests.exceptions.ChunkedEncodingError: ("Connection broken: ConnectionResetError(54, 'Connection reset by peer')", ConnectionResetError(54, 'Connection reset by peer'))

@vitaly-krugl vitaly-krugl changed the title Retry doesn't retry ConnectionResetError Retry doesn't retry upon ConnectionResetError Oct 18, 2022
@gdubicki
Copy link
Sponsor

I found this issue while I was troubleshooting another case of non-retries requests done by requests+urllib3. But ultimately in my case (gitlabform/gitlabform#492) the lack of retry was justified (see my issue for more info).

Was about your case, @vitaly-krugl, did you learn anything more that you can share?

@vitaly-krugl
Copy link
Author

@gdubicki - thanks for sharing your issue. And no, I have not learned anything more.

@dagardner-nv
Copy link

I ran into a similar issue, using urllib3 by itself. In my experience if the remote server is down prior to making the request, then it retries as expected.

However if instead the server is up prior to starting, but goes down unexpectedly in the middle of the request, then the request immediately raises a urllib3.exceptions.ProtocolError without any retries.

Repo script:

import logging
import sys

import urllib3
from urllib3.util import Retry

print(sys.version)
print(urllib3.__version__)
#logging.basicConfig(level=logging.DEBUG)

body = '[{"a":1}]'

retries = Retry(total=10,
                backoff_factor=0.1,
                respect_retry_after_header=True,
                status_forcelist=[429, 500, 502, 503, 504])
http = urllib3.PoolManager(retries=retries)

while True:
    resp = http.request("POST",
                        "http://127.0.0.1:8080/test",
                        headers={"Content-Type": "application/json"},
                        timeout=30,
                        body=body)

Output:

3.11.3 | packaged by conda-forge | (main, Apr  6 2023, 08:57:19) [GCC 11.3.0]
2.0.2
Traceback (most recent call last):
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/connectionpool.py", line 790, in urlopen
    response = self._make_request(
               ^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/connectionpool.py", line 536, in _make_request
    response = conn.getresponse()
               ^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/connection.py", line 454, in getresponse
    httplib_response = super().getresponse()
                       ^^^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/http/client.py", line 1375, in getresponse
    response.begin()
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/http/client.py", line 318, in begin
    version, status, reason = self._read_status()
                              ^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/http/client.py", line 287, in _read_status
    raise RemoteDisconnected("Remote end closed connection without"
http.client.RemoteDisconnected: Remote end closed connection without response

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/dagardner/work/morpheus/.tmp/urllib3_repro.py", line 20, in <module>
    resp = http.request("POST",
           ^^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/_request_methods.py", line 118, in request
    return self.request_encode_body(
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/_request_methods.py", line 217, in request_encode_body
    return self.urlopen(method, url, **extra_kw)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/poolmanager.py", line 433, in urlopen
    response = conn.urlopen(method, u.request_uri, **kw)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/connectionpool.py", line 844, in urlopen
    retries = retries.increment(
              ^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/util/retry.py", line 470, in increment
    raise reraise(type(error), error, _stacktrace)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/util/util.py", line 38, in reraise
    raise value.with_traceback(tb)
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/connectionpool.py", line 790, in urlopen
    response = self._make_request(
               ^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/connectionpool.py", line 536, in _make_request
    response = conn.getresponse()
               ^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/site-packages/urllib3/connection.py", line 454, in getresponse
    httplib_response = super().getresponse()
                       ^^^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/http/client.py", line 1375, in getresponse
    response.begin()
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/http/client.py", line 318, in begin
    version, status, reason = self._read_status()
                              ^^^^^^^^^^^^^^^^^^^
  File "/home/dagardner/work/conda/envs/reqtest/lib/python3.11/http/client.py", line 287, in _read_status
    raise RemoteDisconnected("Remote end closed connection without"
urllib3.exceptions.ProtocolError: ('Connection aborted.', RemoteDisconnected('Remote end closed connection without response'))

@Augustin-FL
Copy link

Augustin-FL commented Jul 12, 2023

Hello,

I do confirm the behavior and the issue: when using preload_content=False and stream(), Retries does not work correctly on urllib3. requests uses urlib3's stream() function and is therefore also affected by the bug.

By "does not work correctly", i mean that:

  • Retry does not handle errors raised when receiving bytes from a remote server (IncompleteRead, ConnectionResetError, etc), even when it should
  • However, Retry does work correctly if an error is raised when sending bytes to the remote server. (Socket closed, Timeout errors), etc...

Here is a simple script to reproduce the issue

import urllib3
from urllib3.util.retry import Retry
http = urllib3.PoolManager(retries=Retry(total=None, backoff_factor=1))

# This is raising an error (but it shoudn't)
resp = http.request("GET", "http://localhost:5000", preload_content=False)
for chunk in resp.stream(32):
    print(chunk)

# However, this is working correctly (no error)
#resp = http.request("GET", "http://localhost:5000")

This reproduction script needs to make a query to an intentionally broken web server

from flask import Flask, make_response
app = Flask(__name__)

@app.route('/')
# generate an intentionally broken response.
# should trigger an IncompleteRead if no retry strategy is implemented
def send_incomplete_response():
    response = make_response('fourteen chars')
    response.headers['Content-Length'] = '10000' 
    return response

app.run()

The issue is also discussed here : #542

@NamanAgg
Copy link

Any progress on this?

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

5 participants