RedHatInsights / insights-core

Insights Core is a data collection and processing framework used by Red Hat Insights
https://cloud.redhat.com/insights
Apache License 2.0
153 stars 183 forks source link

Catch Timeout on test connection #4191

Closed Glutexo closed 2 months ago

Glutexo commented 3 months ago

All Pull Requests:

Check all that apply:

Complete Description of Additions/Changes:

The --test-connection command only used to catch ConnectionError, including ConnectionTimeout, but not other timeouts such as ReadTimeoutError. Re-used the existing REQUEST_FAILED_EXCEPTIONS list to process all timeouts in the same way.

There is an integration test pull request https://github.com/RedHatInsights/insights-client/pull/244 for this fix.

Card IDs:

Glutexo commented 3 months ago

There are no tests yet. I need to add them before un-drafting this pul request.

Glutexo commented 3 months ago

Before:

# INSIGHTS_HTTP_TIMEOUT=0.1 insights-client --test-connection
Running Connection Tests...
=== Begin Upload URL Connection Test ===
Testing https://cert-api.access.redhat.com/r/insights/platform/ingress/v1/upload
POST https://cert-api.access.redhat.com/r/insights/platform/ingress/v1/upload
Fatal error
Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/urllib3/connectionpool.py", line 446, in _make_request
    six.raise_from(e, None)
  File "<string>", line 3, in raise_from
  File "/usr/lib/python3.9/site-packages/urllib3/connectionpool.py", line 441, in _make_request
    httplib_response = conn.getresponse()
  File "/usr/lib64/python3.9/http/client.py", line 1377, in getresponse
    response.begin()
  File "/usr/lib64/python3.9/http/client.py", line 320, in begin
    version, status, reason = self._read_status()
  File "/usr/lib64/python3.9/http/client.py", line 281, in _read_status
    line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1")
  File "/usr/lib64/python3.9/socket.py", line 704, in readinto
    return self._sock.recv_into(b)
  File "/usr/lib64/python3.9/ssl.py", line 1275, in recv_into
    return self.read(nbytes, buffer)
  File "/usr/lib64/python3.9/ssl.py", line 1133, in read
    return self._sslobj.read(len, buffer)
socket.timeout: The read operation timed out

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/requests/adapters.py", line 439, in send
    resp = conn.urlopen(
  File "/usr/lib/python3.9/site-packages/urllib3/connectionpool.py", line 756, in urlopen
    retries = retries.increment(
  File "/usr/lib/python3.9/site-packages/urllib3/util/retry.py", line 532, in increment
    raise six.reraise(type(error), error, _stacktrace)
  File "/usr/lib/python3.9/site-packages/urllib3/packages/six.py", line 709, in reraise
    raise value
  File "/usr/lib/python3.9/site-packages/urllib3/connectionpool.py", line 700, in urlopen
    httplib_response = self._make_request(
  File "/usr/lib/python3.9/site-packages/urllib3/connectionpool.py", line 448, in _make_request
    self._raise_timeout(err=e, url=url, timeout_value=read_timeout)
  File "/usr/lib/python3.9/site-packages/urllib3/connectionpool.py", line 337, in _raise_timeout
    raise ReadTimeoutError(
urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host='cert-api.access.redhat.com', port=443): Read timed out. (read timeout=0.1)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/insights/insights-core/insights/client/phase/v1.py", line 36, in _f
    func(client, config)
  File "/home/insights/insights-core/insights/client/phase/v1.py", line 102, in pre_update
    rc = client.test_connection()
  File "/home/insights/insights-core/insights/client/__init__.py", line 69, in _init_connection
    return func(self, *args, **kwargs)
  File "/home/insights/insights-core/insights/client/__init__.py", line 87, in test_connection
    return self.connection.test_connection()
  File "/home/insights/insights-core/insights/client/connection.py", line 399, in test_connection
    upload_success = self._test_urls(self.upload_url, "POST")
  File "/home/insights/insights-core/insights/client/connection.py", line 374, in _test_urls
    test_req = self.post(url, files=test_files)
  File "/home/insights/insights-core/insights/client/connection.py", line 191, in post
    return self._http_request(url, 'POST', **kwargs)
  File "/home/insights/insights-core/insights/client/connection.py", line 175, in _http_request
    res = self.session.request(url=url, method=method, timeout=self.config.http_timeout, **kwargs)
  File "/usr/lib/python3.9/site-packages/requests/sessions.py", line 544, in request
    resp = self.send(prep, **send_kwargs)
  File "/usr/lib/python3.9/site-packages/requests/sessions.py", line 657, in send
    r = adapter.send(request, **kwargs)
  File "/usr/lib/python3.9/site-packages/requests/adapters.py", line 529, in send
    raise ReadTimeout(e, request=request)
requests.exceptions.ReadTimeout: HTTPSConnectionPool(host='cert-api.access.redhat.com', port=443): Read timed out. (read timeout=0.1)

After:

# INSIGHTS_HTTP_TIMEOUT=0.1 insights-client --test-connection
Running Connection Tests...
=== Begin Upload URL Connection Test ===
Testing https://cert-api.access.redhat.com/r/insights/platform/ingress/v1/upload
POST https://cert-api.access.redhat.com/r/insights/platform/ingress/v1/upload
HTTPSConnectionPool(host='cert-api.access.redhat.com', port=443): Read timed out. (read timeout=0.1)
Connectivity test failed! Please check your network configuration
Additional information may be in /var/log/insights-client/insights-client.log
Glutexo commented 3 months ago

I improved the tests by raising actual exceptions: SSLError (a non-Timeout ConnectionError), ConnectionTimeout (both a Timeout and a ConnectionError) and ReadTimeout (a non-ConnectionError Timeout) rather than the abstract ancestors ConnectionError and Timeout that are caught in the code.

Rebased on current master.

I see the python2x tests are failing. I believed we dropped suppport for Python 2. I will fix the unsupported syntax.

Glutexo commented 3 months ago

Fixed the unsupported Python 2 syntax – an f-string. Rebased on the current master.

Glutexo commented 3 months ago

Re-requesting a review. @m-horky might be the right guy for it

Glutexo commented 3 months ago

I found the culprit for the exception being printed twice:

There is another try-except block in the __testurls function, that only catches ConnectionError (not Timeout). There, the exception is printed and then re-raised. This is completely redundant, as __testurls is only used in _testconnection, which has its own error handling.

I think this can go into this pull request. Will remove.

xiangce commented 2 months ago

Hi @Glutexo - Sorry for bringing this trouble to you, as we just resolved an urgent bug within the repo. Please rebase the master branch of your fork of insights-core and then rebase this branch again. Please reach out to me for any problems when doing the rebase. Apologize again.

Glutexo commented 2 months ago

@xiangce Thank you for reminding me! I fixed everything. However, I accidentally deleted my original remote branch before rebasing. As a result, this pull request got closed and it’s not possible to re-open it even after a new push. I‘ll open a new one.