From 5628895ac9d8c5e84a0a4d5cd7541c17ca49bea1 Mon Sep 17 00:00:00 2001 From: jake-valsamis Date: Fri, 13 Jun 2025 17:43:44 -0400 Subject: [PATCH 1/7] add error handling to client.py using structure defined in errors.py --- src/codeocean/client.py | 40 +++++++++++++++++++++++--- src/codeocean/errors.py | 63 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 src/codeocean/errors.py diff --git a/src/codeocean/client.py b/src/codeocean/client.py index e3b3b6c..80aa2af 100644 --- a/src/codeocean/client.py +++ b/src/codeocean/client.py @@ -3,12 +3,21 @@ from dataclasses import dataclass from requests_toolbelt.adapters.socket_options import TCPKeepAliveAdapter from requests_toolbelt.sessions import BaseUrlSession -from typing import Optional +from typing import Optional, Union from urllib3.util import Retry +import requests from codeocean.capsule import Capsules from codeocean.computation import Computations from codeocean.data_asset import DataAssets +from codeocean.errors import ( + BadRequestError, + UnauthorizedError, + ForbiddenError, + NotFoundError, + InternalServerError, + CodeOceanError +) @dataclass @@ -45,11 +54,34 @@ def __post_init__(self): }) if self.agent_id: self.session.headers.update({"Agent-Id": self.agent_id}) - self.session.hooks["response"] = [ - lambda response, *args, **kwargs: response.raise_for_status() - ] + self.session.hooks["response"] = [self.error_handler] self.session.mount(self.domain, TCPKeepAliveAdapter(max_retries=self.retries)) self.capsules = Capsules(client=self.session) self.computations = Computations(client=self.session) self.data_assets = DataAssets(client=self.session) + + def error_handler(self, response, *args, **kwargs): + try: + response.raise_for_status() + except requests.HTTPError as ex: + try: + error_payload = response.json() + if not isinstance(error_payload, dict): + raise ValueError("Response is not a JSON object") + except ValueError: + error_payload = {"message": response.text or str(ex)} + + status_code = response.status_code + if status_code == 400: + raise BadRequestError.from_dict(error_payload).with_error(ex) + elif status_code == 401: + raise UnauthorizedError.from_dict(error_payload).with_error(ex) + elif status_code == 403: + raise ForbiddenError.from_dict(error_payload).with_error(ex) + elif status_code == 404: + raise NotFoundError.from_dict(error_payload).with_error(ex) + elif status_code >= 500: + raise InternalServerError.from_dict(error_payload).with_error(ex) + else: + raise CodeOceanError.from_dict(error_payload).with_error(ex) diff --git a/src/codeocean/errors.py b/src/codeocean/errors.py new file mode 100644 index 0000000..6697c84 --- /dev/null +++ b/src/codeocean/errors.py @@ -0,0 +1,63 @@ +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Optional, List, Type, TypeVar, Dict, Any +import requests + +T = TypeVar("T", bound="Error") + + +@dataclass +class Error(Exception): + message: str + items: Optional[List[str]] = None + error: Optional[requests.HTTPError] = field(default=None, repr=False) + + def with_error(self, ex: requests.HTTPError) -> Error: + self.error = ex + return self + + @classmethod + def from_dict(cls: Type[T], payload: Dict[str, Any]) -> T: + message = payload.get("message", "An error occurred.") + items = payload.get("items") if isinstance(payload.get("items"), list) else None + return cls(message=message, items=items) + + def __str__(self) -> str: + return self.message + + +@dataclass +class BadRequestError(Error): + """HTTP 400""" + pass + + +@dataclass +class UnauthorizedError(Error): + """HTTP 401""" + pass + + +@dataclass +class ForbiddenError(Error): + """HTTP 403""" + pass + + +@dataclass +class NotFoundError(Error): + """HTTP 404""" + pass + + +@dataclass +class InternalServerError(Error): + """HTTP 5xx""" + pass + + +@dataclass +class CodeOceanError(Error): + """Fallback for unexpected errors""" + pass From 0c3fc2343e0b3b7854187724284657d8e1600b1a Mon Sep 17 00:00:00 2001 From: Zvika Gart Date: Fri, 18 Jul 2025 14:43:36 +0100 Subject: [PATCH 2/7] Single Error class --- src/codeocean/__init__.py | 1 + src/codeocean/client.py | 38 +++-------------- src/codeocean/error.py | 21 ++++++++++ src/codeocean/errors.py | 63 ---------------------------- tests/test_error.py | 86 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 114 insertions(+), 95 deletions(-) create mode 100644 src/codeocean/error.py delete mode 100644 src/codeocean/errors.py create mode 100644 tests/test_error.py diff --git a/src/codeocean/__init__.py b/src/codeocean/__init__.py index 7650029..affb630 100644 --- a/src/codeocean/__init__.py +++ b/src/codeocean/__init__.py @@ -1 +1,2 @@ from codeocean.client import CodeOcean # noqa: F401 +from codeocean.error import Error # noqa: F401 diff --git a/src/codeocean/client.py b/src/codeocean/client.py index 80aa2af..33a42ba 100644 --- a/src/codeocean/client.py +++ b/src/codeocean/client.py @@ -3,21 +3,14 @@ from dataclasses import dataclass from requests_toolbelt.adapters.socket_options import TCPKeepAliveAdapter from requests_toolbelt.sessions import BaseUrlSession -from typing import Optional, Union +from typing import Optional from urllib3.util import Retry import requests from codeocean.capsule import Capsules from codeocean.computation import Computations from codeocean.data_asset import DataAssets -from codeocean.errors import ( - BadRequestError, - UnauthorizedError, - ForbiddenError, - NotFoundError, - InternalServerError, - CodeOceanError -) +from codeocean.error import Error @dataclass @@ -54,34 +47,15 @@ def __post_init__(self): }) if self.agent_id: self.session.headers.update({"Agent-Id": self.agent_id}) - self.session.hooks["response"] = [self.error_handler] + self.session.hooks["response"] = [self._error_handler] self.session.mount(self.domain, TCPKeepAliveAdapter(max_retries=self.retries)) self.capsules = Capsules(client=self.session) self.computations = Computations(client=self.session) self.data_assets = DataAssets(client=self.session) - def error_handler(self, response, *args, **kwargs): + def _error_handler(self, response, *args, **kwargs): try: response.raise_for_status() - except requests.HTTPError as ex: - try: - error_payload = response.json() - if not isinstance(error_payload, dict): - raise ValueError("Response is not a JSON object") - except ValueError: - error_payload = {"message": response.text or str(ex)} - - status_code = response.status_code - if status_code == 400: - raise BadRequestError.from_dict(error_payload).with_error(ex) - elif status_code == 401: - raise UnauthorizedError.from_dict(error_payload).with_error(ex) - elif status_code == 403: - raise ForbiddenError.from_dict(error_payload).with_error(ex) - elif status_code == 404: - raise NotFoundError.from_dict(error_payload).with_error(ex) - elif status_code >= 500: - raise InternalServerError.from_dict(error_payload).with_error(ex) - else: - raise CodeOceanError.from_dict(error_payload).with_error(ex) + except requests.HTTPError as err: + raise Error(err) from err diff --git a/src/codeocean/error.py b/src/codeocean/error.py new file mode 100644 index 0000000..a466927 --- /dev/null +++ b/src/codeocean/error.py @@ -0,0 +1,21 @@ +from __future__ import annotations + +import requests + + +class Error(Exception): + + def __init__(self, err: requests.HTTPError): + self.status_code = err.response.status_code + self.message = "An error occurred." + self.items = None + + try: + body = err.response.json() + if isinstance(body, dict): + self.message = body.get("message", self.message) + elif isinstance(body, list): + self.items = body + except Exception: + # response wasn't JSON – fall back to text + self.message = err.response.text diff --git a/src/codeocean/errors.py b/src/codeocean/errors.py deleted file mode 100644 index 6697c84..0000000 --- a/src/codeocean/errors.py +++ /dev/null @@ -1,63 +0,0 @@ -from __future__ import annotations - -from dataclasses import dataclass, field -from typing import Optional, List, Type, TypeVar, Dict, Any -import requests - -T = TypeVar("T", bound="Error") - - -@dataclass -class Error(Exception): - message: str - items: Optional[List[str]] = None - error: Optional[requests.HTTPError] = field(default=None, repr=False) - - def with_error(self, ex: requests.HTTPError) -> Error: - self.error = ex - return self - - @classmethod - def from_dict(cls: Type[T], payload: Dict[str, Any]) -> T: - message = payload.get("message", "An error occurred.") - items = payload.get("items") if isinstance(payload.get("items"), list) else None - return cls(message=message, items=items) - - def __str__(self) -> str: - return self.message - - -@dataclass -class BadRequestError(Error): - """HTTP 400""" - pass - - -@dataclass -class UnauthorizedError(Error): - """HTTP 401""" - pass - - -@dataclass -class ForbiddenError(Error): - """HTTP 403""" - pass - - -@dataclass -class NotFoundError(Error): - """HTTP 404""" - pass - - -@dataclass -class InternalServerError(Error): - """HTTP 5xx""" - pass - - -@dataclass -class CodeOceanError(Error): - """Fallback for unexpected errors""" - pass diff --git a/tests/test_error.py b/tests/test_error.py new file mode 100644 index 0000000..cd8224a --- /dev/null +++ b/tests/test_error.py @@ -0,0 +1,86 @@ +import unittest +from unittest.mock import Mock +import requests + +from codeocean.error import Error + + +class TestError(unittest.TestCase): + """Test cases for the Error exception class.""" + + def test_error_is_exception_subclass(self): + """Test that Error is a subclass of Exception.""" + self.assertTrue(issubclass(Error, Exception)) + + def test_error_with_json_dict_message(self): + """Test Error creation with JSON dict response containing message.""" + # Create mock HTTPError and response + mock_response = Mock() + mock_response.status_code = 400 + mock_response.json.return_value = {"message": "Custom error message"} + + mock_http_error = Mock(spec=requests.HTTPError) + mock_http_error.response = mock_response + + # Create Error instance + error = Error(mock_http_error) + + # Verify attributes + self.assertEqual(error.status_code, 400) + self.assertEqual(error.message, "Custom error message") + self.assertIsNone(error.items) + + def test_error_with_json_dict_no_message(self): + """Test Error creation with JSON dict response without message field.""" + # Create mock HTTPError and response + mock_response = Mock() + mock_response.status_code = 500 + mock_response.json.return_value = {"error": "some other field"} + + mock_http_error = Mock(spec=requests.HTTPError) + mock_http_error.response = mock_response + + # Create Error instance + error = Error(mock_http_error) + + # Verify attributes + self.assertEqual(error.status_code, 500) + self.assertEqual(error.message, "An error occurred.") + self.assertIsNone(error.items) + + def test_error_with_json_list(self): + """Test Error creation with JSON list response.""" + # Create mock HTTPError and response + mock_response = Mock() + mock_response.status_code = 403 + mock_response.json.return_value = [{"field": "error1"}, {"field": "error2"}] + + mock_http_error = Mock(spec=requests.HTTPError) + mock_http_error.response = mock_response + + # Create Error instance + error = Error(mock_http_error) + + # Verify attributes + self.assertEqual(error.status_code, 403) + self.assertEqual(error.message, "An error occurred.") + self.assertEqual(error.items, [{"field": "error1"}, {"field": "error2"}]) + + def test_error_with_non_json_response(self): + """Test Error creation when response is not JSON.""" + # Create mock HTTPError and response + mock_response = Mock() + mock_response.status_code = 404 + mock_response.json.side_effect = Exception("Not JSON") + mock_response.text = "Page not found" + + mock_http_error = Mock(spec=requests.HTTPError) + mock_http_error.response = mock_response + + # Create Error instance + error = Error(mock_http_error) + + # Verify attributes + self.assertEqual(error.status_code, 404) + self.assertEqual(error.message, "Page not found") + self.assertIsNone(error.items) From 1cc4cf0beb2d624bbd5a55de491e36b55cee3f56 Mon Sep 17 00:00:00 2001 From: Zvika Gart Date: Mon, 21 Jul 2025 10:55:27 +0100 Subject: [PATCH 3/7] Update src/codeocean/error.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/codeocean/error.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/codeocean/error.py b/src/codeocean/error.py index a466927..0b66efc 100644 --- a/src/codeocean/error.py +++ b/src/codeocean/error.py @@ -4,7 +4,17 @@ class Error(Exception): + """ + Represents an HTTP error with additional context extracted from the response. + Attributes: + status_code (int): The HTTP status code of the error response. + message (str): A message describing the error, extracted from the response body or defaulting to a generic message. + items (list or None): If the response body is a list, this attribute contains the list; otherwise, it is None. + + Args: + err (requests.HTTPError): The HTTP error object from which to extract details. + """ def __init__(self, err: requests.HTTPError): self.status_code = err.response.status_code self.message = "An error occurred." From 76ff231196f45bd03010f9e9d60e7c9578eb4146 Mon Sep 17 00:00:00 2001 From: Zvika Gart Date: Mon, 21 Jul 2025 10:57:49 +0100 Subject: [PATCH 4/7] fix --- src/codeocean/error.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/codeocean/error.py b/src/codeocean/error.py index 0b66efc..976fa89 100644 --- a/src/codeocean/error.py +++ b/src/codeocean/error.py @@ -9,7 +9,7 @@ class Error(Exception): Attributes: status_code (int): The HTTP status code of the error response. - message (str): A message describing the error, extracted from the response body or defaulting to a generic message. + message (str): A message describing the error, extracted from the response body. items (list or None): If the response body is a list, this attribute contains the list; otherwise, it is None. Args: From 73917c43d16514348f545a3b9d340062d3973a62 Mon Sep 17 00:00:00 2001 From: jake-valsamis Date: Fri, 25 Jul 2025 12:58:43 -0400 Subject: [PATCH 5/7] return items in error message --- src/codeocean/error.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/codeocean/error.py b/src/codeocean/error.py index 976fa89..2811508 100644 --- a/src/codeocean/error.py +++ b/src/codeocean/error.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import requests @@ -19,13 +20,27 @@ def __init__(self, err: requests.HTTPError): self.status_code = err.response.status_code self.message = "An error occurred." self.items = None + self.url = err.response.url try: body = err.response.json() if isinstance(body, dict): self.message = body.get("message", self.message) + for key, value in body.items(): + if key not in ("message", "code"): + self.items = {key: value} + break elif isinstance(body, list): self.items = body except Exception: # response wasn't JSON – fall back to text self.message = err.response.text + + super().__init__(self.message) + + def __str__(self): + base = f"{self.status_code} Error for URL {self.url}:\n{self.message}" + if self.items: + extra = "\n\nAdditional context:\n" + json.dumps(self.items, indent=2) + return base + extra + return base From 25d47ebbc9c40074292241128fa8b6a1040c0901 Mon Sep 17 00:00:00 2001 From: Zvika Gart Date: Tue, 5 Aug 2025 00:40:24 +0100 Subject: [PATCH 6/7] fix --- src/codeocean/error.py | 33 +++++++++------------ tests/test_error.py | 66 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 25 deletions(-) diff --git a/src/codeocean/error.py b/src/codeocean/error.py index 2811508..d6e5e36 100644 --- a/src/codeocean/error.py +++ b/src/codeocean/error.py @@ -9,38 +9,33 @@ class Error(Exception): Represents an HTTP error with additional context extracted from the response. Attributes: + http_err (requests.HTTPError): The HTTP error object. status_code (int): The HTTP status code of the error response. message (str): A message describing the error, extracted from the response body. - items (list or None): If the response body is a list, this attribute contains the list; otherwise, it is None. + data (Any): If the response body is json, this attribute contains the json object; otherwise, it is None. Args: - err (requests.HTTPError): The HTTP error object from which to extract details. + err (requests.HTTPError): The HTTP error object. """ def __init__(self, err: requests.HTTPError): + self.http_err = err self.status_code = err.response.status_code self.message = "An error occurred." - self.items = None - self.url = err.response.url + self.data = None try: - body = err.response.json() - if isinstance(body, dict): - self.message = body.get("message", self.message) - for key, value in body.items(): - if key not in ("message", "code"): - self.items = {key: value} - break - elif isinstance(body, list): - self.items = body + self.data = err.response.json() + if isinstance(self.data, dict): + self.message = self.data.get("message", self.message) except Exception: # response wasn't JSON – fall back to text self.message = err.response.text super().__init__(self.message) - def __str__(self): - base = f"{self.status_code} Error for URL {self.url}:\n{self.message}" - if self.items: - extra = "\n\nAdditional context:\n" + json.dumps(self.items, indent=2) - return base + extra - return base + def __str__(self) -> str: + msg = str(self.http_err) + msg += f"\n\nMessage: {self.message}" + if self.data: + msg += "\n\nData:\n" + json.dumps(self.data, indent=2) + return msg diff --git a/tests/test_error.py b/tests/test_error.py index cd8224a..41c259c 100644 --- a/tests/test_error.py +++ b/tests/test_error.py @@ -12,12 +12,12 @@ def test_error_is_exception_subclass(self): """Test that Error is a subclass of Exception.""" self.assertTrue(issubclass(Error, Exception)) - def test_error_with_json_dict_message(self): + def test_error_with_json_dict(self): """Test Error creation with JSON dict response containing message.""" # Create mock HTTPError and response mock_response = Mock() mock_response.status_code = 400 - mock_response.json.return_value = {"message": "Custom error message"} + mock_response.json.return_value = {"message": "Custom error message", "datasets": [{"id":"123", "name":"tv"}]} mock_http_error = Mock(spec=requests.HTTPError) mock_http_error.response = mock_response @@ -28,7 +28,7 @@ def test_error_with_json_dict_message(self): # Verify attributes self.assertEqual(error.status_code, 400) self.assertEqual(error.message, "Custom error message") - self.assertIsNone(error.items) + self.assertEqual(error.data, {"message": "Custom error message", "datasets": [{"id":"123", "name":"tv"}]}) def test_error_with_json_dict_no_message(self): """Test Error creation with JSON dict response without message field.""" @@ -46,7 +46,7 @@ def test_error_with_json_dict_no_message(self): # Verify attributes self.assertEqual(error.status_code, 500) self.assertEqual(error.message, "An error occurred.") - self.assertIsNone(error.items) + self.assertEqual(error.data, {"error": "some other field"}) def test_error_with_json_list(self): """Test Error creation with JSON list response.""" @@ -64,7 +64,7 @@ def test_error_with_json_list(self): # Verify attributes self.assertEqual(error.status_code, 403) self.assertEqual(error.message, "An error occurred.") - self.assertEqual(error.items, [{"field": "error1"}, {"field": "error2"}]) + self.assertEqual(error.data, [{"field": "error1"}, {"field": "error2"}]) def test_error_with_non_json_response(self): """Test Error creation when response is not JSON.""" @@ -83,4 +83,58 @@ def test_error_with_non_json_response(self): # Verify attributes self.assertEqual(error.status_code, 404) self.assertEqual(error.message, "Page not found") - self.assertIsNone(error.items) + self.assertIsNone(error.data) + + def test_error_str_method_with_data(self): + """Test Error __str__ method when data is present.""" + # Create mock HTTPError and response + mock_response = Mock() + mock_response.status_code = 400 + mock_response.json.return_value = {"message": "Validation failed", "errors": ["field1", "field2"]} + + mock_http_error = Mock(spec=requests.HTTPError) + mock_http_error.response = mock_response + mock_http_error.__str__ = Mock(return_value="400 Client Error: Bad Request for url: http://example.com") + + # Create Error instance + error = Error(mock_http_error) + + # Test __str__ method + error_str = str(error) + + # Verify the string contains expected components + self.assertEqual(error_str, """400 Client Error: Bad Request for url: http://example.com + +Message: Validation failed + +Data: +{ + "message": "Validation failed", + "errors": [ + "field1", + "field2" + ] +}""") + + def test_error_str_method_without_data(self): + """Test Error __str__ method when data is None.""" + # Create mock HTTPError and response + mock_response = Mock() + mock_response.status_code = 404 + mock_response.json.side_effect = Exception("Not JSON") + mock_response.text = "Page not found" + + mock_http_error = Mock(spec=requests.HTTPError) + mock_http_error.response = mock_response + mock_http_error.__str__ = Mock(return_value="404 Client Error: Not Found for url: http://example.com") + + # Create Error instance + error = Error(mock_http_error) + + # Test __str__ method + error_str = str(error) + + # Verify the string contains expected components + self.assertEqual(error_str, """404 Client Error: Not Found for url: http://example.com + +Message: Page not found""") From 200e0f22df9703e2d4f7b6826dd3e19748ee7317 Mon Sep 17 00:00:00 2001 From: Zvika Gart Date: Tue, 5 Aug 2025 00:42:09 +0100 Subject: [PATCH 7/7] fix --- tests/test_error.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_error.py b/tests/test_error.py index 41c259c..b1a06de 100644 --- a/tests/test_error.py +++ b/tests/test_error.py @@ -17,7 +17,7 @@ def test_error_with_json_dict(self): # Create mock HTTPError and response mock_response = Mock() mock_response.status_code = 400 - mock_response.json.return_value = {"message": "Custom error message", "datasets": [{"id":"123", "name":"tv"}]} + mock_response.json.return_value = {"message": "Custom error message", "datasets": [{"id": "123", "name": "tv"}]} mock_http_error = Mock(spec=requests.HTTPError) mock_http_error.response = mock_response @@ -28,7 +28,7 @@ def test_error_with_json_dict(self): # Verify attributes self.assertEqual(error.status_code, 400) self.assertEqual(error.message, "Custom error message") - self.assertEqual(error.data, {"message": "Custom error message", "datasets": [{"id":"123", "name":"tv"}]}) + self.assertEqual(error.data, {"message": "Custom error message", "datasets": [{"id": "123", "name": "tv"}]}) def test_error_with_json_dict_no_message(self): """Test Error creation with JSON dict response without message field.""" @@ -101,7 +101,7 @@ def test_error_str_method_with_data(self): # Test __str__ method error_str = str(error) - + # Verify the string contains expected components self.assertEqual(error_str, """400 Client Error: Bad Request for url: http://example.com @@ -133,7 +133,7 @@ def test_error_str_method_without_data(self): # Test __str__ method error_str = str(error) - + # Verify the string contains expected components self.assertEqual(error_str, """404 Client Error: Not Found for url: http://example.com