From 647680fe8b38fbf4d8d1b5441ffe040bdaef7d4b Mon Sep 17 00:00:00 2001
From: Valerij Maljulin <vmaljuli@redhat.com>
Date: Oct 14 2019 12:10:53 +0000
Subject: Handle requests exceptions

This fixes #479

Signed-off-by: Valerij Maljulin <vmaljuli@redhat.com>

---

diff --git a/greenwave/request_session.py b/greenwave/request_session.py
index d878872..53aa24b 100644
--- a/greenwave/request_session.py
+++ b/greenwave/request_session.py
@@ -1,16 +1,43 @@
 import requests
 
+from json import dumps
 from requests.adapters import HTTPAdapter
-# pylint: disable=import-error
-from requests.packages.urllib3.util.retry import Retry
+from requests.exceptions import ConnectionError, ConnectTimeout, RetryError
+from urllib3.util.retry import Retry
+from urllib3.exceptions import ProxyError, SSLError
 
 from greenwave import __version__
 
 
+class ErrorResponse(requests.Response):
+    def __init__(self, status_code, error_message, url):
+        super().__init__()
+        self.status_code = status_code
+        self._error_message = error_message
+        self.url = url
+        self.reason = error_message.encode()
+
+    @property
+    def content(self):
+        return dumps({'message' : self._error_message}).encode()
+
+
+class RequestsSession(requests.Session):
+    def request(self, *args, **kwargs):
+        req_url = kwargs.get('url', args[1])
+        try:
+            return super().request(*args, **kwargs)
+        except (ConnectionError, ProxyError, SSLError) as e:
+            ret_val = ErrorResponse(502, str(e), req_url)
+        except (ConnectTimeout, RetryError) as e:
+            ret_val = ErrorResponse(504, str(e), req_url)
+        return ret_val
+
+
 def get_requests_session():
     """ Get http(s) session for request processing.  """
 
-    session = requests.Session()
+    session = RequestsSession()
     retry = Retry(
         total=3,
         read=3,
diff --git a/greenwave/tests/test_request_session.py b/greenwave/tests/test_request_session.py
new file mode 100644
index 0000000..dde05eb
--- /dev/null
+++ b/greenwave/tests/test_request_session.py
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+from mock import patch
+from json import loads
+from greenwave.request_session import get_requests_session
+from requests.exceptions import ConnectionError
+
+
+@patch('requests.adapters.HTTPAdapter.send')
+def test_retry_handler(mocked_request):
+    msg_text = 'It happens...'
+    mocked_request.side_effect = ConnectionError(msg_text)
+    session = get_requests_session()
+    resp = session.get('http://localhost.localdomain')
+    assert resp.status_code == 502
+    assert loads(resp.content) == {'message': msg_text}
diff --git a/greenwave/tests/test_retrieve_gating_yaml.py b/greenwave/tests/test_retrieve_gating_yaml.py
index 031198c..86a88c9 100644
--- a/greenwave/tests/test_retrieve_gating_yaml.py
+++ b/greenwave/tests/test_retrieve_gating_yaml.py
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0+
 
 import socket
+from requests.exceptions import ConnectionError, HTTPError
 
 import pytest
 import mock
@@ -96,6 +97,27 @@ def test_retrieve_yaml_remote_rule_no_namespace():
             assert session.request.mock_calls == [expected_call]
 
 
+def test_retrieve_yaml_remote_rule_connection_error():
+    app = greenwave.app_factory.create_app()
+    with app.app_context():
+        with mock.patch('requests.Session.request') as mocked_request:
+            # Return 404, because we are only interested in the URL in the request
+            # and whether it is correct even with empty namespace.
+            response = mock.MagicMock()
+            response.status_code = 200
+            mocked_request.side_effect = [
+                response, ConnectionError('Something went terribly wrong...')
+            ]
+
+            with pytest.raises(HTTPError) as excinfo:
+                retrieve_yaml_remote_rule("deadbeaf", "pkg", "")
+
+            assert str(excinfo.value) == (
+                '502 Server Error: Something went terribly wrong... for url: '
+                'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml'
+            )
+
+
 @mock.patch('greenwave.resources.xmlrpc.client.ServerProxy')
 def test_retrieve_scm_from_koji_build_socket_error(mock_xmlrpc_client):
     mock_auth_server = mock_xmlrpc_client.return_value
diff --git a/greenwave/tests/test_utils.py b/greenwave/tests/test_utils.py
index 5fca72d..a15b89a 100644
--- a/greenwave/tests/test_utils.py
+++ b/greenwave/tests/test_utils.py
@@ -18,10 +18,7 @@ from greenwave.utils import json_error
     (ConnectionError('ERROR'), 502, 'ERROR'),
     (ConnectTimeout('TIMEOUT'), 502, 'TIMEOUT'),
     (Timeout('TIMEOUT'), 504, 'TIMEOUT'),
-    (InternalServerError(), 500, 'The server encountered an internal error'),
-    (urllib3.exceptions.MaxRetryError(
-        'MAX_RETRY', '.../gating.yaml'), 502, ('There was an error retrieving the '
-                                               'gating.yaml file at .../gating.yaml'))
+    (InternalServerError(), 500, 'The server encountered an internal error')
 ])
 def test_json_connection_error(error, expected_status_code,
                                expected_error_message_part):
diff --git a/greenwave/utils.py b/greenwave/utils.py
index a2717a6..68e397e 100644
--- a/greenwave/utils.py
+++ b/greenwave/utils.py
@@ -35,13 +35,6 @@ def json_error(error):
         current_app.logger.exception('Timeout error: {}'.format(error))
         msg = 'Timeout connecting to upstream server: {}'.format(error)
         status_code = 504
-    elif isinstance(error, urllib3.exceptions.MaxRetryError):
-        current_app.logger.exception('Connection error: {}'.format(error))
-        if error.url.endswith('gating.yaml'):
-            msg = 'There was an error retrieving the gating.yaml file at {}'.format(error.url)
-        else:
-            msg = 'Error connecting to {}'.format(error.url)
-        status_code = 502
     else:
         current_app.logger.exception('Unexpected server error: {}'.format(error))
         msg = 'Server encountered unexpected error'