From c506d17983d98615dff4238ad85ffc7dc3e20adc Mon Sep 17 00:00:00 2001
From: Alex Corvin <acorvin@redhat.com>
Date: Jun 20 2018 16:46:59 +0000
Subject: Use more specific error codes for upstream errors


Previously Greenwave returned a generic 500 error for issues connecting
to upstream services (e.g. WaiverDB). This change updates the error
handler to return a 502 or 504 for connection issues or timeout issues,
respectively.

---

diff --git a/greenwave/tests/test_utils.py b/greenwave/tests/test_utils.py
index 000fba4..65f3a31 100644
--- a/greenwave/tests/test_utils.py
+++ b/greenwave/tests/test_utils.py
@@ -5,7 +5,7 @@ import pytest
 
 import json
 
-from requests import ConnectionError, ConnectTimeout
+from requests import ConnectionError, ConnectTimeout, Timeout
 from werkzeug.exceptions import InternalServerError
 
 import greenwave.app_factory
@@ -43,16 +43,19 @@ def test_retry_count():
     assert sum(calls) == 3
 
 
-@pytest.mark.parametrize('error, expected_error_message_part', [
-    (ConnectionError('ERROR'), 'ERROR'),
-    (ConnectTimeout('TIMEOUT'), 'TIMEOUT'),
-    (InternalServerError(), 'The server encountered an internal error'),
+@pytest.mark.parametrize(('error, expected_status_code,'
+                          'expected_error_message_part'), [
+    (ConnectionError('ERROR'), 502, 'ERROR'),
+    (ConnectTimeout('TIMEOUT'), 502, 'TIMEOUT'),
+    (Timeout('TIMEOUT'), 504, 'TIMEOUT'),
+    (InternalServerError(), 500, 'The server encountered an internal error')
 ])
-def test_json_error(error, expected_error_message_part):
+def test_json_connection_error(error, expected_status_code,
+                               expected_error_message_part):
     app = greenwave.app_factory.create_app()
     with app.app_context():
         with app.test_request_context():
             r = json_error(error)
             data = json.loads(r.get_data())
-            assert r.status_code == 500
+            assert r.status_code == expected_status_code
             assert expected_error_message_part in data['message']
diff --git a/greenwave/utils.py b/greenwave/utils.py
index 9b95892..66b4417 100644
--- a/greenwave/utils.py
+++ b/greenwave/utils.py
@@ -10,6 +10,7 @@ import hashlib
 import yaml
 from flask import jsonify, current_app, request
 from flask.config import Config
+from requests import ConnectionError, Timeout
 from werkzeug.exceptions import HTTPException
 import greenwave.policies
 
@@ -29,10 +30,21 @@ def json_error(error):
         response = jsonify(message=error.description)
         response.status_code = error.code
     else:
-        # Could be ConnectionError or Timeout
-        current_app.logger.exception('Returning 500 to user.')
-        response = jsonify(message=str(error))
-        response.status_code = 500
+        if isinstance(error, ConnectionError):
+            current_app.logger.exception('ConnectionError, returning 502 to user.')
+            msg = 'Error connecting to upstream server: {err}'
+            status_code = 502
+        elif isinstance(error, Timeout):
+            current_app.logger.exception('Timeout error, returning 504 to user.')
+            msg = 'Timeout connecting to upstream server: {err}'
+            status_code = 504
+        else:
+            current_app.logger.exception('Returning 500 to user.')
+            msg = '{err}'
+            status_code = 500
+
+        response = jsonify(message=msg.format(err=error))
+        response.status_code = status_code
 
     response = insert_headers(response)