diff --git a/openhands/integrations/protocols/http_client.py b/openhands/integrations/protocols/http_client.py index 09bdf42953..38bf1d29a8 100644 --- a/openhands/integrations/protocols/http_client.py +++ b/openhands/integrations/protocols/http_client.py @@ -9,6 +9,7 @@ from pydantic import SecretStr from openhands.core.logger import openhands_logger as logger from openhands.integrations.service_types import ( AuthenticationError, + ForbiddenError, RateLimitError, RequestMethod, ResourceNotFoundError, @@ -111,10 +112,11 @@ class HTTPClient(ABC): return RateLimitError(f'{self.provider} API rate limit exceeded') except Exception as json_error: logger.debug(f'Failed to parse 403 error response as JSON: {json_error}') - pass # If we can't parse the response, treat as unknown error + pass # If we can't parse the response, treat as forbidden error + # Not a rate limit, so it's a permission/access issue logger.warning(f'Forbidden error on {self.provider} API: {e}') - return UnknownException(f'Forbidden error: {e}') + return ForbiddenError(f'Access forbidden: {e}') logger.warning(f'Status error on {self.provider} API: {e}') return UnknownException(f'Unknown error: {e}') diff --git a/openhands/integrations/service_types.py b/openhands/integrations/service_types.py index cfc4839059..fd579c8683 100644 --- a/openhands/integrations/service_types.py +++ b/openhands/integrations/service_types.py @@ -178,6 +178,12 @@ class RateLimitError(ValueError): pass +class ForbiddenError(ValueError): + """Raised when access to a resource is forbidden (permissions issue).""" + + pass + + class ResourceNotFoundError(ValueError): """Raised when a requested resource (file, directory, etc.) is not found.""" diff --git a/openhands/server/app.py b/openhands/server/app.py index d5135f2399..3cc6c561b5 100644 --- a/openhands/server/app.py +++ b/openhands/server/app.py @@ -17,7 +17,11 @@ from fastapi.responses import JSONResponse import openhands.agenthub # noqa F401 (we import this to get the agents registered) from openhands.app_server import v1_router from openhands.app_server.config import get_app_lifespan_service -from openhands.integrations.service_types import AuthenticationError +from openhands.integrations.service_types import ( + AuthenticationError, + ForbiddenError, + RateLimitError, +) from openhands.server.routes.conversation import app as conversation_api_router from openhands.server.routes.feedback import app as feedback_api_router from openhands.server.routes.files import app as files_api_router @@ -80,6 +84,22 @@ async def authentication_error_handler(request: Request, exc: AuthenticationErro ) +@app.exception_handler(RateLimitError) +async def rate_limit_error_handler(request: Request, exc: RateLimitError): + return JSONResponse( + status_code=429, + content=str(exc), + ) + + +@app.exception_handler(ForbiddenError) +async def forbidden_error_handler(request: Request, exc: ForbiddenError): + return JSONResponse( + status_code=403, + content=str(exc), + ) + + app.include_router(public_api_router) app.include_router(files_api_router) app.include_router(security_api_router) diff --git a/tests/unit/integrations/github/test_github_rate_limit.py b/tests/unit/integrations/github/test_github_rate_limit.py index 12a19e9f05..0daf634cbf 100644 --- a/tests/unit/integrations/github/test_github_rate_limit.py +++ b/tests/unit/integrations/github/test_github_rate_limit.py @@ -1,9 +1,9 @@ """ -Unit tests for GitHub rate limit error handling. +Unit tests for GitHub rate limit and forbidden error handling. Tests cover: 1. Rate limit error detection (403 with rate limit message) -2. Permission denied error handling (403 without rate limit) +2. Permission denied error handling (403 without rate limit message) """ from unittest.mock import AsyncMock, Mock, patch @@ -11,7 +11,7 @@ from unittest.mock import AsyncMock, Mock, patch import pytest from openhands.integrations.github.service.repos import GithubReposService -from openhands.integrations.protocols.http_client import RateLimitError, UnknownException +from openhands.integrations.service_types import ForbiddenError, RateLimitError from openhands.server.shared import AppMode @@ -61,7 +61,7 @@ class TestRateLimitHandling: @pytest.mark.asyncio async def test_forbidden_error_non_rate_limit(self, repos_service): """ - Test that 403 without rate limit message is raised as UnknownException. + Test that 403 without rate limit message is raised as ForbiddenError. Not all 403 errors are rate limits - could be permission denied, etc. """ @@ -74,9 +74,9 @@ class TestRateLimitHandling: } with patch.object( - repos_service, '_make_request', side_effect=UnknownException(mock_response) + repos_service, '_make_request', side_effect=ForbiddenError(mock_response) ): - with pytest.raises(UnknownException) as exc_info: + with pytest.raises(ForbiddenError) as exc_info: await repos_service.search_repositories( query='test', per_page=100, @@ -86,6 +86,6 @@ class TestRateLimitHandling: app_mode=AppMode.SAAS, ) - # Verify it's UnknownException, not RateLimitError - assert isinstance(exc_info.value, UnknownException) + # Verify it's ForbiddenError, not RateLimitError + assert isinstance(exc_info.value, ForbiddenError) assert not isinstance(exc_info.value, RateLimitError)