Skip to content

Add enhanced logging for error paths in add-git-providers endpoint #8890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rbren
Copy link
Collaborator

@rbren rbren commented Jun 4, 2025

Description

This PR adds enhanced logging for error paths in the add-git-providers endpoint, with a particular focus on HTTP errors.

Changes

  • Added detailed logging in validate_provider_token function to capture HTTP errors with status codes and response content
  • Enhanced error handling in check_provider_tokens function with try/except blocks and more detailed logging
  • Added specific HTTP error handling in the store_provider_tokens endpoint
  • Improved logging throughout the token validation process
  • Changed log level from info to warning or error for error conditions

Testing

The changes have been tested with the pre-commit hooks to ensure code quality.

Impact

These changes will make it easier to diagnose issues with git provider token validation by providing more detailed logs, especially for HTTP errors.


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:c89e726-nikolaik   --name openhands-app-c89e726   docker.all-hands.dev/all-hands-ai/openhands:c89e726

@rbren
Copy link
Collaborator Author

rbren commented Jun 6, 2025

@OpenHands move the logging into the GitHubService and GitLabService. Make sure the hostname/urls being used get logged too

Copy link

openhands-ai bot commented Jun 6, 2025

I'm on it! rbren can track my progress at all-hands.dev

Copy link

openhands-ai bot commented Jun 6, 2025

I've successfully moved the logging from the validate_provider_token function into the GitHubService and GitLabService classes, and added hostname/URL information to the logs.

Here's a summary of the changes made:

  1. Enhanced the GitHubService class:

    • Added logging in the __init__ method to log the base URL being used
    • Added detailed logging in the verify_access method with URL information
    • Added proper error handling with specific log messages for different error types
  2. Enhanced the GitLabService class:

    • Added logging in the __init__ method to log both the base URL and GraphQL URL
    • Added detailed logging in the get_user method with URL information
    • Added proper error handling with specific log messages for different error types
  3. Simplified the validate_provider_token function in utils.py:

    • Removed redundant logging since it's now handled in the service classes
    • Removed unnecessary imports
  4. Fixed all code quality issues and ensured the pre-commit hooks pass

These changes will make it easier to diagnose issues with git provider token validation by providing more detailed logs that include the hostname/URLs being used, especially for HTTP errors.

The changes have been successfully pushed to the improve-git-providers-logging branch, updating PR #8890.

Copy link

openhands-ai bot commented Jun 6, 2025

Looks like there are a few issues preventing this PR from being merged!

  • GitHub Actions are failing:
    • Docker

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #8890

Feel free to include any additional details that might help me get this PR into a better state.

You can manage your notification settings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants