proxy: extract response error handling to single method

This patch introduces a check_response method to proxy server
Application class. The method provides common checking and reporting
of backend response errors (i.e. responses with status code >=
500). This checking was previously duplicated in proxy controller
classes.

The formatting and content of error logging messages is now more
unform, which does result in changes to some logged error messages.

Change-Id: I4b935a47f39642c11364f2a8d50a5f2ebb3b84f1

2 files changed, 75 insertions, 3 deletions

diff --git a/test/unit/proxy/controllers/test_obj.py b/test/unit/proxy/controllers/test_obj.py
index 5c280602b..7d97c04f1 100644
--- a/test/unit/proxy/controllers/test_obj.py
+++ b/test/unit/proxy/controllers/test_obj.py
@@ -1365,10 +1365,14 @@ class TestReplicatedObjController(CommonObjectControllerMixin,
         req, log_lines = do_test((201, (100, 500), 201))
         if six.PY3:
             # We allow the b'' in logs because we want to see bad characters.
-            self.assertIn("ERROR 500 b'' From Object Server", log_lines[0])
+            self.assertIn(
+                "ERROR 500 b'' Trying to PUT /v1/AUTH_kilroy/%ED%88%8E/"
+                "%E9%90%89 From Object Server", log_lines[0])
             self.assertIn(req.path, log_lines[0])
         else:
-            self.assertIn('ERROR 500  From Object Server', log_lines[0])
+            self.assertIn(
+                'ERROR 500  Trying to PUT /v1/AUTH_kilroy/%ED%88%8E/%E9%90%89 '
+                'From Object Server', log_lines[0])
             self.assertIn(req.path.decode('utf-8'), log_lines[0])
 
     def test_DELETE_errors(self):
diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py
index f3205327a..1b4b034c3 100644
--- a/test/unit/proxy/test_server.py
+++ b/test/unit/proxy/test_server.py
@@ -56,7 +56,7 @@ from test.debug_logger import debug_logger
 from test.unit import (
     connect_tcp, readuntil2crlfs, fake_http_connect, FakeRing, FakeMemcache,
     patch_policies, write_fake_ring, mocked_http_conn, DEFAULT_TEST_EC_TYPE,
-    make_timestamp_iter, skip_if_no_xattrs)
+    make_timestamp_iter, skip_if_no_xattrs, FakeHTTPResponse)
 from test.unit.helpers import setup_servers, teardown_servers
 from swift.proxy import server as proxy_server
 from swift.proxy.controllers.obj import ReplicatedObjectController
@@ -1288,6 +1288,74 @@ class TestProxyServer(unittest.TestCase):
         self.assertIs(log_kwargs['exc_info'][1], expected_err)
         self.assertEqual(4, node_error_count(app, node))
 
+    def test_check_response_200(self):
+        app = proxy_server.Application({},
+                                       account_ring=FakeRing(),
+                                       container_ring=FakeRing(),
+                                       logger=debug_logger())
+        node = app.container_ring.get_part_nodes(1)[0]
+        resp = FakeHTTPResponse(Response())
+        ret = app.check_response(node, 'Container', resp, 'PUT', '/v1/a/c')
+        self.assertTrue(ret)
+        error_lines = app.logger.get_lines_for_level('error')
+        self.assertFalse(error_lines)
+        self.assertEqual(0, node_error_count(app, node))
+
+    def test_check_response_507(self):
+        app = proxy_server.Application({},
+                                       account_ring=FakeRing(),
+                                       container_ring=FakeRing(),
+                                       logger=debug_logger())
+        node = app.container_ring.get_part_nodes(1)[0]
+        resp = FakeHTTPResponse(Response(status=507))
+        ret = app.check_response(node, 'Container', resp, 'PUT', '/v1/a/c')
+        self.assertFalse(ret)
+        error_lines = app.logger.get_lines_for_level('error')
+        self.assertEqual(1, len(error_lines))
+        self.assertEqual('ERROR Insufficient Storage 10.0.0.0:1000/sda',
+                         error_lines[0])
+        self.assertEqual(11, node_error_count(app, node))
+        self.assertTrue(app.error_limited(node))
+
+        app.logger.clear()
+        ret = app.check_response(node, 'Account', resp, 'PUT', '/v1/a/c',
+                                 body='full')
+        self.assertFalse(ret)
+        error_lines = app.logger.get_lines_for_level('error')
+        self.assertEqual(1, len(error_lines))
+        self.assertEqual('ERROR Insufficient Storage 10.0.0.0:1000/sda',
+                         error_lines[0])
+        self.assertEqual(11, node_error_count(app, node))
+        self.assertTrue(app.error_limited(node))
+
+    def test_check_response_503(self):
+        app = proxy_server.Application({},
+                                       account_ring=FakeRing(),
+                                       container_ring=FakeRing(),
+                                       logger=debug_logger())
+        node = app.container_ring.get_part_nodes(1)[0]
+        resp = FakeHTTPResponse(Response(status=503))
+        app.logger.clear()
+        ret = app.check_response(node, 'Container', resp, 'PUT', '/v1/a/c')
+        self.assertFalse(ret)
+        error_lines = app.logger.get_lines_for_level('error')
+        self.assertEqual(1, len(error_lines))
+        self.assertEqual('ERROR 503 Trying to PUT /v1/a/c From Container '
+                         'Server 10.0.0.0:1000/sda', error_lines[0])
+        self.assertEqual(1, node_error_count(app, node))
+        self.assertFalse(app.error_limited(node))
+
+        app.logger.clear()
+        ret = app.check_response(node, 'Object', resp, 'GET', '/v1/a/c/o',
+                                 body='full')
+        self.assertFalse(ret)
+        error_lines = app.logger.get_lines_for_level('error')
+        self.assertEqual(1, len(error_lines))
+        self.assertEqual('ERROR 503 full Trying to GET /v1/a/c/o From Object '
+                         'Server 10.0.0.0:1000/sda', error_lines[0])
+        self.assertEqual(2, node_error_count(app, node))
+        self.assertFalse(app.error_limited(node))
+
     def test_valid_api_version(self):
         app = proxy_server.Application({},
                                        account_ring=FakeRing(),