From 312185a405c90e93d891930b12a5ebdb294bbbe8 Mon Sep 17 00:00:00 2001 From: Paul Asjes Date: Thu, 5 Sep 2024 17:14:37 +0200 Subject: [PATCH 1/9] WIP --- workos/exceptions.py | 66 +++++-------------------------- workos/utils/_base_http_client.py | 28 +++---------- 2 files changed, 15 insertions(+), 79 deletions(-) diff --git a/workos/exceptions.py b/workos/exceptions.py index 21a6923f..0532fec5 100644 --- a/workos/exceptions.py +++ b/workos/exceptions.py @@ -2,76 +2,28 @@ import httpx - # Request related exceptions class BaseRequestException(Exception): def __init__( self, response: httpx.Response, - message: Optional[str] = None, - error: Optional[str] = None, - errors: Optional[Mapping[str, Any]] = None, - error_description: Optional[str] = None, - code: Optional[str] = None, - pending_authentication_token: Optional[str] = None, + response_json: Mapping[str, Any], ) -> None: - super(BaseRequestException, self).__init__(message) - - self.message = message - self.error = error - self.errors = errors - self.code = code - self.error_description = error_description - self.pending_authentication_token = pending_authentication_token - self.extract_and_set_response_related_data(response) + super(BaseRequestException, self).__init__(response_json) - def extract_and_set_response_related_data(self, response: httpx.Response) -> None: self.response = response - - try: - response_json = response.json() - self.message = response_json.get("message") - self.error = response_json.get("error") - self.errors = response_json.get("errors") - self.code = response_json.get("code") - self.error_description = response_json.get("error_description") - self.pending_authentication_token = response_json.get( - "pending_authentication_token" - ) - except ValueError: - self.message = None - self.error = None - self.errors = None - self.code = None - self.error_description = None - self.pending_authentication_token = None - - headers = response.headers - self.request_id = headers.get("X-Request-ID") + self.response_json = response_json def __str__(self) -> str: - message = self.message or "No message" + message = self.response_json.get("message") or "No message" exception = "(message=%s" % message - if self.request_id is not None: - exception += ", request_id=%s" % self.request_id - - if self.code is not None: - exception += ", code=%s" % self.code - - if self.error is not None: - exception += ", error=%s" % self.error - - if self.errors is not None: - exception += ", errors=%s" % self.errors - - if self.error_description is not None: - exception += ", error_description=%s" % self.error_description + for key, value in self.response_json.items(): + if key != "message": + exception += ", %s=%s" % (key, value) - if self.pending_authentication_token is not None: - exception += ( - ", pending_authentication_token=%s" % self.pending_authentication_token - ) + request_id = self.response.headers.get("X-Request-ID") + exception += ", request_id=%s" % request_id return exception + ")" diff --git a/workos/utils/_base_http_client.py b/workos/utils/_base_http_client.py index 1d13b987..fa1f788c 100644 --- a/workos/utils/_base_http_client.py +++ b/workos/utils/_base_http_client.py @@ -96,27 +96,15 @@ def _maybe_raise_error_by_status_code( status_code = response.status_code if status_code >= 400 and status_code < 500: if status_code == 401: - raise AuthenticationException(response) + raise AuthenticationException(response, response_json) elif status_code == 403: - raise AuthorizationException(response) + raise AuthorizationException(response, response_json) elif status_code == 404: - raise NotFoundException(response) - - error = ( - response_json.get("error") - if response_json and "error" in response_json - else "Unknown" - ) - error_description = ( - response_json.get("error_description") - if response_json and "error_description" in response_json - else "Unknown" - ) - raise BadRequestException( - response, error=error, error_description=error_description - ) + raise NotFoundException(response, response_json) + + raise BadRequestException(response, response_json) elif status_code >= 500 and status_code < 600: - raise ServerException(response) + raise ServerException(response, response_json) def _prepare_request( self, @@ -159,10 +147,6 @@ def _prepare_request( if params is not None: params = {k: v for k, v in params.items() if v is not None} - # Remove any body values that are None - if json is not None and isinstance(json, Mapping): - json = {k: v for k, v in json.items() if v is not None} - # We'll spread these return values onto the HTTP client request method if bodyless_http_method: return { From e82d9257b13bf8791f15ac4c3275985ff802412f Mon Sep 17 00:00:00 2001 From: Paul Asjes Date: Mon, 9 Sep 2024 13:56:56 +0200 Subject: [PATCH 2/9] Fixed tests --- tests/test_sync_http_client.py | 24 +++--------------------- workos/exceptions.py | 27 ++++++++++++++++++++------- workos/utils/_base_http_client.py | 4 ++++ 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/tests/test_sync_http_client.py b/tests/test_sync_http_client.py index 4c2f27ad..530ccad1 100644 --- a/tests/test_sync_http_client.py +++ b/tests/test_sync_http_client.py @@ -204,7 +204,7 @@ def test_request_exceptions_include_expected_request_data( # This'll fail for sure here but... just using the nice error that'd come up assert ex.__class__ == expected_exception - def test_bad_request_exceptions_include_expected_request_data(self): + def test_bad_request_exceptions_include_request_data(self): request_id = "request-123" error = "example_error" error_description = "Example error description" @@ -212,7 +212,7 @@ def test_bad_request_exceptions_include_expected_request_data(self): self.http_client._client.request = MagicMock( return_value=httpx.Response( status_code=400, - json={"error": error, "error_description": error_description}, + json={"error": error, "error_description": error_description, "foo": "bar"}, headers={"X-Request-ID": request_id}, ), ) @@ -222,29 +222,11 @@ def test_bad_request_exceptions_include_expected_request_data(self): except BadRequestException as ex: assert ( str(ex) - == "(message=No message, request_id=request-123, error=example_error, error_description=Example error description)" + == "(message=No message, error=example_error, error_description=Example error description, foo=bar, request_id=request-123)" ) except Exception as ex: assert ex.__class__ == BadRequestException - def test_bad_request_exceptions_exclude_expected_request_data(self): - request_id = "request-123" - - self.http_client._client.request = MagicMock( - return_value=httpx.Response( - status_code=400, - json={"foo": "bar"}, - headers={"X-Request-ID": request_id}, - ), - ) - - try: - self.http_client.request("bad_place") - except BadRequestException as ex: - assert str(ex) == "(message=No message, request_id=request-123)" - except Exception as ex: - assert ex.__class__ == BadRequestException - def test_request_bad_body_raises_expected_exception_with_request_data(self): request_id = "request-123" diff --git a/workos/exceptions.py b/workos/exceptions.py index 0532fec5..f1ea68c9 100644 --- a/workos/exceptions.py +++ b/workos/exceptions.py @@ -14,16 +14,29 @@ def __init__( self.response = response self.response_json = response_json + self.message = self.extractFromJson("message", "No message") + self.error = self.extractFromJson("error", None) + self.errors = self.extractFromJson("errors", None) + self.code = self.extractFromJson("code", None ) + self.error_description = self.extractFromJson("error_description", None) + + self.request_id = response.headers.get("X-Request-ID") + + def extractFromJson(self, key: str, alt: Optional[str] = None) -> Optional[str]: + if self.response_json is None: + return alt + + return self.response_json.get(key, alt) + def __str__(self) -> str: - message = self.response_json.get("message") or "No message" - exception = "(message=%s" % message + exception = "(message=%s" % self.message - for key, value in self.response_json.items(): - if key != "message": - exception += ", %s=%s" % (key, value) + if self.response_json is not None: + for key, value in self.response_json.items(): + if key != "message": + exception += ", %s=%s" % (key, value) - request_id = self.response.headers.get("X-Request-ID") - exception += ", request_id=%s" % request_id + exception += ", request_id=%s" % self.request_id return exception + ")" diff --git a/workos/utils/_base_http_client.py b/workos/utils/_base_http_client.py index fa1f788c..d0d1c04e 100644 --- a/workos/utils/_base_http_client.py +++ b/workos/utils/_base_http_client.py @@ -147,6 +147,10 @@ def _prepare_request( if params is not None: params = {k: v for k, v in params.items() if v is not None} + # Remove any body values that are None + if json is not None and isinstance(json, Mapping): + json = {k: v for k, v in json.items() if v is not None} + # We'll spread these return values onto the HTTP client request method if bodyless_http_method: return { From a43004e3bf5f0c4f73d043c5181b05daf3e832fd Mon Sep 17 00:00:00 2001 From: Paul Asjes Date: Mon, 9 Sep 2024 14:00:31 +0200 Subject: [PATCH 3/9] Move 'request_id' back to the beginning --- tests/test_sync_http_client.py | 2 +- workos/exceptions.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_sync_http_client.py b/tests/test_sync_http_client.py index 530ccad1..b4359c0d 100644 --- a/tests/test_sync_http_client.py +++ b/tests/test_sync_http_client.py @@ -222,7 +222,7 @@ def test_bad_request_exceptions_include_request_data(self): except BadRequestException as ex: assert ( str(ex) - == "(message=No message, error=example_error, error_description=Example error description, foo=bar, request_id=request-123)" + == "(message=No message, request_id=request-123, error=example_error, error_description=Example error description, foo=bar)" ) except Exception as ex: assert ex.__class__ == BadRequestException diff --git a/workos/exceptions.py b/workos/exceptions.py index f1ea68c9..50b9b813 100644 --- a/workos/exceptions.py +++ b/workos/exceptions.py @@ -30,14 +30,13 @@ def extractFromJson(self, key: str, alt: Optional[str] = None) -> Optional[str]: def __str__(self) -> str: exception = "(message=%s" % self.message + exception += ", request_id=%s" % self.request_id if self.response_json is not None: for key, value in self.response_json.items(): if key != "message": exception += ", %s=%s" % (key, value) - exception += ", request_id=%s" % self.request_id - return exception + ")" From 7abec3e473e04831cfdf558115011dcd8d41b413 Mon Sep 17 00:00:00 2001 From: Paul Asjes Date: Mon, 9 Sep 2024 14:04:01 +0200 Subject: [PATCH 4/9] linting --- tests/test_sync_http_client.py | 6 +++++- workos/exceptions.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_sync_http_client.py b/tests/test_sync_http_client.py index b4359c0d..1aef7ed0 100644 --- a/tests/test_sync_http_client.py +++ b/tests/test_sync_http_client.py @@ -212,7 +212,11 @@ def test_bad_request_exceptions_include_request_data(self): self.http_client._client.request = MagicMock( return_value=httpx.Response( status_code=400, - json={"error": error, "error_description": error_description, "foo": "bar"}, + json={ + "error": error, + "error_description": error_description, + "foo": "bar", + }, headers={"X-Request-ID": request_id}, ), ) diff --git a/workos/exceptions.py b/workos/exceptions.py index 50b9b813..8e75639f 100644 --- a/workos/exceptions.py +++ b/workos/exceptions.py @@ -17,7 +17,7 @@ def __init__( self.message = self.extractFromJson("message", "No message") self.error = self.extractFromJson("error", None) self.errors = self.extractFromJson("errors", None) - self.code = self.extractFromJson("code", None ) + self.code = self.extractFromJson("code", None) self.error_description = self.extractFromJson("error_description", None) self.request_id = response.headers.get("X-Request-ID") From 6e89fa0d04192edd35e7291ca9078d248c26a9c5 Mon Sep 17 00:00:00 2001 From: Paul Asjes Date: Mon, 9 Sep 2024 14:08:50 +0200 Subject: [PATCH 5/9] More linting, now with correct black version --- workos/exceptions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/workos/exceptions.py b/workos/exceptions.py index 8e75639f..0de475bf 100644 --- a/workos/exceptions.py +++ b/workos/exceptions.py @@ -2,6 +2,7 @@ import httpx + # Request related exceptions class BaseRequestException(Exception): def __init__( From c36ea86557e16ed7b7251955f6f98aa202503903 Mon Sep 17 00:00:00 2001 From: Paul Asjes Date: Mon, 9 Sep 2024 14:13:47 +0200 Subject: [PATCH 6/9] Make mypy happy --- workos/exceptions.py | 2 +- workos/utils/_base_http_client.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/workos/exceptions.py b/workos/exceptions.py index 0de475bf..89d66436 100644 --- a/workos/exceptions.py +++ b/workos/exceptions.py @@ -8,7 +8,7 @@ class BaseRequestException(Exception): def __init__( self, response: httpx.Response, - response_json: Mapping[str, Any], + response_json: Optional[Mapping[str, Any]], ) -> None: super(BaseRequestException, self).__init__(response_json) diff --git a/workos/utils/_base_http_client.py b/workos/utils/_base_http_client.py index d0d1c04e..30c1bca4 100644 --- a/workos/utils/_base_http_client.py +++ b/workos/utils/_base_http_client.py @@ -181,7 +181,7 @@ def _handle_response(self, response: httpx.Response) -> ResponseJson: try: response_json = response.json() except ValueError: - raise ServerException(response) + raise ServerException(response, None) self._maybe_raise_error_by_status_code(response, response_json) From 594ee65c842c0cbaefdfa96f3757ce90920f2bda Mon Sep 17 00:00:00 2001 From: Paul Asjes Date: Mon, 9 Sep 2024 14:19:00 +0200 Subject: [PATCH 7/9] Update async tests --- tests/test_async_http_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_async_http_client.py b/tests/test_async_http_client.py index 5f316ee1..4d9c5425 100644 --- a/tests/test_async_http_client.py +++ b/tests/test_async_http_client.py @@ -227,7 +227,7 @@ async def test_bad_request_exceptions_exclude_expected_request_data(self): try: await self.http_client.request("bad_place") except BadRequestException as ex: - assert str(ex) == "(message=No message, request_id=request-123)" + assert str(ex) == "(message=No message, request_id=request-123, foo=bar)" except Exception as ex: assert ex.__class__ == BadRequestException From 0ac2c988c397f4f8293c450c324ee1f80131de66 Mon Sep 17 00:00:00 2001 From: Paul Asjes Date: Mon, 9 Sep 2024 17:25:03 +0200 Subject: [PATCH 8/9] Address feedback --- workos/exceptions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/workos/exceptions.py b/workos/exceptions.py index 89d66436..cc71a433 100644 --- a/workos/exceptions.py +++ b/workos/exceptions.py @@ -16,10 +16,10 @@ def __init__( self.response_json = response_json self.message = self.extractFromJson("message", "No message") - self.error = self.extractFromJson("error", None) + self.error = self.extractFromJson("error", "Unknown") self.errors = self.extractFromJson("errors", None) self.code = self.extractFromJson("code", None) - self.error_description = self.extractFromJson("error_description", None) + self.error_description = self.extractFromJson("error_description", "Unknown") self.request_id = response.headers.get("X-Request-ID") From 4ee8ddffd874a9fa31a4c63bf77633b233cce545 Mon Sep 17 00:00:00 2001 From: Paul Asjes Date: Mon, 9 Sep 2024 17:27:07 +0200 Subject: [PATCH 9/9] lol snake case --- workos/exceptions.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/workos/exceptions.py b/workos/exceptions.py index cc71a433..acd1901c 100644 --- a/workos/exceptions.py +++ b/workos/exceptions.py @@ -15,15 +15,15 @@ def __init__( self.response = response self.response_json = response_json - self.message = self.extractFromJson("message", "No message") - self.error = self.extractFromJson("error", "Unknown") - self.errors = self.extractFromJson("errors", None) - self.code = self.extractFromJson("code", None) - self.error_description = self.extractFromJson("error_description", "Unknown") + self.message = self.extract_from_json("message", "No message") + self.error = self.extract_from_json("error", "Unknown") + self.errors = self.extract_from_json("errors", None) + self.code = self.extract_from_json("code", None) + self.error_description = self.extract_from_json("error_description", "Unknown") self.request_id = response.headers.get("X-Request-ID") - def extractFromJson(self, key: str, alt: Optional[str] = None) -> Optional[str]: + def extract_from_json(self, key: str, alt: Optional[str] = None) -> Optional[str]: if self.response_json is None: return alt