fix: stop masking non-request errors and accept JSON arrays as data

apiclient/client.py

@@ -7,7 +7,7 @@
 from apiclient.request_formatters import BaseRequestFormatter, NoOpRequestFormatter
 from apiclient.request_strategies import BaseRequestStrategy, RequestStrategy
 from apiclient.response_handlers import BaseResponseHandler, RequestsResponseHandler
-from apiclient.utils.typing import OptionalDict
+from apiclient.utils.typing import JsonType, OptionalDict
 
 LOG = logging.getLogger(__name__)
 
@@ -111,7 +111,7 @@ def clone(self):
         """Enable Prototype pattern on client."""
         return copy(self)
 
-    def post(self, endpoint: str, data: dict, params: OptionalDict = None, **kwargs):
+    def post(self, endpoint: str, data: JsonType, params: OptionalDict = None, **kwargs):
         """Send data and return response data from POST endpoint."""
         LOG.debug("POST %s with %s", endpoint, data)
         return self.get_request_strategy().post(endpoint, data=data, params=params, **kwargs)
@@ -121,12 +121,12 @@ def get(self, endpoint: str, params: OptionalDict = None, **kwargs):
         LOG.debug("GET %s", endpoint)
         return self.get_request_strategy().get(endpoint, params=params, **kwargs)
 
-    def put(self, endpoint: str, data: dict, params: OptionalDict = None, **kwargs):
+    def put(self, endpoint: str, data: JsonType, params: OptionalDict = None, **kwargs):
         """Send data to overwrite resource and return response data from PUT endpoint."""
         LOG.debug("PUT %s with %s", endpoint, data)
         return self.get_request_strategy().put(endpoint, data=data, params=params, **kwargs)
 
-    def patch(self, endpoint: str, data: dict, params: OptionalDict = None, **kwargs):
+    def patch(self, endpoint: str, data: JsonType, params: OptionalDict = None, **kwargs):
         """Send data to update resource and return response data from PATCH endpoint."""
         LOG.debug("PATCH %s with %s", endpoint, data)
         return self.get_request_strategy().patch(endpoint, data=data, params=params, **kwargs)

apiclient/request_strategies.py

@@ -5,7 +5,7 @@
 
 from apiclient.exceptions import UnexpectedError
 from apiclient.response import RequestsResponse, Response
-from apiclient.utils.typing import OptionalDict
+from apiclient.utils.typing import JsonType, OptionalDict, OptionalJsonType
 
 if TYPE_CHECKING:  # pragma: no cover
     # Stupid way of getting around cyclic imports when
@@ -51,19 +51,19 @@ def get_session(self):
     def set_session(self, session: requests.Session):
         self.get_client().set_session(session)
 
-    def post(self, endpoint: str, data: dict, params: OptionalDict = None, **kwargs):
+    def post(self, endpoint: str, data: JsonType, params: OptionalDict = None, **kwargs):
         """Send data and return response data from POST endpoint."""
         return self._make_request(self.get_session().post, endpoint, data=data, params=params, **kwargs)
 
     def get(self, endpoint: str, params: OptionalDict = None, **kwargs):
         """Return response data from GET endpoint."""
         return self._make_request(self.get_session().get, endpoint, params=params, **kwargs)
 
-    def put(self, endpoint: str, data: dict, params: OptionalDict = None, **kwargs):
+    def put(self, endpoint: str, data: JsonType, params: OptionalDict = None, **kwargs):
         """Send data to overwrite resource and return response data from PUT endpoint."""
         return self._make_request(self.get_session().put, endpoint, data=data, params=params, **kwargs)
 
-    def patch(self, endpoint: str, data: dict, params: OptionalDict = None, **kwargs):
+    def patch(self, endpoint: str, data: JsonType, params: OptionalDict = None, **kwargs):
         """Send data to update resource and return response data from PATCH endpoint."""
         return self._make_request(self.get_session().patch, endpoint, data=data, params=params, **kwargs)
 
