Merge lp:~facundo/ubuntu-rest-scopes/ignore-network-server-crashes into lp:ubuntu-rest-scopes

[Facundo Batista]

Ignore network and server errors when hitting backends.

Ignore network and server errors when hitting backends.

This code is copied from curucu, where we ended up doing the same thing.

Note that logging and informing metrics is not done in that code, as is done already when hitting the network...

[Guillermo Gonzalez (verterok)]

LGTM

review: Approve

=== modified file 'src/rest_scopes.py'
--- src/rest_scopes.py	2016-03-14 06:20:14 +0000
+++ src/rest_scopes.py	2017-05-16 19:13:32 +0000
@@ -1,9 +1,10 @@
-# Copyright 2014-2015 Canonical
+# Copyright 2014-2017 Canonical
 # All Rights Reserved
 
 """Rest Scopes server."""
 
 import collections
+import httplib
 import importlib
 import json
 import logging
@@ -299,7 +300,24 @@
 
         # fix environment and serve
         environ["PATH_INFO"] = scope_path
-        return scope(environ, start_response)
+        try:
+            return scope(environ, start_response)
+        except urllib2.HTTPError as exc:
+            if exc.code < 500:
+                raise
+            error_message = str(exc)
+        except urllib2.URLError as exc:
+            error_message = "URLError: " + str(exc.reason)
+        except httplib.HTTPException as exc:
+            error_message = "HTTPException: " + str(exc)
+
+        # the backend didn't behave
+        msg = "Original error: " + error_message
+        headers = {
+            'Content-Type': 'text/plain',
+            'Content-Length': str(len(msg)),
+        }
+        return self._error("502 BAD GATEWAY", start_response, msg, **headers)
 
     def curate_list(self, country_blacklist, scopes):
         """Remove blacklisted scopes."""

=== modified file 'src/tests/test_rest_scopes.py'
--- src/tests/test_rest_scopes.py	2016-02-11 13:16:24 +0000
+++ src/tests/test_rest_scopes.py	2017-05-16 19:13:32 +0000
@@ -1,8 +1,9 @@
-# Copyright 2014-2015 Canonical
+# Copyright 2014-2017 Canonical
 # All Rights Reserved
 
 """Tests for the Rest Scopes server."""
 
+import httplib
 import io
 import json
 import uuid
@@ -211,6 +212,63 @@
         }
         app_mock.assert_called_with(new_environ, 'start_response')
 
+    def test_exception_problem(self):
+        app = rest_scopes.App()
+        app.scope_apps['somescope'] = app_mock = Mock()
+        environ = {'PATH_INFO': '/somescope/somemethod'}
+
+        # should transmit the problem
+        app_mock.side_effect = ValueError("scope crashed")
+        self.assertRaises(ValueError, app, environ, 'start_response')
+
+    def test_exception_httperror_problem(self):
+        app = rest_scopes.App()
+        app.scope_apps['somescope'] = app_mock = Mock()
+        environ = {'PATH_INFO': '/somescope/somemethod'}
+
+        # should transmit the problem
+        app_mock.side_effect = urllib2.HTTPError(None, 404, "Not Found", {}, None)
+        self.assertRaises(urllib2.HTTPError, app, environ, 'start_response')
+
+    def test_exception_httperror_ignore(self):
+        app = rest_scopes.App()
+        app.scope_apps['somescope'] = app_mock = Mock()
+        environ = {'PATH_INFO': '/somescope/somemethod'}
+
+        # ignore server side errors, should return 502
+        app_mock.side_effect = urllib2.HTTPError(None, 500, "Internal Server Error", {}, None)
+        start_response = Mock()
+        resp = app(environ, start_response)
+        msg = json.loads(resp[0])['message']
+        self.assertEqual(msg, "Original error: HTTP Error 500: Internal Server Error")
+        start_response.assert_called_with('502 BAD GATEWAY', ANY)
+
+    def test_exception_urlerror_ignore(self):
+        app = rest_scopes.App()
+        app.scope_apps['somescope'] = app_mock = Mock()
+        environ = {'PATH_INFO': '/somescope/somemethod'}
+
+        # ignore network errors, should return 502
+        app_mock.side_effect = urllib2.URLError("test problem")
+        start_response = Mock()
+        resp = app(environ, start_response)
+        msg = json.loads(resp[0])['message']
+        self.assertEqual(msg, "Original error: URLError: test problem")
+        start_response.assert_called_with('502 BAD GATEWAY', ANY)
+
+    def test_exception_httpexception_ignore(self):
+        app = rest_scopes.App()
+        app.scope_apps['somescope'] = app_mock = Mock()
+        environ = {'PATH_INFO': '/somescope/somemethod'}
+
+        # ignore network errors, should return 502
+        app_mock.side_effect = httplib.HTTPException("test problem")
+        start_response = Mock()
+        resp = app(environ, start_response)
+        msg = json.loads(resp[0])['message']
+        self.assertEqual(msg, "Original error: HTTPException: test problem")
+        start_response.assert_called_with('502 BAD GATEWAY', ANY)
+
 
 class ListRemoteScopesTestCase(TestCase):
     """Test the remote scopes listing."""
@@ -409,7 +467,7 @@
             'name': 'name',
             'description': 'desc',
             'author': 'John Doe',
-            'base_url':  "http://localhost:8000/somescope",
+            'base_url': "http://localhost:8000/somescope",
             'invisible': False,
             'icon': None,
             'art': None,
@@ -514,7 +572,7 @@
             'name': 'name',
             'description': 'desc',
             'author': 'John Doe',
-            'base_url':  "http://localhost:8000/somescope",
+            'base_url': "http://localhost:8000/somescope",
             'invisible': False,
             'icon': None,
             'art': None,