Merge pull request #128 from datastax/feature/#126-value-error

Fix #126 - improve handling of API errors

astrapy/api.py

@@ -0,0 +1,150 @@
+import logging
+import httpx
+from typing import Any, Dict, Optional, TypeVar, cast
+
+from astrapy.types import API_RESPONSE
+from astrapy.utils import amake_request, make_request
+
+T = TypeVar("T", bound="APIRequestHandler")
+AT = TypeVar("AT", bound="AsyncAPIRequestHandler")
+
+
+logger = logging.getLogger(__name__)
+
+
+class APIRequestError(ValueError):
+    def __init__(self, response: httpx.Response) -> None:
+        super().__init__(response.text)
+
+        self.response = response
+
+    def __repr__(self) -> str:
+        return f"{self.response}"
+
+
+class APIRequestHandler:
+    def __init__(
+        self: T,
+        client: httpx.Client,
+        base_url: str,
+        auth_header: str,
+        token: str,
+        method: str,
+        json_data: Optional[Dict[str, Any]],
+        url_params: Optional[Dict[str, Any]],
+        path: Optional[str] = None,
+        skip_error_check: bool = False,
+    ) -> None:
+        self.client = client
+        self.base_url = base_url
+        self.auth_header = auth_header
+        self.token = token
+        self.method = method
+        self.path = path
+        self.json_data = json_data
+        self.url_params = url_params
+        self.skip_error_check = skip_error_check
+
+    def raw_request(self: T) -> httpx.Response:
+        return make_request(
+            client=self.client,
+            base_url=self.base_url,
+            auth_header=self.auth_header,
+            token=self.token,
+            method=self.method,
+            path=self.path,
+            json_data=self.json_data,
+            url_params=self.url_params,
+        )
+
+    def request(self: T) -> API_RESPONSE:
+        # Make the raw request to the API
+        self.response = self.raw_request()
+
+        # If the response was not successful (non-success error code) raise an error directly
+        self.response.raise_for_status()
+
+        # Otherwise, process the successful response
+        return self._process_response()
+
+    def _process_response(self: T) -> API_RESPONSE:
+        # In case of other successful responses, parse the JSON body.
+        try:
+            # Cast the response to the expected type.
+            response_body: API_RESPONSE = cast(API_RESPONSE, self.response.json())
+
+            # If the API produced an error, warn and return the API request error class
+            if "errors" in response_body and not self.skip_error_check:
+                logger.debug(response_body["errors"])
+
+                raise APIRequestError(self.response)
+
+            # Otherwise, set the response body
+            return response_body
+        except ValueError:
+            # Handle cases where json() parsing fails (e.g., empty body)
+            raise APIRequestError(self.response)
+
+
+class AsyncAPIRequestHandler:
+    def __init__(
+        self: AT,
+        client: httpx.AsyncClient,
+        base_url: str,
+        auth_header: str,
+        token: str,
+        method: str,
+        json_data: Optional[Dict[str, Any]],
+        url_params: Optional[Dict[str, Any]],
+        path: Optional[str] = None,
+        skip_error_check: bool = False,
+    ) -> None:
+        self.client = client
+        self.base_url = base_url
+        self.auth_header = auth_header
+        self.token = token
+        self.method = method
+        self.path = path
+        self.json_data = json_data
+        self.url_params = url_params
+        self.skip_error_check = skip_error_check
+
+    async def raw_request(self: AT) -> httpx.Response:
+        return await amake_request(
+            client=self.client,
+            base_url=self.base_url,
+            auth_header=self.auth_header,
+            token=self.token,
+            method=self.method,
+            path=self.path,
+            json_data=self.json_data,
+            url_params=self.url_params,
+        )
+
+    async def request(self: AT) -> API_RESPONSE:
+        # Make the raw request to the API
+        self.response = await self.raw_request()
+
+        # If the response was not successful (non-success error code) raise an error directly
+        self.response.raise_for_status()
+
+        # Otherwise, process the successful response
+        return await self._process_response()
+
+    async def _process_response(self: AT) -> API_RESPONSE:
+        # In case of other successful responses, parse the JSON body.
+        try:
+            # Cast the response to the expected type.
+            response_body: API_RESPONSE = cast(API_RESPONSE, self.response.json())
+
+            # If the API produced an error, warn and return the API request error class
+            if "errors" in response_body and not self.skip_error_check:
+                logger.debug(response_body["errors"])
+
+                raise APIRequestError(self.response)
+
+            # Otherwise, set the response body
+            return response_body
+        except ValueError:
+            # Handle cases where json() parsing fails (e.g., empty body)
+            raise APIRequestError(self.response)

astrapy/db.py

@@ -15,10 +15,11 @@
 
 import asyncio
 import httpx
-import json
 import logging
+import json
 import threading
 
+
 from concurrent.futures import ThreadPoolExecutor
 from functools import partial
 from queue import Queue