@@ -77,7 +77,7 @@ def _make_request(
         endpoint: str,
         params: OptionalDict = None,
         headers: OptionalDict = None,
-        data: OptionalDict = None,
+        data: OptionalJsonType = None,
         **kwargs,
     ) -> Response:
         """Make the request with the given method.
@@ -96,7 +96,7 @@ def _make_request(
                     **kwargs,
                 )
             )
-        except Exception as error:
+        except requests.RequestException as error:
             raise UnexpectedError(f"Error when contacting '{endpoint}'") from error
         else:
             self._check_response(response)

tests/integration_tests/test_client_integration.py

@@ -79,11 +79,6 @@ def test_client_response(cassette):
         },
     ]
 
-    # Fails to connect when connecting to non-existent url.
-    with pytest.raises(UnexpectedError) as exc_info:
-        client.get("mock://testserver")
-    assert str(exc_info.value) == "Error when contacting 'mock://testserver'"
-
     # User 10 failed on first attempt 500 with 20001 code
     client.set_error_handler(ClientErrorHandler)
     with pytest.raises(InternalError) as exc_info:

tests/test_request_strategies.py

@@ -1,8 +1,10 @@
 from unittest.mock import Mock, call, sentinel
 
 import pytest
+import requests
 
 from apiclient import APIClient
+from apiclient.exceptions import UnexpectedError
 from apiclient.request_strategies import (
     BaseRequestStrategy,
     QueryParamPaginatedRequestStrategy,
@@ -116,6 +118,39 @@ def test_request_strategy_patch_method_delegates_to_parent_handlers(mock_request
     assert_mock_client_called_once(mock_client, {"data": sentinel.data})
 
 
+def test_request_strategy_post_method_accepts_a_list_body(mock_requests, mock_client):
+    mock_requests.post("mock://testserver.com", json={"active": True}, status_code=200)
+
+    strategy = RequestStrategy()
+    strategy.set_client(mock_client.client)
+
+    response = strategy.post("mock://testserver.com", data=[{"id": sentinel.data}])
+
+    assert response == sentinel.result
+    assert_request_called_once(mock_requests, "mock://testserver.com", "POST")
+    assert_mock_client_called_once(mock_client, [{"id": sentinel.data}])
+
+
+def test_request_strategy_wraps_request_errors_as_unexpected_error(mock_requests, mock_client):
+    mock_requests.get("mock://testserver.com", exc=requests.exceptions.ConnectTimeout)
+
+    strategy = RequestStrategy()
+    strategy.set_client(mock_client.client)
+
+    with pytest.raises(UnexpectedError):
+        strategy.get("mock://testserver.com")
+
+
+def test_request_strategy_does_not_wrap_non_request_errors(mock_requests, mock_client):
+    mock_requests.get("mock://testserver.com", exc=ValueError("not a request error"))
+
+    strategy = RequestStrategy()
+    strategy.set_client(mock_client.client)
+
+    with pytest.raises(ValueError):
+        strategy.get("mock://testserver.com")
+
+
 def test_request_strategy_delete_method_delegates_to_parent_handlers(mock_requests, mock_client):
     mock_requests.delete("mock://testserver.com", json={"active": True}, status_code=200)
 

tests/vcr_cassettes/cassette.yaml

@@ -46,7 +46,8 @@ interactions:
     method: GET
     uri: http://testserver/users/2
   response:
-    body: null
+    body:
+      string: ''
     headers:
       Content-Type: [application/json; charset=utf-8]
     status: {code: 500, message: SERVER_ERROR}
@@ -125,7 +126,8 @@ interactions:
     method: DELETE
     uri: http://testserver/users/4
   response:
-    body: null
+    body:
+      string: ''
     headers:
       Content-Type: [application/json; charset=utf-8]
     status: {code: 200, message: OK}