Add info logging for 401 Unauthorized responses (#8527)

Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
Robert Brennan 2025-05-16 11:46:15 -04:00 committed by GitHub
parent 25619c5a93
commit 21dd91de63
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 38 additions and 3 deletions

View File

@ -2,6 +2,7 @@ from fastapi import APIRouter, Depends, status
from fastapi.responses import JSONResponse
from pydantic import SecretStr
from openhands.core.logger import openhands_logger as logger
from openhands.integrations.provider import (
PROVIDER_TOKEN_TYPE,
ProviderHandler,
@ -42,6 +43,9 @@ async def get_user_repositories(
return await client.get_repositories(sort, server_config.app_mode)
except AuthenticationError as e:
logger.info(
f'Returning 401 Unauthorized - Authentication error for user_id: {user_id}, error: {str(e)}'
)
return JSONResponse(
content=str(e),
status_code=status.HTTP_401_UNAUTHORIZED,
@ -53,6 +57,9 @@ async def get_user_repositories(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
)
logger.info(
f'Returning 401 Unauthorized - Git provider token required for user_id: {user_id}'
)
return JSONResponse(
content='Git provider token required. (such as GitHub).',
status_code=status.HTTP_401_UNAUTHORIZED,
@ -63,6 +70,7 @@ async def get_user_repositories(
async def get_user(
provider_tokens: PROVIDER_TOKEN_TYPE | None = Depends(get_provider_tokens),
access_token: SecretStr | None = Depends(get_access_token),
user_id: str | None = Depends(get_user_id),
) -> User | JSONResponse:
if provider_tokens:
client = ProviderHandler(
@ -74,6 +82,9 @@ async def get_user(
return user
except AuthenticationError as e:
logger.info(
f'Returning 401 Unauthorized - Authentication error for user_id: {user_id}, error: {str(e)}'
)
return JSONResponse(
content=str(e),
status_code=status.HTTP_401_UNAUTHORIZED,
@ -85,6 +96,9 @@ async def get_user(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
)
logger.info(
f'Returning 401 Unauthorized - Git provider token required for user_id: {user_id}'
)
return JSONResponse(
content='Git provider token required. (such as GitHub).',
status_code=status.HTTP_401_UNAUTHORIZED,
@ -99,6 +113,7 @@ async def search_repositories(
order: str = 'desc',
provider_tokens: PROVIDER_TOKEN_TYPE | None = Depends(get_provider_tokens),
access_token: SecretStr | None = Depends(get_access_token),
user_id: str | None = Depends(get_user_id),
) -> list[Repository] | JSONResponse:
if provider_tokens:
client = ProviderHandler(
@ -122,6 +137,9 @@ async def search_repositories(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
)
logger.info(
f'Returning 401 Unauthorized - GitHub token required for user_id: {user_id}'
)
return JSONResponse(
content='GitHub token required.',
status_code=status.HTTP_401_UNAUTHORIZED,
@ -132,6 +150,7 @@ async def search_repositories(
async def get_suggested_tasks(
provider_tokens: PROVIDER_TOKEN_TYPE | None = Depends(get_provider_tokens),
access_token: SecretStr | None = Depends(get_access_token),
user_id: str | None = Depends(get_user_id),
) -> list[SuggestedTask] | JSONResponse:
"""Get suggested tasks for the authenticated user across their most recently pushed repositories.
@ -158,6 +177,7 @@ async def get_suggested_tasks(
content=str(e),
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
)
logger.info(f'Returning 401 Unauthorized - No providers set for user_id: {user_id}')
return JSONResponse(
content='No providers set.',
@ -170,6 +190,7 @@ async def get_repository_branches(
repository: str,
provider_tokens: PROVIDER_TOKEN_TYPE | None = Depends(get_provider_tokens),
access_token: SecretStr | None = Depends(get_access_token),
user_id: str | None = Depends(get_user_id),
) -> list[Branch] | JSONResponse:
"""Get branches for a repository.
@ -199,6 +220,10 @@ async def get_repository_branches(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
)
logger.info(
f'Returning 401 Unauthorized - Git provider token required for user_id: {user_id}'
)
return JSONResponse(
content='Git provider token required. (such as GitHub).',
status_code=status.HTTP_401_UNAUTHORIZED,

View File

@ -2,8 +2,7 @@ from fastapi import APIRouter, Depends, status
from fastapi.responses import JSONResponse
from openhands.core.logger import openhands_logger as logger
from openhands.integrations.provider import CustomSecret
from openhands.integrations.provider import PROVIDER_TOKEN_TYPE
from openhands.integrations.provider import PROVIDER_TOKEN_TYPE, CustomSecret
from openhands.integrations.service_types import ProviderType
from openhands.integrations.utils import validate_provider_token
from openhands.server.settings import (
@ -110,6 +109,10 @@ async def store_provider_tokens(
) -> JSONResponse:
provider_err_msg = await check_provider_tokens(provider_info, provider_tokens)
if provider_err_msg:
# We don't have direct access to user_id here, but we can log the provider info
logger.info(
f'Returning 401 Unauthorized - Provider token error: {provider_err_msg}'
)
return JSONResponse(
status_code=status.HTTP_401_UNAUTHORIZED,
content={'error': provider_err_msg},
@ -203,6 +206,7 @@ async def load_custom_secrets_names(
except Exception as e:
logger.warning(f'Failed to load secret names: {e}')
logger.info('Returning 401 Unauthorized - Failed to get secret names')
return JSONResponse(
status_code=status.HTTP_401_UNAUTHORIZED,
content={'error': 'Failed to get secret names'},

View File

@ -71,6 +71,11 @@ async def load_settings(
return settings_with_token_data
except Exception as e:
logger.warning(f'Invalid token: {e}')
# Get user_id from settings if available
user_id = getattr(settings, 'user_id', 'unknown') if settings else 'unknown'
logger.info(
f'Returning 401 Unauthorized - Invalid token for user_id: {user_id}'
)
return JSONResponse(
status_code=status.HTTP_401_UNAUTHORIZED,
content={'error': 'Invalid token'},

View File

@ -233,6 +233,7 @@ async def test_delegate_step_different_states(
else:
assert controller.delegate is None
assert controller.state.iteration == 5
mock_delegate.close.assert_called_once()
# The close method is called once in end_delegate
assert mock_delegate.close.call_count == 1
await controller.close()