@@ -37,6 +38,7 @@
     AsyncGenerator,
 )
 
+from astrapy.api import AsyncAPIRequestHandler, APIRequestHandler
 from astrapy.defaults import (
     DEFAULT_AUTH_HEADER,
     DEFAULT_JSON_API_PATH,
@@ -59,6 +61,7 @@
     AsyncPaginableRequestMethod,
 )
 
+
 logger = logging.getLogger(__name__)
 
 
@@ -109,7 +112,7 @@ def _request(
         skip_error_check: bool = False,
         **kwargs: Any,
     ) -> API_RESPONSE:
-        response = make_request(
+        request_handler = APIRequestHandler(
             client=self.client,
             base_url=self.astra_db.base_url,
             auth_header=DEFAULT_AUTH_HEADER,
@@ -118,13 +121,13 @@ def _request(
             path=path,
             json_data=json_data,
             url_params=url_params,
+            skip_error_check=skip_error_check,
+            **kwargs,
         )
-        responsebody = cast(API_RESPONSE, response.json())
 
-        if not skip_error_check and "errors" in responsebody:
-            raise ValueError(json.dumps(responsebody["errors"]))
-        else:
-            return responsebody
+        response = request_handler.request()
+
+        return response
 
     def _get(
         self, path: Optional[str] = None, options: Optional[Dict[str, Any]] = None
@@ -955,7 +958,7 @@ async def _request(
         skip_error_check: bool = False,
         **kwargs: Any,
     ) -> API_RESPONSE:
-        response = await amake_request(
+        arequest_handler = AsyncAPIRequestHandler(
             client=self.client,
             base_url=self.astra_db.base_url,
             auth_header=DEFAULT_AUTH_HEADER,
@@ -964,13 +967,12 @@ async def _request(
             path=path,
             json_data=json_data,
             url_params=url_params,
+            skip_error_check=skip_error_check,
         )
-        responsebody = cast(API_RESPONSE, response.json())
 
-        if not skip_error_check and "errors" in responsebody:
-            raise ValueError(json.dumps(responsebody["errors"]))
-        else:
-            return responsebody
+        response = await arequest_handler.request()
+
+        return response
 
     async def _get(
         self, path: Optional[str] = None, options: Optional[Dict[str, Any]] = None

astrapy/ops.py

@@ -16,8 +16,9 @@
 from typing import Any, cast, Dict, Optional
 
 import httpx
+from astrapy.api import APIRequestHandler
 
-from astrapy.utils import make_request, http_methods
+from astrapy.utils import http_methods
 from astrapy.defaults import (
     DEFAULT_DEV_OPS_AUTH_HEADER,
     DEFAULT_DEV_OPS_API_VERSION,
@@ -56,34 +57,44 @@ def _ops_request(
     ) -> httpx.Response:
         _options = {} if options is None else options
 
-        return make_request(
+        request_handler = APIRequestHandler(
             client=self.client,
             base_url=self.base_url,
-            method=method,
             auth_header=DEFAULT_DEV_OPS_AUTH_HEADER,
             token=self.token,
+            method=method,
+            path=path,
             json_data=json_data,
             url_params=_options,
-            path=path,
         )
 
+        response = request_handler.raw_request()
+
+        return response
+
     def _json_ops_request(
         self,
         method: str,
         path: str,
         options: Optional[Dict[str, Any]] = None,
         json_data: Optional[Dict[str, Any]] = None,
     ) -> OPS_API_RESPONSE:
-        req_result = self._ops_request(
+        _options = {} if options is None else options
+
+        request_handler = APIRequestHandler(
+            client=self.client,
+            base_url=self.base_url,
+            auth_header="Authorization",
+            token=self.token,
             method=method,
             path=path,
-            options=options,
             json_data=json_data,
+            url_params=_options,
         )
-        return cast(
-            OPS_API_RESPONSE,
-            req_result.json(),
-        )
+
+        response = request_handler.request()
+
+        return response
 
     def get_databases(
         self, options: Optional[Dict[str, Any]] = None

astrapy/utils.py

@@ -141,8 +141,6 @@ async def amake_request(
     if logger.isEnabledFor(logging.DEBUG):
         log_request_response(r, json_data)
 
-    r.raise_for_status()
-
     return r
 
 

tests/astrapy/test_async_db_dml.py

@@ -23,6 +23,7 @@
 
 import pytest
 
+from astrapy.api import APIRequestError
 from astrapy.types import API_DOC
 from astrapy.db import AsyncAstraDB, AsyncAstraDBCollection
 
@@ -176,7 +177,7 @@ async def test_find_error(
     sort = {"$vector": "clearly not a list of floats!"}
     options = {"limit": 100}
 
-    with pytest.raises(ValueError):
+    with pytest.raises(APIRequestError):
         await async_readonly_vector_collection.find(sort=sort, options=options)