Move logging into GitHubService and GitLabService with hostname/URL information

openhands/integrations/github/github_service.py

@@ -39,6 +39,7 @@ class GitHubService(BaseGitService, GitService):
 
     The class is instantiated via get_impl() in openhands.server.shared.py.
     """
+
     BASE_URL = 'https://api.github.com'
     token: SecretStr = SecretStr('')
     refresh = False
@@ -60,6 +61,9 @@ class GitHubService(BaseGitService, GitService):
 
         if base_domain and base_domain != 'github.com':
             self.BASE_URL = f'https://{base_domain}/api/v3'
+            logger.info(f'Using custom GitHub base URL: {self.BASE_URL}')
+        else:
+            logger.info(f'Using default GitHub base URL: {self.BASE_URL}')
 
         self.external_auth_id = external_auth_id
         self.external_auth_token = external_auth_token
@@ -143,8 +147,26 @@ class GitHubService(BaseGitService, GitService):
     async def verify_access(self) -> bool:
         """Verify if the token is valid by making a simple request."""
         url = f'{self.BASE_URL}'
-        await self._make_request(url)
-        return True
+        logger.info(f'Verifying GitHub access with URL: {url}')
+        try:
+            await self._make_request(url)
+            logger.info(f'Successfully verified GitHub access to {url}')
+            return True
+        except httpx.HTTPStatusError as e:
+            logger.warning(
+                f'HTTP status error verifying GitHub access to {url}: {e.response.status_code} - {e.response.reason_phrase}'
+            )
+            if hasattr(e.response, 'text'):
+                logger.warning(f'Response content: {e.response.text}')
+            raise
+        except httpx.HTTPError as e:
+            logger.warning(
+                f'HTTP error verifying GitHub access to {url}: {type(e).__name__} - {str(e)}'
+            )
+            raise
+        except Exception as e:
+            logger.warning(f'Failed to verify GitHub access to {url}: {e}')
+            raise
 
     async def _fetch_paginated_repos(
         self, url: str, params: dict, max_repos: int, extract_key: str | None = None

openhands/integrations/gitlab/gitlab_service.py

@@ -4,6 +4,7 @@ from typing import Any
 import httpx
 from pydantic import SecretStr
 
+from openhands.core.logger import openhands_logger as logger
 from openhands.integrations.service_types import (
     BaseGitService,
     Branch,
@@ -32,6 +33,7 @@ class GitLabService(BaseGitService, GitService):
 
     The class is instantiated via get_impl() in openhands.server.shared.py.
     """
+
     BASE_URL = 'https://gitlab.com/api/v4'
     GRAPHQL_URL = 'https://gitlab.com/api/graphql'
     token: SecretStr = SecretStr('')
@@ -55,6 +57,11 @@ class GitLabService(BaseGitService, GitService):
         if base_domain:
             self.BASE_URL = f'https://{base_domain}/api/v4'
             self.GRAPHQL_URL = f'https://{base_domain}/api/graphql'
+            logger.info(f'Using custom GitLab base URL: {self.BASE_URL}')
+            logger.info(f'Using custom GitLab GraphQL URL: {self.GRAPHQL_URL}')
+        else:
+            logger.info(f'Using default GitLab base URL: {self.BASE_URL}')
+            logger.info(f'Using default GitLab GraphQL URL: {self.GRAPHQL_URL}')
 
     @property
     def provider(self) -> str:
@@ -176,21 +183,39 @@ class GitLabService(BaseGitService, GitService):
 
     async def get_user(self) -> User:
         url = f'{self.BASE_URL}/user'
-        response, _ = await self._make_request(url)
+        logger.info(f'Getting GitLab user info from URL: {url}')
+        try:
+            response, _ = await self._make_request(url)
 
-        # Use a default avatar URL if not provided
-        # In some self-hosted GitLab instances, the avatar_url field may be returned as None.
-        avatar_url = response.get('avatar_url') or ''
+            # Use a default avatar URL if not provided
+            # In some self-hosted GitLab instances, the avatar_url field may be returned as None.
+            avatar_url = response.get('avatar_url') or ''
 
