Skip to content

[BB-1538] Wrap GitHub API Errors with Grpc Codes #108

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.

Sign up for GitHub

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

Merged
mateoHernandez123 merged 1 commit into from
Oct 17, 2025
Merged

[BB-1538] Wrap GitHub API Errors with Grpc Codes #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions go.sum
View file
Open in desktop

Large diffs are not rendered by default.

7 changes: 1 addition & 6 deletions pkg/connector/api_token.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import (
"github.com/conductorone/baton-sdk/pkg/annotations"
"github.com/conductorone/baton-sdk/pkg/pagination"
resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource"
"github.com/conductorone/baton-sdk/pkg/uhttp"
"github.com/google/go-github/v69/github"
"google.golang.org/grpc/codes"
)

func apiTokenResource(ctx context.Context, token *github.PersonalAccessToken) (*v2.Resource, error) {
Expand Down Expand Up @@ -94,10 +92,7 @@ func (o *apiTokenResourceType) List(
},
})
if err != nil {
if isRatelimited(resp) {
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
}
return nil, "", nil, err
return nil, "", nil, wrapGitHubError(err, resp, "github-connector: failed to list fine-grained personal access tokens")
}

restApiRateLimit, err := extractRateLimitData(resp)
Expand Down
21 changes: 6 additions & 15 deletions pkg/connector/connector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func (gh *GitHub) Validate(ctx context.Context) (annotations.Annotations, error)
if len(gh.enterprises) > 0 {
_, _, err := gh.customClient.ListEnterpriseConsumedLicenses(ctx, gh.enterprises[0], 0)
if err != nil {
return nil, fmt.Errorf("can't list enterprise consumed licenses: %w", err)
return nil, uhttp.WrapErrors(codes.PermissionDenied, "github-connector: failed to access enterprise licenses", err)
}
}
return nil, nil
Expand All @@ -218,9 +218,9 @@ func (gh *GitHub) validateAppCredentials(ctx context.Context) (annotations.Annot
return nil, fmt.Errorf("github-connector: only one org is allowed when using github app")
}

