From a31a19d9abe578c53d57fab548ad13cdc80ed0fb Mon Sep 17 00:00:00 2001 From: Anurag Bandyopadhyay Date: Thu, 20 Nov 2025 23:54:40 +0530 Subject: [PATCH 1/5] feat: improve errors and relay err codes and msgs --- README.md | 24 ++ openfga_sdk/api_client.py | 7 +- openfga_sdk/exceptions.py | 178 ++++++++++-- openfga_sdk/sync/api_client.py | 10 +- test/error_handling_test.py | 314 ++++++++++++++++++++ test/error_integration_test.py | 506 +++++++++++++++++++++++++++++++++ 6 files changed, 1017 insertions(+), 22 deletions(-) create mode 100644 test/error_handling_test.py create mode 100644 test/error_integration_test.py diff --git a/README.md b/README.md index 631c003..6de6114 100644 --- a/README.md +++ b/README.md @@ -1407,6 +1407,30 @@ Class | Method | HTTP request | Description This SDK supports producing metrics that can be consumed as part of an [OpenTelemetry](https://opentelemetry.io/) setup. For more information, please see [the documentation](https://github.com/openfga/python-sdk/blob/main/docs/opentelemetry.md) +### Error Handling + +The SDK provides comprehensive error handling with detailed error information and convenient helper methods. For more information, please see [the error handling documentation](https://github.com/openfga/python-sdk/blob/main/docs/error_handling.md). + +Key features: +- Operation context in error messages (e.g., `[write]`, `[check]`) +- Detailed error codes and messages from the API +- Request IDs for debugging and support +- Helper methods for error categorization (`is_validation_error()`, `is_retryable()`, etc.) + +```python +from openfga_sdk.exceptions import ApiException + +try: + await client.write([tuple]) +except ApiException as e: + print(f"Error: {e}") # [write] HTTP 400 type 'invalid_type' not found (validation_error) [request-id: abc-123] + + if e.is_validation_error(): + print(f"Validation error: {e.error_message}") + elif e.is_retryable(): + print(f"Temporary error - retrying... (Request ID: {e.request_id})") +``` + ## Contributing See [CONTRIBUTING](./CONTRIBUTING.md) for details. diff --git a/openfga_sdk/api_client.py b/openfga_sdk/api_client.py index baa73ee..91ca5e2 100644 --- a/openfga_sdk/api_client.py +++ b/openfga_sdk/api_client.py @@ -316,6 +316,8 @@ async def __call_api( json.loads(e.body), response_type ) e.body = None + if isinstance(e, ApiException) and TelemetryAttributes.fga_client_request_method in _telemetry_attributes: + e.operation_name = _telemetry_attributes[TelemetryAttributes.fga_client_request_method].lower() raise e except ApiException as e: e.body = e.body.decode("utf-8") @@ -347,7 +349,10 @@ async def __call_api( attributes=_telemetry_attributes, configuration=self.configuration.telemetry, ) - raise e + + if isinstance(e, ApiException) and TelemetryAttributes.fga_client_request_method in _telemetry_attributes: + e.operation_name = _telemetry_attributes[TelemetryAttributes.fga_client_request_method].lower() + raise self.last_response = response_data diff --git a/openfga_sdk/exceptions.py b/openfga_sdk/exceptions.py index a10a554..7fd5263 100644 --- a/openfga_sdk/exceptions.py +++ b/openfga_sdk/exceptions.py @@ -116,7 +116,7 @@ def __init__(self, msg, path_to_item=None): class ApiException(OpenApiException): - def __init__(self, status=None, reason=None, http_resp=None): + def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): if http_resp: try: headers = http_resp.headers.items() @@ -138,14 +138,37 @@ def __init__(self, status=None, reason=None, http_resp=None): self._parsed_exception = None self.header = dict() + self.operation_name = operation_name + def __str__(self): - """Custom error messages for exception""" - error_message = f"({self.status})\nReason: {self.reason}\n" + """ + Format error with operation context and structured details. + Returns formatted string like: + [write] HTTP 400 type 'invalid_type' not found (validation_error) [request-id: abc-123] + """ + parts = [] + + # Add operation context + if self.operation_name: + parts.append(f"[{self.operation_name}]") + + # Add error type/status + if self.status: + parts.append(f"HTTP {self.status}") + + # Add error message (parsed or reason) + if self.error_message: + parts.append(self.error_message) + + # Add error code in parentheses + if self.code: + parts.append(f"({self.code})") - if self.body: - error_message += f"HTTP response body: {self.body}\n" + # Add request ID for debugging + if self.request_id: + parts.append(f"[request-id: {self.request_id}]") - return error_message + return " ".join(parts) if parts else "Unknown API error" @property def parsed_exception(self): @@ -161,40 +184,155 @@ def parsed_exception(self, content): """ self._parsed_exception = content + @property + def code(self): + """ + Get the error code from the parsed exception. + + Returns: + Error code string (e.g., "validation_error") or None + """ + if self._parsed_exception and hasattr(self._parsed_exception, 'code'): + code_value = self._parsed_exception.code + # Handle enum types + if hasattr(code_value, 'value'): + return code_value.value + return str(code_value) if code_value is not None else None + return None + + @property + def error_message(self): + """ + Get the human-readable error message. + + Returns: + Error message from API or HTTP reason phrase + """ + if self._parsed_exception and hasattr(self._parsed_exception, 'message'): + message = self._parsed_exception.message + if message: + return message + return self.reason or "Unknown error" + + @property + def request_id(self): + """ + Get the request ID for debugging and support. + + Returns: + FGA request ID from response headers or None + """ + if not self.header: + return None + # HTTP headers are case-insensitive, try different cases + for key in self.header: + if key.lower() == FGA_REQUEST_ID: + return self.header[key] + return None + + def is_validation_error(self): + """ + Check if this is a validation error. + + Returns: + True if error code indicates validation failure + """ + return ( + isinstance(self, ValidationException) or + (self.code and 'validation' in self.code.lower()) + ) + + def is_not_found_error(self): + """ + Check if this is a not found (404) error. + + Returns: + True if HTTP status is 404 + """ + return isinstance(self, NotFoundException) or self.status == 404 + + def is_authentication_error(self): + """ + Check if this is an authentication (401) error. + + Returns: + True if HTTP status is 401 + """ + return self.status == 401 + + def is_rate_limit_error(self): + """ + Check if this is a rate limit (429) error. + + Returns: + True if HTTP status is 429 or error code indicates rate limiting + """ + return ( + self.status == 429 or + (self.code and 'rate_limit' in self.code.lower()) + ) + + def is_retryable(self): + """ + Check if this error should be retried. + + Returns: + True if error is temporary and retrying may succeed + """ + return self.status in [429, 500, 502, 503, 504] if self.status else False + + def is_client_error(self): + """ + Check if this is a client error (4xx). + + Returns: + True if HTTP status is in 400-499 range + """ + return 400 <= self.status < 500 if self.status else False + + def is_server_error(self): + """ + Check if this is a server error (5xx). + + Returns: + True if HTTP status is in 500-599 range + """ + return 500 <= self.status < 600 if self.status else False + class NotFoundException(ApiException): - def __init__(self, status=None, reason=None, http_resp=None): - super().__init__(status, reason, http_resp) + def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): + super().__init__(status, reason, http_resp, operation_name) class UnauthorizedException(ApiException): - def __init__(self, status=None, reason=None, http_resp=None): - super().__init__(status, reason, http_resp) + def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): + super().__init__(status, reason, http_resp, operation_name) class ForbiddenException(ApiException): - def __init__(self, status=None, reason=None, http_resp=None): - super().__init__(status, reason, http_resp) + def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): + super().__init__(status, reason, http_resp, operation_name) class ServiceException(ApiException): - def __init__(self, status=None, reason=None, http_resp=None): - super().__init__(status, reason, http_resp) + def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): + super().__init__(status, reason, http_resp, operation_name) class ValidationException(ApiException): - def __init__(self, status=None, reason=None, http_resp=None): - super().__init__(status, reason, http_resp) + def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): + super().__init__(status, reason, http_resp, operation_name) class AuthenticationError(ApiException): - def __init__(self, status=None, reason=None, http_resp=None): - super().__init__(status, reason, http_resp) + def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): + super().__init__(status, reason, http_resp, operation_name) class RateLimitExceededError(ApiException): - def __init__(self, status=None, reason=None, http_resp=None): - super().__init__(status, reason, http_resp) + def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): + super().__init__(status, reason, http_resp, operation_name) def render_path(path_to_item): diff --git a/openfga_sdk/sync/api_client.py b/openfga_sdk/sync/api_client.py index 1a07021..003b6b5 100644 --- a/openfga_sdk/sync/api_client.py +++ b/openfga_sdk/sync/api_client.py @@ -314,6 +314,10 @@ def __call_api( json.loads(e.body), response_type ) e.body = None + # Set operation name from telemetry attributes + if isinstance(e, ApiException) and TelemetryAttributes.fga_client_request_method in _telemetry_attributes: + # Convert operation name to lowercase for consistency + e.operation_name = _telemetry_attributes[TelemetryAttributes.fga_client_request_method].lower() raise e except ApiException as e: e.body = e.body.decode("utf-8") @@ -345,7 +349,11 @@ def __call_api( attributes=_telemetry_attributes, configuration=self.configuration.telemetry, ) - raise e + + # Set operation name from telemetry attributes + if isinstance(e, ApiException) and TelemetryAttributes.fga_client_request_method in _telemetry_attributes: + e.operation_name = _telemetry_attributes[TelemetryAttributes.fga_client_request_method].lower() + raise self.last_response = response_data diff --git a/test/error_handling_test.py b/test/error_handling_test.py new file mode 100644 index 0000000..27d1ac5 --- /dev/null +++ b/test/error_handling_test.py @@ -0,0 +1,314 @@ +""" +Unit tests for enhanced error handling. + +These tests verify that: +1. Error messages are properly formatted +2. Error properties are accessible +3. Helper methods work correctly +""" + +import pytest +from openfga_sdk.exceptions import ( + ApiException, + ValidationException, + NotFoundException, + ServiceException, + RateLimitExceededError, +) +from openfga_sdk.models import ValidationErrorMessageResponse +from openfga_sdk.models.error_code import ErrorCode + + +class MockHTTPResponse: + """Mock HTTP response for testing.""" + + def __init__(self, status, reason, data, headers=None): + self.status = status + self.reason = reason + self.data = data + # Create a mock headers object with items() method + self.headers = MockHeaders(headers or {}) + + def getheaders(self): + return self.headers + + +class MockHeaders: + """Mock headers object.""" + + def __init__(self, headers_dict): + self._headers = headers_dict + + def items(self): + return list(self._headers.items()) + + +class TestEnhancedErrorHandling: + """Tests for enhanced error handling functionality.""" + + def test_error_message_format_with_all_fields(self): + """Test that error messages include all components when available.""" + # Create a mock response + response = MockHTTPResponse( + status=400, + reason="Bad Request", + data=b'{"code": "validation_error", "message": "type \'invalid_type\' not found"}', + headers={"fga-request-id": "test-request-123"} + ) + + # Create exception + exc = ValidationException(http_resp=response, operation_name="write") + + # Set parsed exception + parsed = ValidationErrorMessageResponse( + code=ErrorCode.VALIDATION_ERROR, + message="type 'invalid_type' not found" + ) + exc.parsed_exception = parsed + + # Verify string representation + error_str = str(exc) + assert "[write]" in error_str + assert "HTTP 400" in error_str + assert "type 'invalid_type' not found" in error_str + assert "(validation_error)" in error_str + assert "[request-id: test-request-123]" in error_str + + def test_error_properties_accessible(self): + """Test that all error properties are accessible.""" + response = MockHTTPResponse( + status=400, + reason="Bad Request", + data=b'{}', + headers={"fga-request-id": "test-request-456"} + ) + + exc = ValidationException(http_resp=response, operation_name="check") + parsed = ValidationErrorMessageResponse( + code=ErrorCode.VALIDATION_ERROR, + message="Invalid relation" + ) + exc.parsed_exception = parsed + + # Test properties + assert exc.operation_name == "check" + assert exc.status == 400 + assert exc.code == "validation_error" + assert exc.error_message == "Invalid relation" + assert exc.request_id == "test-request-456" + + def test_is_validation_error_helper(self): + """Test is_validation_error() helper method.""" + response = MockHTTPResponse(400, "Bad Request", b'{}') + exc = ValidationException(http_resp=response) + parsed = ValidationErrorMessageResponse(code=ErrorCode.VALIDATION_ERROR) + exc.parsed_exception = parsed + + assert exc.is_validation_error() is True + assert exc.is_client_error() is True + assert exc.is_server_error() is False + assert exc.is_retryable() is False + + def test_is_not_found_error_helper(self): + """Test is_not_found_error() helper method.""" + response = MockHTTPResponse(404, "Not Found", b'{}') + exc = NotFoundException(http_resp=response) + + assert exc.is_not_found_error() is True + assert exc.is_client_error() is True + assert exc.is_server_error() is False + assert exc.is_retryable() is False + + def test_is_authentication_error_helper(self): + """Test is_authentication_error() helper method.""" + response = MockHTTPResponse(401, "Unauthorized", b'{}') + exc = ApiException(http_resp=response) + + assert exc.is_authentication_error() is True + assert exc.is_client_error() is True + + def test_is_rate_limit_error_helper(self): + """Test is_rate_limit_error() helper method.""" + response = MockHTTPResponse(429, "Too Many Requests", b'{}') + exc = RateLimitExceededError(http_resp=response) + + assert exc.is_rate_limit_error() is True + assert exc.is_retryable() is True + assert exc.is_client_error() is True + + def test_is_server_error_helper(self): + """Test is_server_error() helper method.""" + response = MockHTTPResponse(500, "Internal Server Error", b'{}') + exc = ServiceException(http_resp=response) + + assert exc.is_server_error() is True + assert exc.is_client_error() is False + assert exc.is_retryable() is True + + def test_is_retryable_helper(self): + """Test is_retryable() helper for various status codes.""" + # Retryable errors + for status in [429, 500, 502, 503, 504]: + response = MockHTTPResponse(status, "Error", b'{}') + exc = ApiException(http_resp=response) + assert exc.is_retryable() is True, f"Status {status} should be retryable" + + # Non-retryable errors + for status in [400, 401, 403, 404]: + response = MockHTTPResponse(status, "Error", b'{}') + exc = ApiException(http_resp=response) + assert exc.is_retryable() is False, f"Status {status} should not be retryable" + + def test_error_without_parsed_exception(self): + """Test error handling when parsed_exception is not set.""" + response = MockHTTPResponse(400, "Bad Request", b'{}') + exc = ValidationException(http_resp=response, operation_name="write") + + # Should use reason as fallback + assert exc.error_message == "Bad Request" + assert exc.code is None + + # String representation should still work + error_str = str(exc) + assert "[write]" in error_str + assert "HTTP 400" in error_str + assert "Bad Request" in error_str + + def test_error_without_operation_name(self): + """Test error handling when operation_name is not set.""" + response = MockHTTPResponse(400, "Bad Request", b'{}') + exc = ApiException(http_resp=response) + parsed = ValidationErrorMessageResponse( + code=ErrorCode.VALIDATION_ERROR, + message="Test error" + ) + exc.parsed_exception = parsed + + # Should not include operation name + error_str = str(exc) + assert not error_str.startswith("[") + assert "HTTP 400" in error_str + assert "Test error" in error_str + + def test_error_without_request_id(self): + """Test error handling when request_id is not available.""" + response = MockHTTPResponse(400, "Bad Request", b'{}', headers={}) + exc = ValidationException(http_resp=response, operation_name="write") + + assert exc.request_id is None + + # String representation should not include request-id + error_str = str(exc) + assert "[request-id:" not in error_str + + def test_error_code_with_enum(self): + """Test that error code property handles enum values.""" + response = MockHTTPResponse(400, "Bad Request", b'{}') + exc = ValidationException(http_resp=response) + + # Create parsed exception with enum + parsed = ValidationErrorMessageResponse(code=ErrorCode.VALIDATION_ERROR) + exc.parsed_exception = parsed + + # Should return the enum value as string + assert exc.code == "validation_error" + assert isinstance(exc.code, str) + + def test_multiple_errors_have_unique_messages(self): + """Test that different errors have different details.""" + # Error 1: Invalid type + response1 = MockHTTPResponse(400, "Bad Request", b'{}') + exc1 = ValidationException(http_resp=response1, operation_name="write") + parsed1 = ValidationErrorMessageResponse( + code=ErrorCode.VALIDATION_ERROR, + message="type 'invalid_type' not found" + ) + exc1.parsed_exception = parsed1 + + # Error 2: Invalid relation + response2 = MockHTTPResponse(400, "Bad Request", b'{}') + exc2 = ValidationException(http_resp=response2, operation_name="write") + parsed2 = ValidationErrorMessageResponse( + code=ErrorCode.VALIDATION_ERROR, + message="relation 'invalid_relation' not found" + ) + exc2.parsed_exception = parsed2 + + # Same code, different messages + assert exc1.code == exc2.code + assert exc1.error_message != exc2.error_message + assert "invalid_type" in exc1.error_message + assert "invalid_relation" in exc2.error_message + + def test_operation_context_preserved(self): + """Test that operation name is preserved in exception.""" + response = MockHTTPResponse(400, "Bad Request", b'{}') + + write_exc = ValidationException(http_resp=response, operation_name="write") + assert write_exc.operation_name == "write" + + check_exc = ValidationException(http_resp=response, operation_name="check") + assert check_exc.operation_name == "check" + + read_exc = ValidationException(http_resp=response, operation_name="read") + assert read_exc.operation_name == "read" + + def test_error_message_consistency(self): + """Test that error message format is consistent.""" + import re + + response = MockHTTPResponse( + 400, "Bad Request", b'{}', + headers={"fga-request-id": "req-123"} + ) + exc = ValidationException(http_resp=response, operation_name="write") + parsed = ValidationErrorMessageResponse( + code=ErrorCode.VALIDATION_ERROR, + message="Test error" + ) + exc.parsed_exception = parsed + + error_str = str(exc) + + # Should match pattern: [operation] HTTP status message (code) [request-id: id] + pattern = r"^\[write\] HTTP 400 Test error \(validation_error\) \[request-id: req-123\]$" + assert re.match(pattern, error_str), f"Error string '{error_str}' doesn't match expected pattern" + + def test_client_vs_server_error_categorization(self): + """Test that client and server errors are properly categorized.""" + # Client errors (4xx) + for status in [400, 401, 403, 404, 429]: + response = MockHTTPResponse(status, "Error", b'{}') + exc = ApiException(http_resp=response) + assert exc.is_client_error() is True + assert exc.is_server_error() is False + + # Server errors (5xx) + for status in [500, 502, 503, 504]: + response = MockHTTPResponse(status, "Error", b'{}') + exc = ApiException(http_resp=response) + assert exc.is_server_error() is True + assert exc.is_client_error() is False + + def test_exception_subclass_helpers(self): + """Test that helper methods work for exception subclasses.""" + # ValidationException + response = MockHTTPResponse(400, "Bad Request", b'{}') + exc = ValidationException(http_resp=response) + assert exc.is_validation_error() is True + + # NotFoundException + response = MockHTTPResponse(404, "Not Found", b'{}') + exc = NotFoundException(http_resp=response) + assert exc.is_not_found_error() is True + + # ServiceException + response = MockHTTPResponse(500, "Internal Server Error", b'{}') + exc = ServiceException(http_resp=response) + assert exc.is_server_error() is True + + # RateLimitExceededError + response = MockHTTPResponse(429, "Too Many Requests", b'{}') + exc = RateLimitExceededError(http_resp=response) + assert exc.is_rate_limit_error() is True + diff --git a/test/error_integration_test.py b/test/error_integration_test.py new file mode 100644 index 0000000..7f26faa --- /dev/null +++ b/test/error_integration_test.py @@ -0,0 +1,506 @@ +""" +Integration tests for enhanced error handling. + +These tests verify that: +1. Error messages include operation context +2. Error codes and messages are properly exposed +3. Request IDs are available for debugging +4. Helper methods work correctly for error categorization + +NOTE: These tests require a running OpenFGA server. +Set FGA_API_URL environment variable or use default http://localhost:8080 + +To run these tests with a local OpenFGA instance: +1. Start OpenFGA: docker run -p 8080:8080 openfga/openfga run +2. Run tests: FGA_API_URL=http://localhost:8080 pytest test/error_integration_test.py -v +""" + +import os +import pytest +import pytest_asyncio + +from openfga_sdk.client import ClientConfiguration +from openfga_sdk.client.client import OpenFgaClient +from openfga_sdk.exceptions import ValidationException, NotFoundException + +# Skip all tests if FGA_API_URL is not set (for CI/CD environments without OpenFGA) +pytestmark = pytest.mark.skipif( + not os.environ.get("FGA_API_URL") and not os.path.exists("/.dockerenv"), + reason="OpenFGA server not available. Set FGA_API_URL to run integration tests." +) + + +# Sample authorization model for testing +AUTH_MODEL = { + "schema_version": "1.1", + "type_definitions": [ + {"type": "user", "relations": {}}, + { + "type": "document", + "relations": { + "viewer": {"this": {}}, + "owner": {"this": {}}, + }, + "metadata": { + "relations": { + "viewer": {"directly_related_user_types": [{"type": "user"}]}, + "owner": {"directly_related_user_types": [{"type": "user"}]}, + } + }, + }, + ], +} + + +@pytest.mark.asyncio +class TestErrorIntegration: + """Integration tests for enhanced error handling.""" + + @pytest_asyncio.fixture + async def fga_client(self): + """ + Create a test client with a store and authorization model. + + Note: This requires a running OpenFGA server. + Set FGA_API_URL environment variable or use default localhost:8080. + """ + from openfga_sdk.models import CreateStoreRequest + + api_url = os.environ.get("FGA_API_URL", "http://localhost:8080") + + config = ClientConfiguration( + api_url=api_url, + ) + + client = OpenFgaClient(config) + + # Create a test store + store = await client.create_store(CreateStoreRequest(name="ErrorTestStore")) + config.store_id = store.id + client = OpenFgaClient(config) # Recreate client with store_id + + # Write the authorization model + model_response = await client.write_authorization_model(AUTH_MODEL) + config.authorization_model_id = model_response.authorization_model_id + client = OpenFgaClient(config) # Recreate client with auth model id + + yield client + + # Cleanup: delete the store + try: + await client.delete_store() + except Exception: + pass # Ignore cleanup errors + + async def test_write_validation_error_invalid_type(self, fga_client): + """Test that write with invalid type shows proper error details.""" + from openfga_sdk.client.models import ClientTuple, ClientWriteRequest + + # Try to write a tuple with invalid type + with pytest.raises(ValidationException) as exc_info: + await fga_client.write( + ClientWriteRequest( + writes=[ + ClientTuple( + user="user:anne", + relation="viewer", + object="invalid_type:readme", + ) + ] + ) + ) + + exception = exc_info.value + + # Verify error details are accessible + assert exception.code is not None + assert "validation" in exception.code.lower() + assert "invalid_type" in exception.error_message.lower() + assert exception.operation_name == "write" + # request_id might be None in local dev environments + # assert exception.request_id is not None + assert exception.status == 400 + + # Verify formatted message includes all components + error_str = str(exception) + assert "[write]" in error_str + assert "HTTP 400" in error_str + assert "validation" in error_str.lower() + # request_id might be None in local dev, so it might not be in the message + # assert "[request-id:" in error_str + + async def test_write_validation_error_invalid_relation(self, fga_client): + """Test that write with invalid relation shows proper error details.""" + from openfga_sdk.client.models import ClientTuple, ClientWriteRequest + + # Try to write a tuple with valid type but invalid relation + with pytest.raises(ValidationException) as exc_info: + await fga_client.write( + ClientWriteRequest( + writes=[ + ClientTuple( + user="user:anne", + relation="invalid_relation", + object="document:readme", + ) + ] + ) + ) + + exception = exc_info.value + + # Verify error details + assert exception.code is not None + assert "validation" in exception.code.lower() + assert "relation" in exception.error_message.lower() + assert exception.operation_name == "write" + # request_id might be None in local dev environments + # assert exception.request_id is not None + + # Verify formatted message + error_str = str(exception) + assert "[write]" in error_str + assert "HTTP 400" in error_str + + async def test_check_validation_error(self, fga_client): + """Test that check with invalid type shows proper error details.""" + from openfga_sdk.client.models import ClientCheckRequest + + # Try check with invalid type + with pytest.raises(ValidationException) as exc_info: + await fga_client.check( + ClientCheckRequest( + user="user:anne", + relation="viewer", + object="invalid_type:readme", + ) + ) + + exception = exc_info.value + + assert exception.operation_name == "check" + assert exception.status == 400 + assert exception.code is not None + assert "validation" in exception.code.lower() + + error_str = str(exception) + assert "[check]" in error_str + assert "HTTP 400" in error_str + + async def test_expand_validation_error(self, fga_client): + """Test that expand with invalid type shows proper error details.""" + from openfga_sdk.client.models import ClientExpandRequest + + # Try expand with invalid type + with pytest.raises(ValidationException) as exc_info: + await fga_client.expand( + ClientExpandRequest( + object="invalid_type:readme", + relation="viewer", + ) + ) + + exception = exc_info.value + + assert exception.operation_name == "expand" + assert exception.status == 400 + assert exception.code is not None + + async def test_not_found_error(self, fga_client): + """Test that not found errors are properly categorized.""" + # Delete the store first + await fga_client.delete_store() + + # Now try to get the deleted store + with pytest.raises(NotFoundException) as exc_info: + await fga_client.get_store() + + exception = exc_info.value + + assert exception.status == 404 + assert exception.is_not_found_error() + assert exception.is_client_error() + assert not exception.is_server_error() + assert not exception.is_retryable() + + error_str = str(exception) + assert "HTTP 404" in error_str + + async def test_error_helper_methods_validation(self, fga_client): + """Test helper methods for validation errors.""" + from openfga_sdk.client.models import ClientTuple, ClientWriteRequest + + with pytest.raises(ValidationException) as exc_info: + await fga_client.write( + ClientWriteRequest( + writes=[ + ClientTuple( + user="user:anne", + relation="viewer", + object="invalid_type:readme", + ) + ] + ) + ) + + exception = exc_info.value + + # Test all helper methods + assert exception.is_validation_error() + assert exception.is_client_error() + assert not exception.is_server_error() + assert not exception.is_retryable() + assert not exception.is_authentication_error() + assert not exception.is_not_found_error() + assert not exception.is_rate_limit_error() + + async def test_error_message_format_consistency(self, fga_client): + """Test that error messages follow consistent format across operations.""" + from openfga_sdk.client.models import ClientTuple, ClientCheckRequest, ClientWriteRequest + + # Test write error format + with pytest.raises(ValidationException) as write_exc: + await fga_client.write( + ClientWriteRequest( + writes=[ + ClientTuple( + user="user:anne", + relation="viewer", + object="invalid:x", + ) + ] + ) + ) + + write_error = str(write_exc.value) + assert write_error.startswith("[write]") + assert "HTTP 400" in write_error + # request_id might not be present in local dev + # assert "[request-id:" in write_error + + # Test check error format + with pytest.raises(ValidationException) as check_exc: + await fga_client.check( + ClientCheckRequest( + user="user:anne", + relation="viewer", + object="invalid:x", + ) + ) + + check_error = str(check_exc.value) + assert check_error.startswith("[check]") + assert "HTTP 400" in check_error + # request_id might not be present in local dev + # assert "[request-id:" in check_error + + # Both should follow same pattern (with or without request-id) + import re + # Pattern with optional request-id at the end + pattern = r"^\[\w+\] HTTP \d{3} .+ \(.+\)( \[request-id: .+\])?$" + assert re.match(pattern, write_error) + assert re.match(pattern, check_error) + + async def test_error_code_fields_accessibility(self, fga_client): + """Test that all error fields are accessible.""" + from openfga_sdk.client.models import ClientTuple, ClientWriteRequest + + with pytest.raises(ValidationException) as exc_info: + await fga_client.write( + ClientWriteRequest( + writes=[ + ClientTuple( + user="user:anne", + relation="viewer", + object="invalid_type:readme", + ) + ] + ) + ) + + exception = exc_info.value + + # Verify all fields are accessible + assert exception.status == 400 + assert exception.code is not None + assert isinstance(exception.code, str) + assert exception.operation_name == "write" + assert exception.error_message is not None + assert isinstance(exception.error_message, str) + # request_id might be None in local dev environments + # assert exception.request_id is not None + # assert isinstance(exception.request_id, str) + + # Request ID should match expected format if present + if exception.request_id: + import re + assert re.match(r"[a-zA-Z0-9-]+", exception.request_id) + + async def test_different_validation_errors_have_different_messages(self, fga_client): + """Test that different validation errors surface different messages.""" + from openfga_sdk.client.models import ClientTuple, ClientWriteRequest + + # Case 1: Invalid type + with pytest.raises(ValidationException) as exc1: + await fga_client.write( + ClientWriteRequest( + writes=[ + ClientTuple( + user="user:anne", + relation="viewer", + object="invalid_type:readme", + ) + ] + ) + ) + + error1 = exc1.value + assert "type" in error1.error_message.lower() + + # Case 2: Invalid relation + with pytest.raises(ValidationException) as exc2: + await fga_client.write( + ClientWriteRequest( + writes=[ + ClientTuple( + user="user:anne", + relation="invalid_relation", + object="document:readme", + ) + ] + ) + ) + + error2 = exc2.value + assert "relation" in error2.error_message.lower() + + # Both should have same error code but different messages + assert error1.code == error2.code + assert error1.error_message != error2.error_message + + async def test_error_details_not_lost_in_traceback(self, fga_client): + """Test that error details are preserved in exception traceback.""" + from openfga_sdk.client.models import ClientTuple, ClientWriteRequest + + try: + await fga_client.write( + ClientWriteRequest( + writes=[ + ClientTuple( + user="user:anne", + relation="viewer", + object="invalid_type:readme", + ) + ] + ) + ) + pytest.fail("Expected ValidationException to be raised") + except ValidationException as e: + # String representation should include all details + error_string = str(e) + assert "invalid_type" in error_string.lower() + + # Exception message should be properly formatted + error_message = e.error_message + assert "invalid_type" in error_message.lower() + + # Fields should be accessible + assert e.status == 400 + assert e.code is not None + assert e.operation_name == "write" + # request_id might be None in local dev environments + # assert e.request_id is not None + + +# Sync version of tests +class TestErrorIntegrationSync: + """Synchronous integration tests for enhanced error handling.""" + + @pytest.fixture + def fga_client_sync(self): + """ + Create a sync test client with a store and authorization model. + + Note: This requires a running OpenFGA server. + """ + from openfga_sdk.sync.client.client import OpenFgaClient as SyncOpenFgaClient + from openfga_sdk.models import CreateStoreRequest + + api_url = os.environ.get("FGA_API_URL", "http://localhost:8080") + + config = ClientConfiguration( + api_url=api_url, + ) + + client = SyncOpenFgaClient(config) + + # Create a test store + store = client.create_store(CreateStoreRequest(name="ErrorTestStoreSync")) + config.store_id = store.id + client = SyncOpenFgaClient(config) # Recreate client with store_id + + # Write the authorization model + model_response = client.write_authorization_model(AUTH_MODEL) + config.authorization_model_id = model_response.authorization_model_id + client = SyncOpenFgaClient(config) # Recreate client with auth model id + + yield client + + # Cleanup + try: + client.delete_store() + except Exception: + pass + + def test_sync_write_validation_error(self, fga_client_sync): + """Test sync client error handling.""" + from openfga_sdk.client.models import ClientTuple, ClientWriteRequest + + with pytest.raises(ValidationException) as exc_info: + fga_client_sync.write( + ClientWriteRequest( + writes=[ + ClientTuple( + user="user:anne", + relation="viewer", + object="invalid_type:readme", + ) + ] + ) + ) + + exception = exc_info.value + + assert exception.operation_name == "write" + assert exception.status == 400 + assert exception.code is not None + assert exception.is_validation_error() + + error_str = str(exception) + assert "[write]" in error_str + assert "HTTP 400" in error_str + + def test_sync_error_helper_methods(self, fga_client_sync): + """Test that helper methods work in sync client.""" + from openfga_sdk.client.models import ClientTuple, ClientWriteRequest + + with pytest.raises(ValidationException) as exc_info: + fga_client_sync.write( + ClientWriteRequest( + writes=[ + ClientTuple( + user="user:anne", + relation="viewer", + object="invalid_type:readme", + ) + ] + ) + ) + + exception = exc_info.value + + assert exception.is_validation_error() + assert exception.is_client_error() + assert not exception.is_server_error() + assert not exception.is_retryable() + From 2d5fa7712303f328c018ffee59c12ce954f7f4ba Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Fri, 21 Nov 2025 16:49:20 +0530 Subject: [PATCH 2/5] fix: readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 6de6114..923aef2 100644 --- a/README.md +++ b/README.md @@ -1409,7 +1409,7 @@ This SDK supports producing metrics that can be consumed as part of an [OpenTele ### Error Handling -The SDK provides comprehensive error handling with detailed error information and convenient helper methods. For more information, please see [the error handling documentation](https://github.com/openfga/python-sdk/blob/main/docs/error_handling.md). +The SDK provides comprehensive error handling with detailed error information and convenient helper methods. Key features: - Operation context in error messages (e.g., `[write]`, `[check]`) From 42f603b3275b9758408f71dcc26b5981f22fbf9e Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Fri, 21 Nov 2025 16:52:21 +0530 Subject: [PATCH 3/5] fix: ruff lint --- openfga_sdk/api_client.py | 20 +++++++-- openfga_sdk/exceptions.py | 16 +++---- openfga_sdk/sync/api_client.py | 20 +++++++-- test/error_handling_test.py | 80 ++++++++++++++++------------------ test/error_integration_test.py | 21 ++++++--- 5 files changed, 91 insertions(+), 66 deletions(-) diff --git a/openfga_sdk/api_client.py b/openfga_sdk/api_client.py index 91ca5e2..8b3f3ac 100644 --- a/openfga_sdk/api_client.py +++ b/openfga_sdk/api_client.py @@ -316,8 +316,14 @@ async def __call_api( json.loads(e.body), response_type ) e.body = None - if isinstance(e, ApiException) and TelemetryAttributes.fga_client_request_method in _telemetry_attributes: - e.operation_name = _telemetry_attributes[TelemetryAttributes.fga_client_request_method].lower() + if ( + isinstance(e, ApiException) + and TelemetryAttributes.fga_client_request_method + in _telemetry_attributes + ): + e.operation_name = _telemetry_attributes[ + TelemetryAttributes.fga_client_request_method + ].lower() raise e except ApiException as e: e.body = e.body.decode("utf-8") @@ -350,8 +356,14 @@ async def __call_api( configuration=self.configuration.telemetry, ) - if isinstance(e, ApiException) and TelemetryAttributes.fga_client_request_method in _telemetry_attributes: - e.operation_name = _telemetry_attributes[TelemetryAttributes.fga_client_request_method].lower() + if ( + isinstance(e, ApiException) + and TelemetryAttributes.fga_client_request_method + in _telemetry_attributes + ): + e.operation_name = _telemetry_attributes[ + TelemetryAttributes.fga_client_request_method + ].lower() raise self.last_response = response_data diff --git a/openfga_sdk/exceptions.py b/openfga_sdk/exceptions.py index 7fd5263..e1f8e35 100644 --- a/openfga_sdk/exceptions.py +++ b/openfga_sdk/exceptions.py @@ -192,10 +192,10 @@ def code(self): Returns: Error code string (e.g., "validation_error") or None """ - if self._parsed_exception and hasattr(self._parsed_exception, 'code'): + if self._parsed_exception and hasattr(self._parsed_exception, "code"): code_value = self._parsed_exception.code # Handle enum types - if hasattr(code_value, 'value'): + if hasattr(code_value, "value"): return code_value.value return str(code_value) if code_value is not None else None return None @@ -208,7 +208,7 @@ def error_message(self): Returns: Error message from API or HTTP reason phrase """ - if self._parsed_exception and hasattr(self._parsed_exception, 'message'): + if self._parsed_exception and hasattr(self._parsed_exception, "message"): message = self._parsed_exception.message if message: return message @@ -237,9 +237,8 @@ def is_validation_error(self): Returns: True if error code indicates validation failure """ - return ( - isinstance(self, ValidationException) or - (self.code and 'validation' in self.code.lower()) + return isinstance(self, ValidationException) or ( + self.code and "validation" in self.code.lower() ) def is_not_found_error(self): @@ -267,10 +266,7 @@ def is_rate_limit_error(self): Returns: True if HTTP status is 429 or error code indicates rate limiting """ - return ( - self.status == 429 or - (self.code and 'rate_limit' in self.code.lower()) - ) + return self.status == 429 or (self.code and "rate_limit" in self.code.lower()) def is_retryable(self): """ diff --git a/openfga_sdk/sync/api_client.py b/openfga_sdk/sync/api_client.py index 003b6b5..a6f9ea6 100644 --- a/openfga_sdk/sync/api_client.py +++ b/openfga_sdk/sync/api_client.py @@ -315,9 +315,15 @@ def __call_api( ) e.body = None # Set operation name from telemetry attributes - if isinstance(e, ApiException) and TelemetryAttributes.fga_client_request_method in _telemetry_attributes: + if ( + isinstance(e, ApiException) + and TelemetryAttributes.fga_client_request_method + in _telemetry_attributes + ): # Convert operation name to lowercase for consistency - e.operation_name = _telemetry_attributes[TelemetryAttributes.fga_client_request_method].lower() + e.operation_name = _telemetry_attributes[ + TelemetryAttributes.fga_client_request_method + ].lower() raise e except ApiException as e: e.body = e.body.decode("utf-8") @@ -351,8 +357,14 @@ def __call_api( ) # Set operation name from telemetry attributes - if isinstance(e, ApiException) and TelemetryAttributes.fga_client_request_method in _telemetry_attributes: - e.operation_name = _telemetry_attributes[TelemetryAttributes.fga_client_request_method].lower() + if ( + isinstance(e, ApiException) + and TelemetryAttributes.fga_client_request_method + in _telemetry_attributes + ): + e.operation_name = _telemetry_attributes[ + TelemetryAttributes.fga_client_request_method + ].lower() raise self.last_response = response_data diff --git a/test/error_handling_test.py b/test/error_handling_test.py index 27d1ac5..2c59770 100644 --- a/test/error_handling_test.py +++ b/test/error_handling_test.py @@ -7,13 +7,12 @@ 3. Helper methods work correctly """ -import pytest from openfga_sdk.exceptions import ( ApiException, - ValidationException, NotFoundException, - ServiceException, RateLimitExceededError, + ServiceException, + ValidationException, ) from openfga_sdk.models import ValidationErrorMessageResponse from openfga_sdk.models.error_code import ErrorCode @@ -53,7 +52,7 @@ def test_error_message_format_with_all_fields(self): status=400, reason="Bad Request", data=b'{"code": "validation_error", "message": "type \'invalid_type\' not found"}', - headers={"fga-request-id": "test-request-123"} + headers={"fga-request-id": "test-request-123"}, ) # Create exception @@ -61,8 +60,7 @@ def test_error_message_format_with_all_fields(self): # Set parsed exception parsed = ValidationErrorMessageResponse( - code=ErrorCode.VALIDATION_ERROR, - message="type 'invalid_type' not found" + code=ErrorCode.VALIDATION_ERROR, message="type 'invalid_type' not found" ) exc.parsed_exception = parsed @@ -79,14 +77,13 @@ def test_error_properties_accessible(self): response = MockHTTPResponse( status=400, reason="Bad Request", - data=b'{}', - headers={"fga-request-id": "test-request-456"} + data=b"{}", + headers={"fga-request-id": "test-request-456"}, ) exc = ValidationException(http_resp=response, operation_name="check") parsed = ValidationErrorMessageResponse( - code=ErrorCode.VALIDATION_ERROR, - message="Invalid relation" + code=ErrorCode.VALIDATION_ERROR, message="Invalid relation" ) exc.parsed_exception = parsed @@ -99,7 +96,7 @@ def test_error_properties_accessible(self): def test_is_validation_error_helper(self): """Test is_validation_error() helper method.""" - response = MockHTTPResponse(400, "Bad Request", b'{}') + response = MockHTTPResponse(400, "Bad Request", b"{}") exc = ValidationException(http_resp=response) parsed = ValidationErrorMessageResponse(code=ErrorCode.VALIDATION_ERROR) exc.parsed_exception = parsed @@ -111,7 +108,7 @@ def test_is_validation_error_helper(self): def test_is_not_found_error_helper(self): """Test is_not_found_error() helper method.""" - response = MockHTTPResponse(404, "Not Found", b'{}') + response = MockHTTPResponse(404, "Not Found", b"{}") exc = NotFoundException(http_resp=response) assert exc.is_not_found_error() is True @@ -121,7 +118,7 @@ def test_is_not_found_error_helper(self): def test_is_authentication_error_helper(self): """Test is_authentication_error() helper method.""" - response = MockHTTPResponse(401, "Unauthorized", b'{}') + response = MockHTTPResponse(401, "Unauthorized", b"{}") exc = ApiException(http_resp=response) assert exc.is_authentication_error() is True @@ -129,7 +126,7 @@ def test_is_authentication_error_helper(self): def test_is_rate_limit_error_helper(self): """Test is_rate_limit_error() helper method.""" - response = MockHTTPResponse(429, "Too Many Requests", b'{}') + response = MockHTTPResponse(429, "Too Many Requests", b"{}") exc = RateLimitExceededError(http_resp=response) assert exc.is_rate_limit_error() is True @@ -138,7 +135,7 @@ def test_is_rate_limit_error_helper(self): def test_is_server_error_helper(self): """Test is_server_error() helper method.""" - response = MockHTTPResponse(500, "Internal Server Error", b'{}') + response = MockHTTPResponse(500, "Internal Server Error", b"{}") exc = ServiceException(http_resp=response) assert exc.is_server_error() is True @@ -149,19 +146,21 @@ def test_is_retryable_helper(self): """Test is_retryable() helper for various status codes.""" # Retryable errors for status in [429, 500, 502, 503, 504]: - response = MockHTTPResponse(status, "Error", b'{}') + response = MockHTTPResponse(status, "Error", b"{}") exc = ApiException(http_resp=response) assert exc.is_retryable() is True, f"Status {status} should be retryable" # Non-retryable errors for status in [400, 401, 403, 404]: - response = MockHTTPResponse(status, "Error", b'{}') + response = MockHTTPResponse(status, "Error", b"{}") exc = ApiException(http_resp=response) - assert exc.is_retryable() is False, f"Status {status} should not be retryable" + assert exc.is_retryable() is False, ( + f"Status {status} should not be retryable" + ) def test_error_without_parsed_exception(self): """Test error handling when parsed_exception is not set.""" - response = MockHTTPResponse(400, "Bad Request", b'{}') + response = MockHTTPResponse(400, "Bad Request", b"{}") exc = ValidationException(http_resp=response, operation_name="write") # Should use reason as fallback @@ -176,11 +175,10 @@ def test_error_without_parsed_exception(self): def test_error_without_operation_name(self): """Test error handling when operation_name is not set.""" - response = MockHTTPResponse(400, "Bad Request", b'{}') + response = MockHTTPResponse(400, "Bad Request", b"{}") exc = ApiException(http_resp=response) parsed = ValidationErrorMessageResponse( - code=ErrorCode.VALIDATION_ERROR, - message="Test error" + code=ErrorCode.VALIDATION_ERROR, message="Test error" ) exc.parsed_exception = parsed @@ -192,7 +190,7 @@ def test_error_without_operation_name(self): def test_error_without_request_id(self): """Test error handling when request_id is not available.""" - response = MockHTTPResponse(400, "Bad Request", b'{}', headers={}) + response = MockHTTPResponse(400, "Bad Request", b"{}", headers={}) exc = ValidationException(http_resp=response, operation_name="write") assert exc.request_id is None @@ -203,7 +201,7 @@ def test_error_without_request_id(self): def test_error_code_with_enum(self): """Test that error code property handles enum values.""" - response = MockHTTPResponse(400, "Bad Request", b'{}') + response = MockHTTPResponse(400, "Bad Request", b"{}") exc = ValidationException(http_resp=response) # Create parsed exception with enum @@ -217,20 +215,19 @@ def test_error_code_with_enum(self): def test_multiple_errors_have_unique_messages(self): """Test that different errors have different details.""" # Error 1: Invalid type - response1 = MockHTTPResponse(400, "Bad Request", b'{}') + response1 = MockHTTPResponse(400, "Bad Request", b"{}") exc1 = ValidationException(http_resp=response1, operation_name="write") parsed1 = ValidationErrorMessageResponse( - code=ErrorCode.VALIDATION_ERROR, - message="type 'invalid_type' not found" + code=ErrorCode.VALIDATION_ERROR, message="type 'invalid_type' not found" ) exc1.parsed_exception = parsed1 # Error 2: Invalid relation - response2 = MockHTTPResponse(400, "Bad Request", b'{}') + response2 = MockHTTPResponse(400, "Bad Request", b"{}") exc2 = ValidationException(http_resp=response2, operation_name="write") parsed2 = ValidationErrorMessageResponse( code=ErrorCode.VALIDATION_ERROR, - message="relation 'invalid_relation' not found" + message="relation 'invalid_relation' not found", ) exc2.parsed_exception = parsed2 @@ -242,7 +239,7 @@ def test_multiple_errors_have_unique_messages(self): def test_operation_context_preserved(self): """Test that operation name is preserved in exception.""" - response = MockHTTPResponse(400, "Bad Request", b'{}') + response = MockHTTPResponse(400, "Bad Request", b"{}") write_exc = ValidationException(http_resp=response, operation_name="write") assert write_exc.operation_name == "write" @@ -258,13 +255,11 @@ def test_error_message_consistency(self): import re response = MockHTTPResponse( - 400, "Bad Request", b'{}', - headers={"fga-request-id": "req-123"} + 400, "Bad Request", b"{}", headers={"fga-request-id": "req-123"} ) exc = ValidationException(http_resp=response, operation_name="write") parsed = ValidationErrorMessageResponse( - code=ErrorCode.VALIDATION_ERROR, - message="Test error" + code=ErrorCode.VALIDATION_ERROR, message="Test error" ) exc.parsed_exception = parsed @@ -272,20 +267,22 @@ def test_error_message_consistency(self): # Should match pattern: [operation] HTTP status message (code) [request-id: id] pattern = r"^\[write\] HTTP 400 Test error \(validation_error\) \[request-id: req-123\]$" - assert re.match(pattern, error_str), f"Error string '{error_str}' doesn't match expected pattern" + assert re.match(pattern, error_str), ( + f"Error string '{error_str}' doesn't match expected pattern" + ) def test_client_vs_server_error_categorization(self): """Test that client and server errors are properly categorized.""" # Client errors (4xx) for status in [400, 401, 403, 404, 429]: - response = MockHTTPResponse(status, "Error", b'{}') + response = MockHTTPResponse(status, "Error", b"{}") exc = ApiException(http_resp=response) assert exc.is_client_error() is True assert exc.is_server_error() is False # Server errors (5xx) for status in [500, 502, 503, 504]: - response = MockHTTPResponse(status, "Error", b'{}') + response = MockHTTPResponse(status, "Error", b"{}") exc = ApiException(http_resp=response) assert exc.is_server_error() is True assert exc.is_client_error() is False @@ -293,22 +290,21 @@ def test_client_vs_server_error_categorization(self): def test_exception_subclass_helpers(self): """Test that helper methods work for exception subclasses.""" # ValidationException - response = MockHTTPResponse(400, "Bad Request", b'{}') + response = MockHTTPResponse(400, "Bad Request", b"{}") exc = ValidationException(http_resp=response) assert exc.is_validation_error() is True # NotFoundException - response = MockHTTPResponse(404, "Not Found", b'{}') + response = MockHTTPResponse(404, "Not Found", b"{}") exc = NotFoundException(http_resp=response) assert exc.is_not_found_error() is True # ServiceException - response = MockHTTPResponse(500, "Internal Server Error", b'{}') + response = MockHTTPResponse(500, "Internal Server Error", b"{}") exc = ServiceException(http_resp=response) assert exc.is_server_error() is True # RateLimitExceededError - response = MockHTTPResponse(429, "Too Many Requests", b'{}') + response = MockHTTPResponse(429, "Too Many Requests", b"{}") exc = RateLimitExceededError(http_resp=response) assert exc.is_rate_limit_error() is True - diff --git a/test/error_integration_test.py b/test/error_integration_test.py index 7f26faa..ce17f22 100644 --- a/test/error_integration_test.py +++ b/test/error_integration_test.py @@ -16,17 +16,19 @@ """ import os + import pytest import pytest_asyncio from openfga_sdk.client import ClientConfiguration from openfga_sdk.client.client import OpenFgaClient -from openfga_sdk.exceptions import ValidationException, NotFoundException +from openfga_sdk.exceptions import NotFoundException, ValidationException + # Skip all tests if FGA_API_URL is not set (for CI/CD environments without OpenFGA) pytestmark = pytest.mark.skipif( not os.environ.get("FGA_API_URL") and not os.path.exists("/.dockerenv"), - reason="OpenFGA server not available. Set FGA_API_URL to run integration tests." + reason="OpenFGA server not available. Set FGA_API_URL to run integration tests.", ) @@ -256,7 +258,11 @@ async def test_error_helper_methods_validation(self, fga_client): async def test_error_message_format_consistency(self, fga_client): """Test that error messages follow consistent format across operations.""" - from openfga_sdk.client.models import ClientTuple, ClientCheckRequest, ClientWriteRequest + from openfga_sdk.client.models import ( + ClientCheckRequest, + ClientTuple, + ClientWriteRequest, + ) # Test write error format with pytest.raises(ValidationException) as write_exc: @@ -296,6 +302,7 @@ async def test_error_message_format_consistency(self, fga_client): # Both should follow same pattern (with or without request-id) import re + # Pattern with optional request-id at the end pattern = r"^\[\w+\] HTTP \d{3} .+ \(.+\)( \[request-id: .+\])?$" assert re.match(pattern, write_error) @@ -334,9 +341,12 @@ async def test_error_code_fields_accessibility(self, fga_client): # Request ID should match expected format if present if exception.request_id: import re + assert re.match(r"[a-zA-Z0-9-]+", exception.request_id) - async def test_different_validation_errors_have_different_messages(self, fga_client): + async def test_different_validation_errors_have_different_messages( + self, fga_client + ): """Test that different validation errors surface different messages.""" from openfga_sdk.client.models import ClientTuple, ClientWriteRequest @@ -423,8 +433,8 @@ def fga_client_sync(self): Note: This requires a running OpenFGA server. """ - from openfga_sdk.sync.client.client import OpenFgaClient as SyncOpenFgaClient from openfga_sdk.models import CreateStoreRequest + from openfga_sdk.sync.client.client import OpenFgaClient as SyncOpenFgaClient api_url = os.environ.get("FGA_API_URL", "http://localhost:8080") @@ -503,4 +513,3 @@ def test_sync_error_helper_methods(self, fga_client_sync): assert exception.is_client_error() assert not exception.is_server_error() assert not exception.is_retryable() - From e6ed185484071e31b964dd6f044303242413f126 Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Fri, 21 Nov 2025 17:05:36 +0530 Subject: [PATCH 4/5] feat: update --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 923aef2..2f81122 100644 --- a/README.md +++ b/README.md @@ -1414,7 +1414,6 @@ The SDK provides comprehensive error handling with detailed error information an Key features: - Operation context in error messages (e.g., `[write]`, `[check]`) - Detailed error codes and messages from the API -- Request IDs for debugging and support - Helper methods for error categorization (`is_validation_error()`, `is_retryable()`, etc.) ```python From 10fbf4cb789801968bad8bc5ef9071b37d0adaf2 Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Fri, 21 Nov 2025 17:53:40 +0530 Subject: [PATCH 5/5] feat: address copilot comments --- openfga_sdk/api_client.py | 12 ++++++--- openfga_sdk/exceptions.py | 46 +++++++++++++++++++++++----------- openfga_sdk/sync/api_client.py | 15 ++++++----- test/error_integration_test.py | 1 + 4 files changed, 49 insertions(+), 25 deletions(-) diff --git a/openfga_sdk/api_client.py b/openfga_sdk/api_client.py index 8b3f3ac..d4cc632 100644 --- a/openfga_sdk/api_client.py +++ b/openfga_sdk/api_client.py @@ -321,9 +321,11 @@ async def __call_api( and TelemetryAttributes.fga_client_request_method in _telemetry_attributes ): - e.operation_name = _telemetry_attributes[ + operation_name = _telemetry_attributes.get( TelemetryAttributes.fga_client_request_method - ].lower() + ) + if isinstance(operation_name, str): + e.operation_name = operation_name.lower() raise e except ApiException as e: e.body = e.body.decode("utf-8") @@ -361,9 +363,11 @@ async def __call_api( and TelemetryAttributes.fga_client_request_method in _telemetry_attributes ): - e.operation_name = _telemetry_attributes[ + operation_name = _telemetry_attributes.get( TelemetryAttributes.fga_client_request_method - ].lower() + ) + if isinstance(operation_name, str): + e.operation_name = operation_name.lower() raise self.last_response = response_data diff --git a/openfga_sdk/exceptions.py b/openfga_sdk/exceptions.py index e1f8e35..0523b23 100644 --- a/openfga_sdk/exceptions.py +++ b/openfga_sdk/exceptions.py @@ -116,7 +116,9 @@ def __init__(self, msg, path_to_item=None): class ApiException(OpenApiException): - def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): + def __init__( + self, status=None, reason=None, http_resp=None, *, operation_name=None + ): if http_resp: try: headers = http_resp.headers.items() @@ -297,38 +299,52 @@ def is_server_error(self): class NotFoundException(ApiException): - def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): - super().__init__(status, reason, http_resp, operation_name) + def __init__( + self, status=None, reason=None, http_resp=None, *, operation_name=None + ): + super().__init__(status, reason, http_resp, operation_name=operation_name) class UnauthorizedException(ApiException): - def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): - super().__init__(status, reason, http_resp, operation_name) + def __init__( + self, status=None, reason=None, http_resp=None, *, operation_name=None + ): + super().__init__(status, reason, http_resp, operation_name=operation_name) class ForbiddenException(ApiException): - def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): - super().__init__(status, reason, http_resp, operation_name) + def __init__( + self, status=None, reason=None, http_resp=None, *, operation_name=None + ): + super().__init__(status, reason, http_resp, operation_name=operation_name) class ServiceException(ApiException): - def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): - super().__init__(status, reason, http_resp, operation_name) + def __init__( + self, status=None, reason=None, http_resp=None, *, operation_name=None + ): + super().__init__(status, reason, http_resp, operation_name=operation_name) class ValidationException(ApiException): - def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): - super().__init__(status, reason, http_resp, operation_name) + def __init__( + self, status=None, reason=None, http_resp=None, *, operation_name=None + ): + super().__init__(status, reason, http_resp, operation_name=operation_name) class AuthenticationError(ApiException): - def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): - super().__init__(status, reason, http_resp, operation_name) + def __init__( + self, status=None, reason=None, http_resp=None, *, operation_name=None + ): + super().__init__(status, reason, http_resp, operation_name=operation_name) class RateLimitExceededError(ApiException): - def __init__(self, status=None, reason=None, http_resp=None, operation_name=None): - super().__init__(status, reason, http_resp, operation_name) + def __init__( + self, status=None, reason=None, http_resp=None, *, operation_name=None + ): + super().__init__(status, reason, http_resp, operation_name=operation_name) def render_path(path_to_item): diff --git a/openfga_sdk/sync/api_client.py b/openfga_sdk/sync/api_client.py index a6f9ea6..7990e25 100644 --- a/openfga_sdk/sync/api_client.py +++ b/openfga_sdk/sync/api_client.py @@ -320,11 +320,12 @@ def __call_api( and TelemetryAttributes.fga_client_request_method in _telemetry_attributes ): - # Convert operation name to lowercase for consistency - e.operation_name = _telemetry_attributes[ + operation_name = _telemetry_attributes.get( TelemetryAttributes.fga_client_request_method - ].lower() - raise e + ) + if isinstance(operation_name, str): + e.operation_name = operation_name.lower() + raise except ApiException as e: e.body = e.body.decode("utf-8") response_type = response_types_map.get(e.status, None) @@ -362,9 +363,11 @@ def __call_api( and TelemetryAttributes.fga_client_request_method in _telemetry_attributes ): - e.operation_name = _telemetry_attributes[ + operation_name = _telemetry_attributes.get( TelemetryAttributes.fga_client_request_method - ].lower() + ) + if isinstance(operation_name, str): + e.operation_name = operation_name.lower() raise self.last_response = response_data diff --git a/test/error_integration_test.py b/test/error_integration_test.py index ce17f22..99c6b16 100644 --- a/test/error_integration_test.py +++ b/test/error_integration_test.py @@ -92,6 +92,7 @@ async def fga_client(self): try: await client.delete_store() except Exception: + # Ignore exceptions during cleanup (e.g., store may already be deleted or server unavailable) pass # Ignore cleanup errors async def test_write_validation_error_invalid_type(self, fga_client):