utils: fix error handling for http requests in utils in py3 (#709)

* utils: fix error handling for http requests in utils in py3

* corrected bytestring handling by replacing string cast with decode

* more robust string type checking for error messages

* _http: RequestException: revert type handling for json and str formatting

google/datalab/utils/_http.py

@@ -44,17 +44,17 @@ def __init__(self, status, content):
     self.message = 'HTTP request failed'
     # Try extract a message from the body; swallow possible resulting ValueErrors and KeyErrors.
     try:
-      if type(content) == str:
+      if isinstance(content, str):
         error = json.loads(content)['error']
       else:
         error = json.loads(str(content, encoding='UTF-8'))['error']
       if 'errors' in error:
         error = error['errors'][0]
-      self.message += ': ' + error['message']
+      self.message += ': {}'.format(error['message'])
     except Exception:
-      lines = content.split('\n') if isinstance(content, basestring) else []
+      lines = content.splitlines() if isinstance(content, basestring) else []
       if lines:
-        self.message += ': ' + lines[0]
+        self.message += ': {}'.format(lines[0])
 
   def __str__(self):
     return self.message

tests/_util/http_tests.py

@@ -92,7 +92,20 @@ def test_parses_json_response(self, mock_request, mock_response):
 
   @mock.patch('httplib2.Response')
   @mock.patch('google.datalab.utils._http.Http.http.request')
-  def test_raises_http_error(self, mock_request, mock_response):
+  def test_raises_http_error_json(self, mock_request, mock_response):
+    TestCases._setup_mocks(
+      mock_request, mock_response,
+      b'{"error": {"errors": [{"message": "Not Found"}]}}', 404)
+    with self.assertRaises(Exception) as error:
+      Http.request('http://www.example.org')
+
+    e = error.exception
+    self.assertEqual(e.status, 404)
+    self.assertEqual(e.message, 'HTTP request failed: Not Found')
+
+  @mock.patch('httplib2.Response')
+  @mock.patch('google.datalab.utils._http.Http.http.request')
+  def test_raises_http_error_str(self, mock_request, mock_response):
     TestCases._setup_mocks(mock_request, mock_response, 'Not Found', 404)
 
     with self.assertRaises(Exception) as error:
@@ -102,6 +115,18 @@ def test_raises_http_error(self, mock_request, mock_response):
     self.assertEqual(e.status, 404)
     self.assertEqual(e.content, 'Not Found')
 
+  @mock.patch('httplib2.Response')
+  @mock.patch('google.datalab.utils._http.Http.http.request')
+  def test_raises_http_error_bytes(self, mock_request, mock_response):
+    TestCases._setup_mocks(mock_request, mock_response, b'Not Found', 404)
+
+    with self.assertRaises(Exception) as error:
+      Http.request('http://www.example.org')
+
+    e = error.exception
+    self.assertEqual(e.status, 404)
+    self.assertEqual(e.content, b'Not Found')
+
   @staticmethod
   def _setup_mocks(mock_request, mock_response, content, status=200):
     response = mock_response()