Repository: infobloxopen/infoblox-client

Issue:
Unable to capture 500 errors
Hello,
currently, it seems not possible to distinguish 404 errors from 5XX using connector's `_get_object`. 
https://github.com/infobloxopen/infoblox-client/blob/d3131bd32efbe101276279ce29cf4d7954e040c7/infoblox_client/connector.py#L341-L344

the connector threat all http error exactly the same. as a result all get_object query are returning `None` object
```python
# Example to reproduce the issue with requests-mock
import pytest
from infoblox_client import connector


def test_infoblox_client_error(requests_mock):
  requests_mock.get(
    "https://example.com/wapi/v2.10.3/zone_auth?",
    text="",
    status_code=500
  )
  conn = connector.Connector(
    {
      "host": "example.com",
      "username": "foo",
      "password": "bar",
      "wapi_version": "2.10.3"
    }
  )
  error_500 = conn.get_object('zone_auth')

  requests_mock.get(
    "https://example.com/wapi/v2.10.3/zone_auth?",
    text="",
    status_code=404
  )
  conn = connector.Connector(
    {
      "host": "example.com",
      "username": "foo",
      "password": "bar",
      "wapi_version": "2.10.3"
    }
  )
  error_404 = conn.get_object('zone_auth')
  assert error_404 != error_500, "Unable to distinguish 404 from 500 error"
  ```
Unfortunately, sometimes the infoblox api is returning 5XX error. It would be a good improvement to raise an error for unexpected results.

In worst case scenario the api is called twice. that could affect infoblox api performances.
https://github.com/infobloxopen/infoblox-client/blob/d3131bd32efbe101276279ce29cf4d7954e040c7/infoblox_client/connector.py#L289-L296

also the name "InfobloxObjectNotFound" is visible in get_object's docstring, but never use.
https://github.com/infobloxopen/infoblox-client/blob/d3131bd32efbe101276279ce29cf4d7954e040c7/infoblox_client/connector.py#L263-L266

thank you a lot for your help





Discussion:

Patch:
diff --git a/infoblox_client/connector.py b/infoblox_client/connector.py
index a2946b6..2e9928b 100644
--- a/infoblox_client/connector.py
+++ b/infoblox_client/connector.py
@@ -44,6 +44,8 @@ CLOUD_WAPI_MAJOR_VERSION = 2
 
 
 def reraise_neutron_exception(func):
+    """This decorator catches third-party exceptions and replaces them with
+    Infoblox exceptions"""
     @functools.wraps(func)
     def callee(*args, **kwargs):
         try:
@@ -269,7 +271,8 @@ class Connector(object):
         Returns:
             A list of the Infoblox objects requested
         Raises:
-            InfobloxObjectNotFound
+            requests.exceptions.HTTPError: If API responded with error HTTP
+            status code.
         """
         self._validate_obj_type_or_die(obj_type, obj_type_expected=False)
 
@@ -287,19 +290,14 @@ class Connector(object):
                                                 paging=paging)
         # Clear proxy flag if wapi version is too old (non-cloud)
         proxy_flag = self.cloud_api_enabled and force_proxy
-        ib_object = self._handle_get_object(obj_type, query_params, extattrs,
-                                            proxy_flag)
-        if ib_object:
-            return ib_object
 
-        # Do second get call with force_proxy if not done yet
-        if self.cloud_api_enabled and not force_proxy:
-            ib_object = self._handle_get_object(obj_type, query_params,
-                                                extattrs, proxy_flag=True)
-            if ib_object:
-                return ib_object
-
-        return None
+        try:
+            return self._handle_get_object(obj_type, query_params, extattrs,
+                                           proxy_flag)
+        except req_exc.HTTPError:
+            # Do second get call with force_proxy if not done yet
+            return self._handle_get_object(obj_type, query_params,
+                                           extattrs, proxy_flag=True)
 
     def _handle_get_object(self, obj_type, query_params, extattrs,
                            proxy_flag=False):
@@ -347,7 +345,7 @@ class Connector(object):
         if r.status_code != requests.codes.ok:
             LOG.warning("Failed on object search with url %s: %s",
                         url, r.content)
-            return None
+            r.raise_for_status()
         return self._parse_reply(r)
 
     @reraise_neutron_exception


Test patch:
diff --git a/tests/test_connector.py b/tests/test_connector.py
index 2410666..e888b7b 100644
--- a/tests/test_connector.py
+++ b/tests/test_connector.py
@@ -356,7 +356,7 @@ class TestInfobloxConnector(unittest.TestCase):
                          url)
 
     def test_get_object_with_proxy_flag(self):
-        self.connector._get_object = mock.MagicMock(return_value=False)
+        self.connector._get_object = mock.MagicMock(return_value=None)
         self.connector._construct_url = mock.MagicMock()
         self.connector.cloud_api_enabled = True
 
@@ -371,7 +371,9 @@ class TestInfobloxConnector(unittest.TestCase):
                                                self.connector._construct_url)
 
     def test_get_object_without_proxy_flag(self):
-        self.connector._get_object = mock.MagicMock(return_value=False)
+        self.connector._get_object = mock.MagicMock(
+            side_effect=[requests.HTTPError(), None],
+        )
         self.connector._construct_url = mock.MagicMock()
         self.connector.cloud_api_enabled = True
 
@@ -382,15 +384,17 @@ class TestInfobloxConnector(unittest.TestCase):
                            mock.call('network', {}, None, force_proxy=True)]
         self.connector._construct_url.assert_has_calls(construct_calls)
 
-    def test__get_object_search_error_return_none(self):
-        response = mock.Mock()
-        response.status_code = '404'
-        response.content = 'Object not found'
+    def test__get_object_raises_search_error(self):
+        url = 'http://some-url/'
+        response = requests.Response()
+        response.status_code = 404
+        response._content = 'Object not found'
+        response.url = url
         self.connector.session = mock.Mock()
         self.connector.session.get.return_value = response
 
-        url = 'http://some-url/'
-        self.assertEqual(None, self.connector._get_object('network', url))
+        with self.assertRaises(requests.HTTPError):
+            self.connector._get_object('network', url)
 
     def test_get_object_with_pagination_with_no_result(self):
         self.connector._get_object = mock.MagicMock(return_value=None)