Improve error handling details (#1122)

google_nest_sdm/auth.py

@@ -15,15 +15,23 @@
 
 import logging
 from abc import ABC, abstractmethod
+from dataclasses import dataclass, field
 from asyncio import TimeoutError
 from typing import Any
+from http import HTTPStatus
 
 import aiohttp
-from aiohttp.client_exceptions import ClientError, ClientResponseError
+from aiohttp.client_exceptions import ClientError
 from google.auth.credentials import Credentials
 from google.oauth2.credentials import Credentials as OAuthCredentials
+from mashumaro.mixins.json import DataClassJSONMixin
 
-from .exceptions import ApiException, AuthException
+from .exceptions import (
+    ApiException,
+    AuthException,
+    ApiForbiddenException,
+    NotFoundException,
+)
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -36,6 +44,55 @@
 MESSAGE = "message"
 
 
+@dataclass
+class Status(DataClassJSONMixin):
+    """Status of the media item."""
+
+    code: int = field(default=HTTPStatus.OK)
+    """The status code, which should be an enum value of google.rpc.Code"""
+
+    message: str | None = None
+    """A developer-facing error message, which should be in English"""
+
+    details: list[dict[str, Any]] = field(default_factory=list)
+    """A list of messages that carry the error details"""
+
+
+@dataclass
+class Error:
+    """Error details from the API response."""
+
+    status: str | None = None
+    code: int | None = None
+    message: str | None = None
+    details: list[dict[str, Any]] | None = field(default_factory=list)
+
+    def __str__(self) -> str:
+        """Return a string representation of the error details."""
+        error_message = ""
+        if self.status:
+            error_message += self.status
+        if self.code:
+            if error_message:
+                error_message += f" ({self.code})"
+            else:
+                error_message += str(self.code)
+        if self.message:
+            if error_message:
+                error_message += ": "
+            error_message += self.message
+        if self.details:
+            error_message += f"\nError details: ({self.details})"
+        return error_message
+
+
+@dataclass
+class ErrorResponse(DataClassJSONMixin):
+    """A response message that contains an error message."""
+
+    error: Error | None = None
+
+
 class AbstractAuth(ABC):
     """Abstract class to make authenticated requests."""
 
@@ -124,34 +181,40 @@ async def post_json(self, url: str, **kwargs: Any) -> dict[str, Any]:
         _LOGGER.debug("response=%s", result)
         return result
 
-    @staticmethod
-    async def _raise_for_status(resp: aiohttp.ClientResponse) -> aiohttp.ClientResponse:
+    @classmethod
+    async def _raise_for_status(
+        cls, resp: aiohttp.ClientResponse
+    ) -> aiohttp.ClientResponse:
         """Raise exceptions on failure methods."""
-        detail = await AbstractAuth._error_detail(resp)
+        error_detail = await cls._error_detail(resp)
         try:
             resp.raise_for_status()
-        except ClientResponseError as err:
-            if err.status == HTTP_UNAUTHORIZED:
-                raise AuthException(f"Unable to authenticate with API: {err}") from err
-            detail.append(err.message)
-            raise ApiException(": ".join(detail)) from err
-        except ClientError as err:
+        except aiohttp.ClientResponseError as err:
+            error_message = f"{err.message} response from API ({resp.status})"
+            if error_detail:
+                error_message += f": {error_detail}"
+            if err.status == HTTPStatus.FORBIDDEN:
+                raise ApiForbiddenException(error_message)
+            if err.status == HTTPStatus.UNAUTHORIZED:
+                raise AuthException(error_message)
+            if err.status == HTTPStatus.NOT_FOUND:
+                raise NotFoundException(error_message)
+            raise ApiException(error_message) from err
+        except aiohttp.ClientError as err:
             raise ApiException(f"Error from API: {err}") from err
         return resp
 
-    @staticmethod
-    async def _error_detail(resp: aiohttp.ClientResponse) -> list[str]:
+    @classmethod
+    async def _error_detail(cls, resp: aiohttp.ClientResponse) -> Error | None:
         """Returns an error message string from the APi response."""
         if resp.status < 400:
-            return []
+            return None
         try:
-            result = await resp.json()
-            error = result.get(ERROR, {})
+            result = await resp.text()
         except ClientError:
-            return []
-        message = ["Error from API", f"{resp.status}"]
-        if STATUS in error:
-            message.append(f"{error[STATUS]}")
-        if MESSAGE in error:
-            message.append(error[MESSAGE])
-        return message
+            return None
+        try:
+            error_response = ErrorResponse.from_json(result)
+        except (LookupError, ValueError):
+            return None
+        return error_response.error

google_nest_sdm/exceptions.py

@@ -17,6 +17,14 @@ class AuthException(ApiException):
     """Raised due to auth problems talking to API or subscriber."""
 
 
+class NotFoundException(ApiException):
+    """Raised when the API returns an error that a resource was not found."""
+
+
+class ApiForbiddenException(ApiException):
+    """Raised when the user is not authorized to perform a specific function."""
+
+
 class ConfigurationException(GoogleNestException):
     """Raised due to misconfiguration problems."""
 

tests/test_google_nest_api.py

@@ -1,13 +1,20 @@
 import json
 from typing import Awaitable, Callable
 from unittest.mock import patch
+import re
+from http import HTTPStatus
 
 import aiohttp
 import pytest
 
 from google_nest_sdm import google_nest_api
 from google_nest_sdm.auth import AbstractAuth
-from google_nest_sdm.exceptions import ApiException, AuthException
+from google_nest_sdm.exceptions import (
+    ApiException,
+    AuthException,
+    NotFoundException,
+    ApiForbiddenException,
+)
 
 from .conftest import (
     FAKE_TOKEN,
@@ -300,10 +307,79 @@ async def test_get_structure_missing_structures(
     assert structure is None
 
 
+@pytest.mark.parametrize(
+    "status,error_dict,exc_type,err_match",
+    [
+        (
+            HTTPStatus.BAD_REQUEST,
+            {
+                "code": 400,
+                "message": "Some error message",
+                "status": "FAILED_PRECONDITION",
+            },
+            ApiException,
+            re.compile(r"Bad Request response from API \(400\).*Some error message"),
+        ),
+        (
+            HTTPStatus.UNAUTHORIZED,
+            {
+                "code": 401,
+                "message": "Some error message",
+                "status": "UNAUTHENTICATED",
+            },
+            AuthException,
+            re.compile(r"Unauthorized response from API \(401\).*Some error message"),
+        ),
+        (
+            HTTPStatus.FORBIDDEN,
+            {
+                "code": 403,
+                "message": "Some error message",
+                "status": "PERMISSION_DENIED",
+            },
+            ApiForbiddenException,
+            re.compile(r"Forbidden response from API \(403\):.*Some error message"),
+        ),
+        (
+            HTTPStatus.NOT_FOUND,
+            {
+                "code": 404,
+                "message": "Some error message",
+                "status": "NOT_FOUND",
+            },
+            NotFoundException,
+            re.compile(r"Not Found response from API \(404\):.*Some error message"),
+        ),
+        (
+            HTTPStatus.INTERNAL_SERVER_ERROR,
+            {
+                "code": 500,
+                "message": "Some error message",
+                "status": "INTERNAL",
+            },
+            ApiException,
+            re.compile(r"Internal Server Error response from API \(500\).*Some error message"),
+        ),
+        (
+            503,
+            {
+                "code": 503,
+                "message": "Some error message",
+                "status": "UNAVAILABLE",
+            },
+            ApiException,
+            re.escape("Service Unavailable response from API (503)"),
+        ),
+    ],
+)
 async def test_api_post_error_with_json_response(
     app: aiohttp.web.Application,
     device_handler: DeviceHandler,
     api_client: Callable[[], Awaitable[google_nest_api.GoogleNestAPI]],
+    status: int,
+    error_dict: dict,
+    exc_type: type[Exception],
+    err_match: str,
 ) -> None:
     device_id = device_handler.add_device(
         traits={
@@ -315,17 +391,15 @@ async def test_api_post_error_with_json_response(
     )
 
     json_response = {
-        "error": {
-            "code": 400,
-            "message": "Some error message",
-            "status": "FAILED_PRECONDITION",
-        },
+        "error": error_dict,
     }
 
     async def fail_handler(request: aiohttp.web.Request) -> aiohttp.web.Response:
         assert request.headers["Authorization"] == "Bearer %s" % FAKE_TOKEN
         return aiohttp.web.Response(
-            status=400, body=json.dumps(json_response), content_type="application/json"
+            status=status,
+            body=json.dumps(json_response),
+            content_type="application/json",
         )
 
     app.router.add_post(f"/{device_id}:executeCommand", fail_handler)
@@ -337,7 +411,5 @@ async def fail_handler(request: aiohttp.web.Request) -> aiohttp.web.Response:
     assert device.name == device_id
     trait = device.traits["sdm.devices.traits.ThermostatTemperatureSetpoint"]
 
-    with pytest.raises(
-        ApiException, match=r".*FAILED_PRECONDITION: Some error message.*"
-    ):
+    with pytest.raises(exc_type, match=err_match):
         await trait.set_heat(25.0)