diff --git a/linode_api4/errors.py b/linode_api4/errors.py index bc2df6108..511ac8c57 100644 --- a/linode_api4/errors.py +++ b/linode_api4/errors.py @@ -1,4 +1,11 @@ +# Necessary to maintain compatibility with Python < 3.11 +from __future__ import annotations + from builtins import super +from json import JSONDecodeError +from typing import Any, Dict, Optional + +from requests import Response class ApiError(RuntimeError): @@ -8,14 +15,90 @@ class ApiError(RuntimeError): often, this will be caused by invalid input to the API. """ - def __init__(self, message, status=400, json=None): + def __init__( + self, + message: str, + status: int = 400, + json: Optional[Dict[str, Any]] = None, + response: Optional[Response] = None, + ): super().__init__(message) + self.status = status self.json = json + self.response = response + self.errors = [] + if json and "errors" in json and isinstance(json["errors"], list): self.errors = [e["reason"] for e in json["errors"]] + @classmethod + def from_response( + cls, + response: Response, + message: Optional[str] = None, + disable_formatting: bool = False, + ) -> Optional[ApiError]: + """ + Creates an ApiError object from the given response, + or None if the response does not contain an error. + + :arg response: The response to create an ApiError from. + :arg message: An optional message to prepend to the error's message. + :arg disable_formatting: If true, the error's message will not automatically be formatted + with details from the API response. + + :returns: The new API error. + """ + + if response.status_code < 400 or response.status_code > 599: + # No error was found + return None + + request = response.request + + try: + response_json = response.json() + except JSONDecodeError: + response_json = None + + # Use the user-defined message is formatting is disabled + if disable_formatting: + return cls( + message, + status=response.status_code, + json=response_json, + response=response, + ) + + # Build the error string + error_fmt = "N/A" + + if response_json is not None and "errors" in response_json: + errors = [] + + for error in response_json["errors"]: + field = error.get("field") + reason = error.get("reason") + errors.append(f"{field + ': ' if field else ''}{reason}") + + error_fmt = "; ".join(errors) + + elif len(response.text or "") > 0: + error_fmt = response.text + + return cls( + ( + f"{message + ': ' if message is not None else ''}" + f"{f'{request.method} {request.path_url}: ' if request else ''}" + f"[{response.status_code}] {error_fmt}" + ), + status=response.status_code, + json=response_json, + response=response, + ) + class UnexpectedResponseError(RuntimeError): """ @@ -26,7 +109,41 @@ class UnexpectedResponseError(RuntimeError): library, and should be fixed with changes to this codebase. """ - def __init__(self, message, status=200, json=None): + def __init__( + self, + message: str, + status: int = 200, + json: Optional[Dict[str, Any]] = None, + response: Optional[Response] = None, + ): super().__init__(message) + self.status = status self.json = json + self.response = response + + @classmethod + def from_response( + cls, + message: str, + response: Response, + ) -> Optional[UnexpectedResponseError]: + """ + Creates an UnexpectedResponseError object from the given response and message. + + :arg message: The message to create this error with. + :arg response: The response to create an UnexpectedResponseError from. + :returns: The new UnexpectedResponseError. + """ + + try: + response_json = response.json() + except JSONDecodeError: + response_json = None + + return cls( + message, + status=response.status_code, + json=response_json, + response=response, + ) diff --git a/linode_api4/linode_client.py b/linode_api4/linode_client.py index 1bbc631b7..dbb45d0df 100644 --- a/linode_api4/linode_client.py +++ b/linode_api4/linode_client.py @@ -287,23 +287,9 @@ def _api_call( if warning: logger.warning("Received warning from server: {}".format(warning)) - if 399 < response.status_code < 600: - j = None - error_msg = "{}: ".format(response.status_code) - try: - j = response.json() - if "errors" in j.keys(): - for e in j["errors"]: - msg = e.get("reason", "") - field = e.get("field", None) - - error_msg += "{}{}; ".format( - f"[{field}] " if field is not None else "", - msg, - ) - except: - pass - raise ApiError(error_msg, status=response.status_code, json=j) + api_error = ApiError.from_response(response) + if api_error is not None: + raise api_error if response.status_code != 204: j = response.json() diff --git a/linode_api4/login_client.py b/linode_api4/login_client.py index 1263ee49c..e21c5c4b2 100644 --- a/linode_api4/login_client.py +++ b/linode_api4/login_client.py @@ -434,10 +434,9 @@ def oauth_redirect(): ) if r.status_code != 200: - raise ApiError( - "OAuth token exchange failed", - status=r.status_code, - json=r.json(), + raise ApiError.from_response( + r, + message="OAuth token exchange failed", ) token = r.json()["access_token"] @@ -479,7 +478,7 @@ def refresh_oauth_token(self, refresh_token): ) if r.status_code != 200: - raise ApiError("Refresh failed", r) + raise ApiError.from_response(r, message="Refresh failed") token = r.json()["access_token"] scopes = OAuthScopes.parse(r.json()["scopes"]) @@ -516,5 +515,5 @@ def expire_token(self, token): ) if r.status_code != 200: - raise ApiError("Failed to expire token!", r) + raise ApiError.from_response(r, "Failed to expire token!") return True diff --git a/linode_api4/objects/account.py b/linode_api4/objects/account.py index 4777ff1c4..375e5fc03 100644 --- a/linode_api4/objects/account.py +++ b/linode_api4/objects/account.py @@ -436,8 +436,10 @@ def thumbnail(self, dump_to=None): ) if not result.status_code == 200: - raise ApiError( - "No thumbnail found for OAuthClient {}".format(self.id) + raise ApiError.from_response( + result, + "No thumbnail found for OAuthClient {}".format(self.id), + disable_formatting=True, ) if dump_to: @@ -472,12 +474,9 @@ def set_thumbnail(self, thumbnail): data=thumbnail, ) - if not result.status_code == 200: - errors = [] - j = result.json() - if "errors" in j: - errors = [e["reason"] for e in j["errors"]] - raise ApiError("{}: {}".format(result.status_code, errors), json=j) + api_exc = ApiError.from_response(result) + if api_exc is not None: + raise api_exc return True diff --git a/linode_api4/objects/support.py b/linode_api4/objects/support.py index f835b3f31..548f58f16 100644 --- a/linode_api4/objects/support.py +++ b/linode_api4/objects/support.py @@ -174,12 +174,9 @@ def upload_attachment(self, attachment: Union[Path, str]): files={"file": f}, ) - if not result.status_code == 200: - errors = [] - j = result.json() - if "errors" in j: - errors = [e["reason"] for e in j["errors"]] - raise ApiError("{}: {}".format(result.status_code, errors), json=j) + api_exc = ApiError.from_response(result) + if api_exc is not None: + raise api_exc return True diff --git a/test/integration/linode_client/test_errors.py b/test/integration/linode_client/test_errors.py new file mode 100644 index 000000000..2c3ab57b5 --- /dev/null +++ b/test/integration/linode_client/test_errors.py @@ -0,0 +1,28 @@ +from linode_api4.errors import ApiError + + +def test_error_404(test_linode_client): + api_exc = None + + try: + test_linode_client.get("/invalid/endpoint") + except ApiError as exc: + api_exc = exc + + assert str(api_exc) == "GET /v4beta/invalid/endpoint: [404] Not found" + + +def test_error_400(test_linode_client): + api_exc = None + + try: + test_linode_client.linode.instance_create( + "g6-fake-plan", "us-fakeregion" + ) + except ApiError as exc: + api_exc = exc + + assert str(api_exc) == ( + "POST /v4beta/linode/instances: [400] type: A valid plan type by that ID was not found; " + "region: region is not valid" + ) diff --git a/test/unit/errors_test.py b/test/unit/errors_test.py new file mode 100644 index 000000000..017c96280 --- /dev/null +++ b/test/unit/errors_test.py @@ -0,0 +1,104 @@ +from types import SimpleNamespace +from unittest import TestCase + +from linode_api4.errors import ApiError, UnexpectedResponseError + + +class ApiErrorTest(TestCase): + def test_from_response(self): + mock_response = SimpleNamespace( + status_code=400, + json=lambda: { + "errors": [ + {"reason": "foo"}, + {"field": "bar", "reason": "oh no"}, + ] + }, + text='{"errors": [{"reason": "foo"}, {"field": "bar", "reason": "oh no"}]}', + request=SimpleNamespace( + method="POST", + path_url="foo/bar", + ), + ) + + exc = ApiError.from_response(mock_response) + + assert str(exc) == "POST foo/bar: [400] foo; bar: oh no" + assert exc.status == 400 + assert exc.json == { + "errors": [{"reason": "foo"}, {"field": "bar", "reason": "oh no"}] + } + assert exc.response.request.method == "POST" + assert exc.response.request.path_url == "foo/bar" + + def test_from_response_non_json_body(self): + mock_response = SimpleNamespace( + status_code=500, + json=lambda: None, + text="foobar", + request=SimpleNamespace( + method="POST", + path_url="foo/bar", + ), + ) + + exc = ApiError.from_response(mock_response) + + assert str(exc) == "POST foo/bar: [500] foobar" + assert exc.status == 500 + assert exc.json is None + assert exc.response.request.method == "POST" + assert exc.response.request.path_url == "foo/bar" + + def test_from_response_empty_body(self): + mock_response = SimpleNamespace( + status_code=500, + json=lambda: None, + text=None, + request=SimpleNamespace( + method="POST", + path_url="foo/bar", + ), + ) + + exc = ApiError.from_response(mock_response) + + assert str(exc) == "POST foo/bar: [500] N/A" + assert exc.status == 500 + assert exc.json is None + assert exc.response.request.method == "POST" + assert exc.response.request.path_url == "foo/bar" + + def test_from_response_no_request(self): + mock_response = SimpleNamespace( + status_code=500, json=lambda: None, text="foobar", request=None + ) + + exc = ApiError.from_response(mock_response) + + assert str(exc) == "[500] foobar" + assert exc.status == 500 + assert exc.json is None + assert exc.response.request is None + + +class UnexpectedResponseErrorTest(TestCase): + def test_from_response(self): + mock_response = SimpleNamespace( + status_code=400, + json=lambda: { + "foo": "bar", + }, + request=SimpleNamespace( + method="POST", + path_url="foo/bar", + ), + ) + + exc = UnexpectedResponseError.from_response("foobar", mock_response) + + assert str(exc) == "foobar" + assert exc.status == 400 + assert exc.json == {"foo": "bar"} + assert exc.response.request.method == "POST" + assert exc.response.request.path_url == "foo/bar"