_, _, err := findInstallation(ctx, gh.appClient, orgLogins[0])
_, resp, err := findInstallation(ctx, gh.appClient, orgLogins[0])
if err != nil {
return nil, fmt.Errorf("github-connector: failed to retrieve org: %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to retrieve org installation")
}
return nil, nil
}
Expand Down Expand Up @@ -272,9 +272,9 @@ func New(ctx context.Context, ghc *cfg.Github, appKey string) (*GitHub, error) {
if err != nil {
return nil, err
}
installation, _, err := findInstallation(ctx, appClient, ghc.Orgs[0])
installation, resp, err := findInstallation(ctx, appClient, ghc.Orgs[0])
if err != nil {
return nil, err
return nil, wrapGitHubError(err, resp, "github-connector: failed to find app installation")
}

token, err := getInstallationToken(ctx, appClient, installation.GetID())
Expand Down Expand Up @@ -453,16 +453,7 @@ func getOrgs(ctx context.Context, client *github.Client, orgs []string) ([]strin
for {
orgs, resp, err := client.Organizations.List(ctx, "", &github.ListOptions{Page: page, PerPage: maxPageSize})
if err != nil {
if isRatelimited(resp) {
return nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
}
if isAuthError(resp) {
return nil, uhttp.WrapErrors(codes.Unauthenticated, "github-connector: failed to retrieve org", err)
}
if isPermissionError(resp) {
return nil, uhttp.WrapErrors(codes.PermissionDenied, "github-connector: failed to retrieve org", err)
}
return nil, fmt.Errorf("github-connector: failed to retrieve org: %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to retrieve organizations")
}
if resp.StatusCode == http.StatusUnauthorized {
return nil, status.Error(codes.Unauthenticated, "github token is not authorized")
Expand Down
7 changes: 2 additions & 5 deletions pkg/connector/enterprise_role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (o *enterpriseRoleResourceType) fillCache(ctx context.Context) error {
for continuePagination {
consumedLicenses, _, err := o.customClient.ListEnterpriseConsumedLicenses(ctx, enterprise, page)
if err != nil {
return fmt.Errorf("baton-github: error listing enterprise consumed licenses for %s: %w", enterprise, err)
return uhttp.WrapErrors(codes.PermissionDenied, fmt.Sprintf("baton-github: error listing enterprise consumed licenses for %s", enterprise), err)
}

if len(consumedLicenses.Users) == 0 {
Expand Down Expand Up @@ -141,10 +141,7 @@ func (o *enterpriseRoleResourceType) Grants(
for _, userLogin := range cache[resource.Id.Resource] {
user, resp, err := o.client.Users.Get(ctx, userLogin)
if err != nil {
if isRatelimited(resp) {
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
}
return nil, "", nil, fmt.Errorf("baton-github: error getting user %s: %w", userLogin, err)
return nil, "", nil, wrapGitHubError(err, resp, fmt.Sprintf("baton-github: failed to get user %s", userLogin))
}

principalId, err := resourceSdk.NewResourceID(resourceTypeUser, *user.ID)
Expand Down
22 changes: 22 additions & 0 deletions pkg/connector/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ import (
v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2"
"github.com/conductorone/baton-sdk/pkg/annotations"
"github.com/conductorone/baton-sdk/pkg/pagination"
"github.com/conductorone/baton-sdk/pkg/uhttp"
"github.com/google/go-github/v69/github"
"github.com/shurcooL/githubv4"
"golang.org/x/text/cases"
"golang.org/x/text/language"
"google.golang.org/grpc/codes"
"google.golang.org/protobuf/types/known/timestamppb"
)

Expand Down Expand Up @@ -249,3 +251,23 @@ func isPermissionError(resp *github.Response) bool {
}
return resp.StatusCode == http.StatusForbidden
}

// wrapGitHubError wraps GitHub API errors with appropriate gRPC status codes based on the HTTP response.
// It handles rate limiting, authentication errors, permission errors, and generic errors.
// The contextMsg parameter should describe the operation that failed (e.g., "failed to list teams").
func wrapGitHubError(err error, resp *github.Response, contextMsg string) error {
if err == nil {
return nil
}

if isRatelimited(resp) {
Copy link
Contributor

@btipling btipling Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add the rate limit here from the error as per https://conductorone.atlassian.net/browse/BB-1660 or we could do it in a separate PR

Copy link
Contributor Author

@mateoHernandez123 mateoHernandez123 Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is best to do a specific PR to resolve that

return uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
Copy link

@pquerna pquerna Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't seem right?

Copy link

@pquerna pquerna Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like if its rate limited.... shouldn't we be returning that

Copy link
Contributor

@laurenleach laurenleach Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pquerna this is wrapping the error with the status code so baton-sdk will wait and retry

Copy link

@pquerna pquerna Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want the rate limit metadata?

Copy link
Contributor

@btipling btipling Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do and it seems like the GitHub go SDK does expose it. I have filed a separate Jira issue for it https://conductorone.atlassian.net/browse/BB-1660

}
if isAuthError(resp) {
return uhttp.WrapErrors(codes.Unauthenticated, contextMsg, err)
}
if isPermissionError(resp) {
return uhttp.WrapErrors(codes.PermissionDenied, contextMsg, err)
}
return fmt.Errorf("%s: %w", contextMsg, err)
}
4 changes: 2 additions & 2 deletions pkg/connector/invitation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (i *invitationResourceType) List(ctx context.Context, parentID *v2.Resource
if isNotFoundError(resp) {
return nil, "", nil, nil
}
return nil, "", nil, fmt.Errorf("github-connector: ListPendingOrgInvitatioins failed: %w", err)
return nil, "", nil, wrapGitHubError(err, resp, "github-connector: failed to list pending org invitations")
}

restApiRateLimit, err := extractRateLimitData(resp)
Expand Down Expand Up @@ -140,7 +140,7 @@ func (i *invitationResourceType) CreateAccount(
Email: params.email,
})
if err != nil {
return nil, nil, nil, fmt.Errorf("github-connectorv2: failed to invite user to org: %w", err)
return nil, nil, nil, wrapGitHubError(err, resp, "github-connector: failed to create org invitation")
}

restApiRateLimit, err := extractRateLimitData(resp)
Expand Down
55 changes: 22 additions & 33 deletions pkg/connector/org.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,7 @@ func (o *orgResourceType) List(

orgs, resp, err := o.client.Organizations.List(ctx, "", opts)
if err != nil {
if isRatelimited(resp) {
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
}
return nil, "", nil, fmt.Errorf("github-connector: failed to fetch org: %w", err)
return nil, "", nil, wrapGitHubError(err, resp, "github-connector: failed to fetch organizations")
}

nextPage, reqAnnos, err := parseResp(resp)
Expand All @@ -129,11 +126,7 @@ func (o *orgResourceType) List(
l.Warn("insufficient access to list org membership, skipping org", zap.String("org", org.GetLogin()))
continue
}

if isRatelimited(resp) {
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
}
return nil, "", nil, err
return nil, "", nil, wrapGitHubError(err, resp, "github-connector: failed to get org membership")
}

// Only sync orgs that we are an admin for
Expand Down Expand Up @@ -227,11 +220,7 @@ func (o *orgResourceType) Grants(
if isNotFoundError(resp) {
return nil, "", nil, uhttp.WrapErrors(codes.NotFound, fmt.Sprintf("org: %s not found", orgName))
}
errMsg := "github-connectorv2: failed to list org members"
if isRatelimited(resp) {
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
}
return nil, "", nil, fmt.Errorf("%s: %w", errMsg, err)
return nil, "", nil, wrapGitHubError(err, resp, "github-connector: failed to list org members")
}

var nextPage string
Expand Down Expand Up @@ -294,9 +283,9 @@ func (o *orgResourceType) Grant(ctx context.Context, principal *v2.Resource, en
return nil, err
}

user, _, err := o.client.Users.GetByID(ctx, principalID)
user, resp, err := o.client.Users.GetByID(ctx, principalID)
if err != nil {
return nil, fmt.Errorf("github-connectorv2: failed to get user: %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to get user")
}

requestedRole := ""
Expand All @@ -309,21 +298,21 @@ func (o *orgResourceType) Grant(ctx context.Context, principal *v2.Resource, en
return nil, fmt.Errorf("github-connectorv2: invalid entitlement id: %s", en.Id)
}

isMember, _, err := o.client.Organizations.IsMember(ctx, orgName, user.GetLogin())
isMember, resp, err := o.client.Organizations.IsMember(ctx, orgName, user.GetLogin())
if err != nil {
return nil, fmt.Errorf("github-connectorv2: failed to get org membership: %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to check org membership")
}

// TODO: check existing invitations. Duplicate invitations aren't allowed, so this will fail with 4xx from github.

// If user isn't a member, invite them to the org with the requested role
if !isMember {
_, _, err = o.client.Organizations.CreateOrgInvitation(ctx, orgName, &github.CreateOrgInvitationOptions{
_, resp, err = o.client.Organizations.CreateOrgInvitation(ctx, orgName, &github.CreateOrgInvitationOptions{
InviteeID: user.ID,
Role: &requestedRole,
})
if err != nil {
return nil, fmt.Errorf("github-connectorv2: failed to invite user to org: %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to invite user to org")
}
return nil, nil
}
Expand All @@ -334,9 +323,9 @@ func (o *orgResourceType) Grant(ctx context.Context, principal *v2.Resource, en
}

// If the user is a member, check to see what role they have
membership, _, err := o.client.Organizations.GetOrgMembership(ctx, user.GetLogin(), orgName)
membership, resp, err := o.client.Organizations.GetOrgMembership(ctx, user.GetLogin(), orgName)
if err != nil {
return nil, fmt.Errorf("github-connectorv2: failed to get org membership: %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to get org membership")
}

// Skip if user already has requested role
Expand All @@ -346,9 +335,9 @@ func (o *orgResourceType) Grant(ctx context.Context, principal *v2.Resource, en
}

// User is a member but grant is for admin, so make them an admin.
_, _, err = o.client.Organizations.EditOrgMembership(ctx, user.GetLogin(), orgName, &github.Membership{Role: github.Ptr(orgRoleAdmin)})
_, resp, err = o.client.Organizations.EditOrgMembership(ctx, user.GetLogin(), orgName, &github.Membership{Role: github.Ptr(orgRoleAdmin)})
if err != nil {
return nil, fmt.Errorf("github-connectorv2: failed to make user an admin : %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to make user an admin")
}

return nil, nil
Expand Down Expand Up @@ -386,31 +375,31 @@ func (o *orgResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotati
return nil, err
}

user, _, err := o.client.Users.GetByID(ctx, principalID)
user, resp, err := o.client.Users.GetByID(ctx, principalID)
if err != nil {
return nil, fmt.Errorf("github-connectorv2: failed to get user: %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to get user")
}

membership, _, err := o.client.Organizations.GetOrgMembership(ctx, user.GetLogin(), orgName)
membership, resp, err := o.client.Organizations.GetOrgMembership(ctx, user.GetLogin(), orgName)
if err != nil {
return nil, fmt.Errorf("github-connectorv2: failed to get org membership: %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to get org membership")
}

if membership.GetState() != "active" {
return nil, fmt.Errorf("github-connectorv2: user is not an active member of the org")
}

if en.Id == memberRoleID {
_, err = o.client.Organizations.RemoveOrgMembership(ctx, user.GetLogin(), orgName)
resp, err = o.client.Organizations.RemoveOrgMembership(ctx, user.GetLogin(), orgName)
if err != nil {
return nil, fmt.Errorf("github-connectorv2: failed to revoke org membership from user: %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to revoke org membership from user")
}
return nil, nil
}

_, _, err = o.client.Organizations.EditOrgMembership(ctx, user.GetLogin(), orgName, &github.Membership{Role: github.Ptr(orgRoleMember)})
_, resp, err = o.client.Organizations.EditOrgMembership(ctx, user.GetLogin(), orgName, &github.Membership{Role: github.Ptr(orgRoleMember)})
if err != nil {
return nil, fmt.Errorf("github-connectorv2: failed to revoke org admin from user: %w", err)
return nil, wrapGitHubError(err, resp, "github-connector: failed to revoke org admin from user")
}

return nil, nil
Expand Down Expand Up @@ -449,7 +438,7 @@ func (o *orgResourceType) listOrganizationsFromAppInstallations(
for orgName := range o.orgs {
org, resp, err = o.client.Organizations.Get(ctx, orgName)
if err != nil {
return nil, "", nil, fmt.Errorf("github-connector: failed to fetch organization: %w", err)
return nil, "", nil, wrapGitHubError(err, resp, "github-connector: failed to fetch organization")
}
}

Expand Down
14 changes: 3 additions & 11 deletions pkg/connector/org_role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,9 @@ import (
"github.com/conductorone/baton-sdk/pkg/types/entitlement"
"github.com/conductorone/baton-sdk/pkg/types/grant"
"github.com/conductorone/baton-sdk/pkg/types/resource"
"github.com/conductorone/baton-sdk/pkg/uhttp"
"github.com/google/go-github/v69/github"
"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
"go.uber.org/zap"
"google.golang.org/grpc/codes"
)

type OrganizationRole struct {
Expand Down Expand Up @@ -90,10 +88,7 @@ func (o *orgRoleResourceType) List(
// Return empty list with no error to indicate we skipped this resource
return nil, "", nil, nil
}
if isRatelimited(resp) {
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
}
return nil, "", nil, fmt.Errorf("failed to list organization roles: %w", err)
return nil, "", nil, wrapGitHubError(err, resp, "github-connector: failed to list organization roles")
}

var ret []*v2.Resource
Expand Down Expand Up @@ -180,7 +175,7 @@ func (o *orgRoleResourceType) Grants(
}
return rv, pageToken, nil, nil
}
return nil, "", nil, fmt.Errorf("failed to list role users: %w", err)
return nil, "", nil, wrapGitHubError(err, resp, "github-connector: failed to list users assigned to org role")
}
nextPage, respAnnos, err := parseResp(resp)
if err != nil {
Expand Down Expand Up @@ -227,10 +222,7 @@ func (o *orgRoleResourceType) Grants(
}
return nil, pageToken, nil, nil
}
if isRatelimited(resp) {
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
}
return nil, "", nil, fmt.Errorf("failed to list role teams: %w", err)
return nil, "", nil, wrapGitHubError(err, resp, "github-connector: failed to list teams assigned to org role")
}

nextPage, respAnnos, err := parseResp(resp)
Expand Down
Loading