From e1bc0048d9f2ed09c2faf71f1b230fe9b39c04bb Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 17 Dec 2025 13:42:34 +0100 Subject: [PATCH] fix: track HTTP status code errors in context for observability Add NewGitHubAPIStatusErrorResponse helper function to properly track GitHub API errors when the API call succeeds but returns an unexpected HTTP status code (e.g., 404, 422, 500). Previously, these errors were returned via utils.NewToolResultError which bypasses the context-based error tracking used by the remote server's error_categorizer.go for observability metrics. This resulted in 100% tool call success rates in observability even when errors occurred. The fix adds a new helper function that: 1. Creates a synthetic error from the status code and response body 2. Records the error in context via NewGitHubAPIErrorResponse 3. Returns the MCP error result to the client Updated all tool files to use the new pattern for status code errors: - pullrequests.go: 12 fixes - repositories.go: 18 fixes - issues.go: 10 fixes - notifications.go: 6 fixes - projects.go: 5 fixes - search.go: 3 fixes - search_utils.go: 1 fix - gists.go: 4 fixes - code_scanning.go: 2 fixes - dependabot.go: 2 fixes - secret_scanning.go: 2 fixes - security_advisories.go: 4 fixes Total: ~69 error paths now properly tracked. Note: Parameter validation errors (RequiredParam failures) and internal I/O errors (io.ReadAll failures) intentionally continue to use utils.NewToolResultError as they are not GitHub API errors. --- pkg/errors/error.go | 8 +++++++ pkg/errors/error_test.go | 27 ++++++++++++++++++++++ pkg/github/code_scanning.go | 5 ++-- pkg/github/dependabot.go | 4 ++-- pkg/github/gists.go | 8 +++---- pkg/github/issues.go | 20 ++++++++-------- pkg/github/notifications.go | 12 +++++----- pkg/github/projects.go | 10 ++++---- pkg/github/pullrequests.go | 26 ++++++++++----------- pkg/github/repositories.go | 38 +++++++++++++++---------------- pkg/github/search.go | 6 ++--- pkg/github/search_utils.go | 3 ++- pkg/github/secret_scanning.go | 4 ++-- pkg/github/security_advisories.go | 9 ++++---- 14 files changed, 108 insertions(+), 72 deletions(-) diff --git a/pkg/errors/error.go b/pkg/errors/error.go index be2cf58f9..095f8d5b7 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -124,3 +124,11 @@ func NewGitHubGraphQLErrorResponse(ctx context.Context, message string, err erro } return utils.NewToolResultErrorFromErr(message, err) } + +// NewGitHubAPIStatusErrorResponse handles cases where the API call succeeds (err == nil) +// but returns an unexpected HTTP status code. It creates a synthetic error from the +// status code and response body, then records it in context for observability tracking. +func NewGitHubAPIStatusErrorResponse(ctx context.Context, message string, resp *github.Response, body []byte) *mcp.CallToolResult { + err := fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(body)) + return NewGitHubAPIErrorResponse(ctx, message, resp, err) +} diff --git a/pkg/errors/error_test.go b/pkg/errors/error_test.go index 0d7aa6afa..b5ef40596 100644 --- a/pkg/errors/error_test.go +++ b/pkg/errors/error_test.go @@ -231,6 +231,33 @@ func TestGitHubErrorContext(t *testing.T) { assert.Equal(t, originalErr, gqlError.Err) }) + t.Run("NewGitHubAPIStatusErrorResponse creates MCP error result from status code", func(t *testing.T) { + // Given a context with GitHub error tracking enabled + ctx := ContextWithGitHubErrors(context.Background()) + + resp := &github.Response{Response: &http.Response{StatusCode: 422}} + body := []byte(`{"message": "Validation Failed"}`) + + // When we create a status error response + result := NewGitHubAPIStatusErrorResponse(ctx, "failed to create issue", resp, body) + + // Then it should return an MCP error result + require.NotNil(t, result) + assert.True(t, result.IsError) + + // And the error should be stored in the context + apiErrors, err := GetGitHubAPIErrors(ctx) + require.NoError(t, err) + require.Len(t, apiErrors, 1) + + apiError := apiErrors[0] + assert.Equal(t, "failed to create issue", apiError.Message) + assert.Equal(t, resp, apiError.Response) + // The synthetic error should contain the status code and body + assert.Contains(t, apiError.Err.Error(), "unexpected status 422") + assert.Contains(t, apiError.Err.Error(), "Validation Failed") + }) + t.Run("NewGitHubAPIErrorToCtx with uninitialized context does not error", func(t *testing.T) { // Given a regular context without GitHub error tracking initialized ctx := context.Background() diff --git a/pkg/github/code_scanning.go b/pkg/github/code_scanning.go index 009d26623..632fcddf9 100644 --- a/pkg/github/code_scanning.go +++ b/pkg/github/code_scanning.go @@ -3,7 +3,6 @@ package github import ( "context" "encoding/json" - "fmt" "io" "net/http" @@ -80,7 +79,7 @@ func GetCodeScanningAlert(t translations.TranslationHelperFunc) inventory.Server if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to get alert: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get alert", resp, body), nil, nil } r, err := json.Marshal(alert) @@ -184,7 +183,7 @@ func ListCodeScanningAlerts(t translations.TranslationHelperFunc) inventory.Serv if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to list alerts: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list alerts", resp, body), nil, nil } r, err := json.Marshal(alerts) diff --git a/pkg/github/dependabot.go b/pkg/github/dependabot.go index 50f39396a..c1a4ce46c 100644 --- a/pkg/github/dependabot.go +++ b/pkg/github/dependabot.go @@ -80,7 +80,7 @@ func GetDependabotAlert(t translations.TranslationHelperFunc) inventory.ServerTo if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, err } - return utils.NewToolResultError(fmt.Sprintf("failed to get alert: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get alert", resp, body), nil, nil } r, err := json.Marshal(alert) @@ -172,7 +172,7 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, err } - return utils.NewToolResultError(fmt.Sprintf("failed to list alerts: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list alerts", resp, body), nil, nil } r, err := json.Marshal(alerts) diff --git a/pkg/github/gists.go b/pkg/github/gists.go index eaee96474..511d7ea89 100644 --- a/pkg/github/gists.go +++ b/pkg/github/gists.go @@ -90,7 +90,7 @@ func ListGists(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to list gists: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list gists", resp, body), nil, nil } r, err := json.Marshal(gists) @@ -149,7 +149,7 @@ func GetGist(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to get gist: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get gist", resp, body), nil, nil } r, err := json.Marshal(gist) @@ -248,7 +248,7 @@ func CreateGist(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to create gist: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create gist", resp, body), nil, nil } minimalResponse := MinimalResponse{ @@ -350,7 +350,7 @@ func UpdateGist(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to update gist: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to update gist", resp, body), nil, nil } minimalResponse := MinimalResponse{ diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 806dc5701..32562ad53 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -340,7 +340,7 @@ func GetIssue(ctx context.Context, client *github.Client, cache *lockdown.RepoAc if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get issue: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get issue", resp, body), nil } if flags.LockdownMode { @@ -396,7 +396,7 @@ func GetIssueComments(ctx context.Context, client *github.Client, cache *lockdow if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get issue comments: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get issue comments", resp, body), nil } if flags.LockdownMode { if cache == nil { @@ -455,7 +455,7 @@ func GetSubIssues(ctx context.Context, client *github.Client, cache *lockdown.Re if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to list sub-issues: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list sub-issues", resp, body), nil } if featureFlags.LockdownMode { @@ -588,7 +588,7 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to list issue types: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list issue types", resp, body), nil, nil } r, err := json.Marshal(issueTypes) @@ -673,7 +673,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to create comment: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create comment", resp, body), nil, nil } r, err := json.Marshal(createdComment) @@ -823,7 +823,7 @@ func AddSubIssue(ctx context.Context, client *github.Client, owner string, repo if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to add sub-issue: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add sub-issue", resp, body), nil } r, err := json.Marshal(subIssue) @@ -855,7 +855,7 @@ func RemoveSubIssue(ctx context.Context, client *github.Client, owner string, re if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to remove sub-issue: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to remove sub-issue", resp, body), nil } r, err := json.Marshal(subIssue) @@ -904,7 +904,7 @@ func ReprioritizeSubIssue(ctx context.Context, client *github.Client, owner stri if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to reprioritize sub-issue: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to reprioritize sub-issue", resp, body), nil } r, err := json.Marshal(subIssue) @@ -1195,7 +1195,7 @@ func CreateIssue(ctx context.Context, client *github.Client, owner string, repo if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil } - return utils.NewToolResultError(fmt.Sprintf("failed to create issue: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create issue", resp, body), nil } // Return minimal response with just essential information @@ -1256,7 +1256,7 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to update issue: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to update issue", resp, body), nil } // Use GraphQL API for state updates diff --git a/pkg/github/notifications.go b/pkg/github/notifications.go index 7ed1e0a9c..c6e18529f 100644 --- a/pkg/github/notifications.go +++ b/pkg/github/notifications.go @@ -147,7 +147,7 @@ func ListNotifications(t translations.TranslationHelperFunc) inventory.ServerToo if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to get notifications: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get notifications", resp, body), nil, nil } // Marshal response to JSON @@ -236,7 +236,7 @@ func DismissNotification(t translations.TranslationHelperFunc) inventory.ServerT if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to mark notification as %s: %s", state, string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, fmt.Sprintf("failed to mark notification as %s", state), resp, body), nil, nil } return utils.NewToolResultText(fmt.Sprintf("Notification marked as %s", state)), nil, nil @@ -329,7 +329,7 @@ func MarkAllNotificationsRead(t translations.TranslationHelperFunc) inventory.Se if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to mark all notifications as read: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to mark all notifications as read", resp, body), nil, nil } return utils.NewToolResultText("All notifications marked as read"), nil, nil @@ -387,7 +387,7 @@ func GetNotificationDetails(t translations.TranslationHelperFunc) inventory.Serv if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to get notification details: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get notification details", resp, body), nil, nil } r, err := json.Marshal(thread) @@ -481,7 +481,7 @@ func ManageNotificationSubscription(t translations.TranslationHelperFunc) invent if resp.StatusCode < 200 || resp.StatusCode >= 300 { body, _ := io.ReadAll(resp.Body) - return utils.NewToolResultError(fmt.Sprintf("failed to %s notification subscription: %s", action, string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, fmt.Sprintf("failed to %s notification subscription", action), resp, body), nil, nil } if action == NotificationActionDelete { @@ -589,7 +589,7 @@ func ManageRepositoryNotificationSubscription(t translations.TranslationHelperFu // Handle non-2xx status codes if resp != nil && (resp.StatusCode < 200 || resp.StatusCode >= 300) { body, _ := io.ReadAll(resp.Body) - return utils.NewToolResultError(fmt.Sprintf("failed to %s repository subscription: %s", action, string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, fmt.Sprintf("failed to %s repository subscription", action), resp, body), nil, nil } if action == RepositorySubscriptionActionDelete { diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 60f9c2d10..d33ac5780 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -219,7 +219,7 @@ func GetProject(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get project: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get project", resp, body), nil, nil } minimalProject := convertToMinimalProject(project) @@ -423,7 +423,7 @@ func GetProjectField(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get project field: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get project field", resp, body), nil, nil } r, err := json.Marshal(projectField) if err != nil { @@ -782,7 +782,7 @@ func AddProjectItem(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("%s: %s", ProjectAddFailedError, string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, ProjectAddFailedError, resp, body), nil, nil } r, err := json.Marshal(addedItem) if err != nil { @@ -896,7 +896,7 @@ func UpdateProjectItem(t translations.TranslationHelperFunc) inventory.ServerToo if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("%s: %s", ProjectUpdateFailedError, string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, ProjectUpdateFailedError, resp, body), nil, nil } r, err := json.Marshal(updatedItem) if err != nil { @@ -988,7 +988,7 @@ func DeleteProjectItem(t translations.TranslationHelperFunc) inventory.ServerToo if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("%s: %s", ProjectDeleteFailedError, string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, ProjectDeleteFailedError, resp, body), nil, nil } return utils.NewToolResultText("project item successfully deleted"), nil, nil } diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 78b841356..52e2de5b0 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -150,7 +150,7 @@ func GetPullRequest(ctx context.Context, client *github.Client, cache *lockdown. if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request", resp, body), nil } // sanitize title/body on response @@ -209,7 +209,7 @@ func GetPullRequestDiff(ctx context.Context, client *github.Client, owner, repo if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get pull request diff: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request diff", resp, body), nil } defer func() { _ = resp.Body.Close() }() @@ -234,7 +234,7 @@ func GetPullRequestStatus(ctx context.Context, client *github.Client, owner, rep if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request", resp, body), nil } // Get combined status for the head SHA @@ -253,7 +253,7 @@ func GetPullRequestStatus(ctx context.Context, client *github.Client, owner, rep if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get combined status: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get combined status", resp, body), nil } r, err := json.Marshal(status) @@ -284,7 +284,7 @@ func GetPullRequestFiles(ctx context.Context, client *github.Client, owner, repo if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get pull request files: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request files", resp, body), nil } r, err := json.Marshal(files) @@ -436,7 +436,7 @@ func GetPullRequestReviews(ctx context.Context, client *github.Client, cache *lo if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get pull request reviews: %s", string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request reviews", resp, body), nil } if ff.LockdownMode { @@ -589,7 +589,7 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to create pull request: %s", string(bodyBytes))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create pull request", resp, bodyBytes), nil, nil } // Return minimal response with just essential information @@ -767,7 +767,7 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to update pull request: %s", string(bodyBytes))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to update pull request", resp, bodyBytes), nil, nil } } @@ -867,7 +867,7 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to request reviewers: %s", string(bodyBytes))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to request reviewers", resp, bodyBytes), nil, nil } } @@ -1021,7 +1021,7 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to list pull requests: %s", string(bodyBytes))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list pull requests", resp, bodyBytes), nil, nil } // sanitize title/body on each PR @@ -1143,7 +1143,7 @@ func MergePullRequest(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to merge pull request: %s", string(bodyBytes))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to merge pull request", resp, bodyBytes), nil, nil } r, err := json.Marshal(result) @@ -1302,7 +1302,7 @@ func UpdatePullRequestBranch(t translations.TranslationHelperFunc) inventory.Ser if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to update pull request branch: %s", string(bodyBytes))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to update pull request branch", resp, bodyBytes), nil, nil } r, err := json.Marshal(result) @@ -1907,7 +1907,7 @@ func RequestCopilotReview(t translations.TranslationHelperFunc) inventory.Server if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to request copilot review: %s", string(bodyBytes))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to request copilot review", resp, bodyBytes), nil, nil } // Return nothing on success, as there's not much value in returning the Pull Request itself diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 179c08475..ad400e0c8 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -100,7 +100,7 @@ func GetCommit(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get commit: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get commit", resp, body), nil, nil } // Convert to minimal commit @@ -206,7 +206,7 @@ func ListCommits(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to list commits: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list commits", resp, body), nil, nil } // Convert to minimal commits @@ -294,7 +294,7 @@ func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to list branches: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list branches", resp, body), nil, nil } // Convert to minimal branches @@ -487,7 +487,7 @@ If the SHA is not provided, the tool will attempt to acquire it by fetching the if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to create/update file: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create/update file", resp, body), nil, nil } r, err := json.Marshal(fileContent) @@ -601,7 +601,7 @@ func CreateRepository(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to create repository: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create repository", resp, body), nil, nil } // Return minimal response with just essential information @@ -877,7 +877,7 @@ func ForkRepository(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to fork repository: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to fork repository", resp, body), nil, nil } // Return minimal response with just essential information @@ -992,7 +992,7 @@ func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get commit: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get commit", resp, body), nil, nil } // Create a tree entry for the file deletion by setting SHA to nil @@ -1021,7 +1021,7 @@ func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to create tree: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create tree", resp, body), nil, nil } // Create a new commit with the new tree @@ -1045,7 +1045,7 @@ func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to create commit: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create commit", resp, body), nil, nil } // Update the branch reference to point to the new commit @@ -1068,7 +1068,7 @@ func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to update reference: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to update reference", resp, body), nil, nil } // Create a response similar to what the DeleteFile API would return @@ -1453,7 +1453,7 @@ func ListTags(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to list tags: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list tags", resp, body), nil, nil } r, err := json.Marshal(tags) @@ -1533,7 +1533,7 @@ func GetTag(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get tag reference: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get tag reference", resp, body), nil, nil } // Then get the tag object @@ -1552,7 +1552,7 @@ func GetTag(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get tag object: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get tag object", resp, body), nil, nil } r, err := json.Marshal(tagObj) @@ -1628,7 +1628,7 @@ func ListReleases(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to list releases: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list releases", resp, body), nil, nil } r, err := json.Marshal(releases) @@ -1695,7 +1695,7 @@ func GetLatestRelease(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get latest release: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get latest release", resp, body), nil, nil } r, err := json.Marshal(release) @@ -1773,7 +1773,7 @@ func GetReleaseByTag(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get release by tag: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get release by tag", resp, body), nil, nil } r, err := json.Marshal(release) @@ -2048,7 +2048,7 @@ func ListStarredRepositories(t translations.TranslationHelperFunc) inventory.Ser if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to list starred repositories: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list starred repositories", resp, body), nil, nil } // Convert to minimal format @@ -2146,7 +2146,7 @@ func StarRepository(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to star repository: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to star repository", resp, body), nil, nil } return utils.NewToolResultText(fmt.Sprintf("Successfully starred repository %s/%s", owner, repo)), nil, nil @@ -2212,7 +2212,7 @@ func UnstarRepository(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to unstar repository: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to unstar repository", resp, body), nil, nil } return utils.NewToolResultText(fmt.Sprintf("Successfully unstarred repository %s/%s", owner, repo)), nil, nil diff --git a/pkg/github/search.go b/pkg/github/search.go index 77182fbb6..ae4d69ae5 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -106,7 +106,7 @@ func SearchRepositories(t translations.TranslationHelperFunc) inventory.ServerTo if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to search repositories: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to search repositories", resp, body), nil, nil } // Return either minimal or full response based on parameter @@ -248,7 +248,7 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to search code: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to search code", resp, body), nil, nil } r, err := json.Marshal(result) @@ -314,7 +314,7 @@ func userOrOrgHandler(accountType string, deps ToolDependencies) mcp.ToolHandler if err != nil { return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil } - return utils.NewToolResultError(fmt.Sprintf("failed to search %ss: %s", accountType, string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, fmt.Sprintf("failed to search %ss", accountType), resp, body), nil, nil } minimalUsers := make([]MinimalUser, 0, len(result.Users)) diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index 9f7e41dec..1008200d1 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -8,6 +8,7 @@ import ( "net/http" "regexp" + ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/utils" "github.com/google/go-github/v79/github" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -104,7 +105,7 @@ func searchHandler( if err != nil { return utils.NewToolResultErrorFromErr(errorPrefix+": failed to read response body", err), nil } - return utils.NewToolResultError(fmt.Sprintf("%s: %s", errorPrefix, string(body))), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), nil } r, err := json.Marshal(result) diff --git a/pkg/github/secret_scanning.go b/pkg/github/secret_scanning.go index 0c7dc279f..4e3ced7e2 100644 --- a/pkg/github/secret_scanning.go +++ b/pkg/github/secret_scanning.go @@ -80,7 +80,7 @@ func GetSecretScanningAlert(t translations.TranslationHelperFunc) inventory.Serv if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get alert: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get alert", resp, body), nil, nil } r, err := json.Marshal(alert) @@ -175,7 +175,7 @@ func ListSecretScanningAlerts(t translations.TranslationHelperFunc) inventory.Se if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to list alerts: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list alerts", resp, body), nil, nil } r, err := json.Marshal(alerts) diff --git a/pkg/github/security_advisories.go b/pkg/github/security_advisories.go index 55994fc28..b35fb5a1c 100644 --- a/pkg/github/security_advisories.go +++ b/pkg/github/security_advisories.go @@ -7,6 +7,7 @@ import ( "io" "net/http" + ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/translations" "github.com/github/github-mcp-server/pkg/utils" @@ -193,7 +194,7 @@ func ListGlobalSecurityAdvisories(t translations.TranslationHelperFunc) inventor if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to list advisories: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list advisories", resp, body), nil, nil } r, err := json.Marshal(advisories) @@ -298,7 +299,7 @@ func ListRepositorySecurityAdvisories(t translations.TranslationHelperFunc) inve if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to list repository advisories: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list repository advisories", resp, body), nil, nil } r, err := json.Marshal(advisories) @@ -356,7 +357,7 @@ func GetGlobalSecurityAdvisory(t translations.TranslationHelperFunc) inventory.S if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to get advisory: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get advisory", resp, body), nil, nil } r, err := json.Marshal(advisory) @@ -452,7 +453,7 @@ func ListOrgRepositorySecurityAdvisories(t translations.TranslationHelperFunc) i if err != nil { return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return utils.NewToolResultError(fmt.Sprintf("failed to list organization repository advisories: %s", string(body))), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list organization repository advisories", resp, body), nil, nil } r, err := json.Marshal(advisories)