-        return User(
-            id=response.get('id'),
-            username=response.get('username'),
-            avatar_url=avatar_url,
-            name=response.get('name'),
-            email=response.get('email'),
-            company=response.get('organization'),
-            login=response.get('username'),
-        )
+            logger.info(f'Successfully retrieved GitLab user info from {url}')
+            return User(
+                id=response.get('id'),
+                username=response.get('username'),
+                avatar_url=avatar_url,
+                name=response.get('name'),
+                email=response.get('email'),
+                company=response.get('organization'),
+                login=response.get('username'),
+            )
+        except httpx.HTTPStatusError as e:
+            logger.warning(
+                f'HTTP status error retrieving GitLab user info from {url}: {e.response.status_code} - {e.response.reason_phrase}'
+            )
+            if hasattr(e.response, 'text'):
+                logger.warning(f'Response content: {e.response.text}')
+            raise
+        except httpx.HTTPError as e:
+            logger.warning(
+                f'HTTP error retrieving GitLab user info from {url}: {type(e).__name__} - {str(e)}'
+            )
+            raise
+        except Exception as e:
+            logger.warning(f'Failed to retrieve GitLab user info from {url}: {e}')
+            raise
 
     async def search_repositories(
         self, query: str, per_page: int = 30, sort: str = 'updated', order: str = 'desc'

openhands/integrations/utils.py

@@ -1,5 +1,3 @@
-import traceback
-
 from pydantic import SecretStr
 
 from openhands.core.logger import openhands_logger as logger
@@ -17,6 +15,7 @@ async def validate_provider_token(
 
     Args:
         token: The token to check
+        base_domain: Optional base domain for self-hosted instances
 
     Returns:
         'github' if it's a GitHub token
@@ -28,19 +27,18 @@ async def validate_provider_token(
         github_service = GitHubService(token=token, base_domain=base_domain)
         await github_service.verify_access()
         return ProviderType.GITHUB
-    except Exception as e:
-        logger.debug(
-            f'Failed to validate Github token: {e} \n {traceback.format_exc()}'
-        )
+    except Exception:
+        # Exceptions are already logged in the service class
+        pass
 
     # Try GitLab next
     try:
         gitlab_service = GitLabService(token=token, base_domain=base_domain)
         await gitlab_service.get_user()
         return ProviderType.GITLAB
-    except Exception as e:
-        logger.debug(
-            f'Failed to validate GitLab token: {e} \n {traceback.format_exc()}'
-        )
+    except Exception:
+        # Exceptions are already logged in the service class
+        pass
 
+    logger.warning('Token validation failed for both GitHub and GitLab')
     return None

openhands/server/routes/secrets.py

@@ -1,3 +1,4 @@
+import httpx
 from fastapi import APIRouter, Depends, status
 from fastapi.responses import JSONResponse
 
@@ -73,32 +74,63 @@ async def check_provider_tokens(
 ) -> str:
     msg = ''
     if incoming_provider_tokens.provider_tokens:
+        logger.info('Validating provider tokens')
         # Determine whether tokens are valid
         for token_type, token_value in incoming_provider_tokens.provider_tokens.items():
-            if token_value.token:
-                confirmed_token_type = await validate_provider_token(
-                    token_value.token, token_value.host
-                )  # FE always sends latest host
-                msg = process_token_validation_result(confirmed_token_type, token_type)
+            logger.info(f'Checking token for provider type: {token_type}')
+            try:
+                if token_value.token:
+                    logger.info(
+                        f'Validating new token for {token_type} with host {token_value.host}'
+                    )
+                    confirmed_token_type = await validate_provider_token(
+                        token_value.token, token_value.host
+                    )  # FE always sends latest host
+
+                    if not confirmed_token_type:
+                        logger.warning(
+                            f'Token validation failed for {token_type}: Invalid token'
+                        )
+                    elif confirmed_token_type != token_type:
+                        logger.warning(
+                            f'Token validation failed for {token_type}: Token is for {confirmed_token_type}'
+                        )
 
-            existing_token = (
-                existing_provider_tokens.get(token_type, None)
-                if existing_provider_tokens
-                else None
-            )
-            if (
-                existing_token
-                and (existing_token.host != token_value.host)
-                and existing_token.token
-            ):
-                confirmed_token_type = await validate_provider_token(
-                    existing_token.token, token_value.host
-                )  # Host has changed, check it against existing token
-                if not confirmed_token_type or confirmed_token_type != token_type:
                     msg = process_token_validation_result(
                         confirmed_token_type, token_type
                     )
 
+                existing_token = (
+                    existing_provider_tokens.get(token_type, None)
+                    if existing_provider_tokens
+                    else None
+                )
+                if (
+                    existing_token
+                    and (existing_token.host != token_value.host)
+                    and existing_token.token
+                ):
+                    logger.info(
+                        f'Host changed for {token_type} from {existing_token.host} to {token_value.host}, validating existing token'
+                    )
+                    confirmed_token_type = await validate_provider_token(
+                        existing_token.token, token_value.host
+                    )  # Host has changed, check it against existing token
+
+                    if not confirmed_token_type or confirmed_token_type != token_type:
+                        logger.warning(
+                            f'Existing token validation failed for {token_type} with new host {token_value.host}'
+                        )
+                        msg = process_token_validation_result(
+                            confirmed_token_type, token_type
+                        )
+            except Exception as e:
+                logger.error(
+                    f'Error during token validation for {token_type}: {e}',
+                    exc_info=True,
+                )
+                msg = f'Error validating {token_type} token: {str(e)}'
+
     return msg
 
 
@@ -108,24 +140,54 @@ async def store_provider_tokens(
     secrets_store: SecretsStore = Depends(get_secrets_store),
     provider_tokens: PROVIDER_TOKEN_TYPE | None = Depends(get_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}'
-        )
+    logger.info('Processing add-git-providers request')
+
+    try:
+        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.warning(
+                f'Returning 401 Unauthorized - Provider token error: {provider_err_msg}'
+            )
+            return JSONResponse(
+                status_code=status.HTTP_401_UNAUTHORIZED,
+                content={'error': provider_err_msg},
+            )
+    except httpx.HTTPStatusError as e:
+        error_msg = f'HTTP status error during token validation: {e.response.status_code} - {e.response.reason_phrase}'
+        logger.error(error_msg)
+        if hasattr(e.response, 'text'):
+            logger.error(f'Response content: {e.response.text}')
         return JSONResponse(
-            status_code=status.HTTP_401_UNAUTHORIZED,
-            content={'error': provider_err_msg},
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            content={'error': error_msg},
+        )
+    except httpx.HTTPError as e:
+        error_msg = f'HTTP error during token validation: {type(e).__name__} - {str(e)}'
+        logger.error(error_msg)
+        return JSONResponse(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            content={'error': error_msg},
+        )
+    except Exception as e:
+        error_msg = f'Unexpected error during token validation: {str(e)}'
+        logger.error(error_msg, exc_info=True)
+        return JSONResponse(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            content={'error': error_msg},
         )
 
     try:
         user_secrets = await secrets_store.load()
         if not user_secrets:
             user_secrets = UserSecrets()
+            logger.info('Creating new UserSecrets as none exists')
 
         if provider_info.provider_tokens:
             existing_providers = [provider for provider in user_secrets.provider_tokens]
+            logger.info(
+                f'Processing provider tokens. Existing providers: {existing_providers}'
+            )
 
             # Merge incoming settings store with the existing one
             for provider, token_value in list(provider_info.provider_tokens.items()):
@@ -133,22 +195,27 @@ async def store_provider_tokens(
                     existing_token = user_secrets.provider_tokens.get(provider)
                     if existing_token and existing_token.token:
                         provider_info.provider_tokens[provider] = existing_token
+                        logger.info(f'Using existing token for provider {provider}')
 
                 provider_info.provider_tokens[provider] = provider_info.provider_tokens[
                     provider
                 ].model_copy(update={'host': token_value.host})
+                logger.info(
+                    f'Updated host for provider {provider} to {token_value.host}'
+                )
 
         updated_secrets = user_secrets.model_copy(
             update={'provider_tokens': provider_info.provider_tokens}
         )
         await secrets_store.store(updated_secrets)
+        logger.info('Successfully stored updated provider tokens')
 
         return JSONResponse(
             status_code=status.HTTP_200_OK,
             content={'message': 'Git providers stored'},
         )
     except Exception as e:
-        logger.warning(f'Something went wrong storing git providers: {e}')
+        logger.error(f'Error storing git providers: {e}', exc_info=True)
         return JSONResponse(
             status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
             content={'error': 'Something went wrong storing git providers'},