Skip to content

Commit 0a6760d

Browse files
mattdhollowaymattdholloway
authored
Merge branch 'main' into raw-client-error-annotation
2 parents 349877f + bc5d08d commit 0a6760d

15 files changed

+155
-79
lines changed

‎pkg/errors/error.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,10 @@ func NewGitHubGraphQLErrorResponse(ctx context.Context, message string, err erro
163163
return utils.NewToolResultErrorFromErr(message, err)
164164
}
165165

166-
func NewGitHubRawAPIErrorResponse(ctx context.Context, message string, resp *http.Response, err error) *mcp.CallToolResult {
167-
rawAPIErr := newGitHubRawAPIError(message, resp, err)
168-
if ctx != nil {
169-
_, _ = addRawAPIErrorToContext(ctx, rawAPIErr) // Explicitly ignore error for graceful handling
170-
}
171-
return utils.NewToolResultErrorFromErr(message, err)
166+
// NewGitHubAPIStatusErrorResponse handles cases where the API call succeeds (err == nil)
167+
// but returns an unexpected HTTP status code. It creates a synthetic error from the
168+
// status code and response body, then records it in context for observability tracking.
169+
func NewGitHubAPIStatusErrorResponse(ctx context.Context, message string, resp *github.Response, body []byte) *mcp.CallToolResult {
170+
err := fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(body))
171+
return NewGitHubAPIErrorResponse(ctx, message, resp, err)
172172
}

‎pkg/errors/error_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,33 @@ func TestGitHubErrorContext(t *testing.T) {
287287
assert.Equal(t, originalErr, gqlError.Err)
288288
})
289289

290+
t.Run("NewGitHubAPIStatusErrorResponse creates MCP error result from status code", func(t *testing.T) {
291+
// Given a context with GitHub error tracking enabled
292+
ctx := ContextWithGitHubErrors(context.Background())
293+
294+
resp := &github.Response{Response: &http.Response{StatusCode: 422}}
295+
body := []byte(`{"message": "Validation Failed"}`)
296+
297+
// When we create a status error response
298+
result := NewGitHubAPIStatusErrorResponse(ctx, "failed to create issue", resp, body)
299+
300+
// Then it should return an MCP error result
301+
require.NotNil(t, result)
302+
assert.True(t, result.IsError)
303+
304+
// And the error should be stored in the context
305+
apiErrors, err := GetGitHubAPIErrors(ctx)
306+
require.NoError(t, err)
307+
require.Len(t, apiErrors, 1)
308+
309+
apiError := apiErrors[0]
310+
assert.Equal(t, "failed to create issue", apiError.Message)
311+
assert.Equal(t, resp, apiError.Response)
312+
// The synthetic error should contain the status code and body
313+
assert.Contains(t, apiError.Err.Error(), "unexpected status 422")
314+
assert.Contains(t, apiError.Err.Error(), "Validation Failed")
315+
})
316+
290317
t.Run("NewGitHubAPIErrorToCtx with uninitialized context does not error", func(t *testing.T) {
291318
// Given a regular context without GitHub error tracking initialized
292319
ctx := context.Background()

‎pkg/github/code_scanning.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package github
33
import (
44
"context"
55
"encoding/json"
6-
"fmt"
76
"io"
87
"net/http"
98

@@ -80,7 +79,7 @@ func GetCodeScanningAlert(t translations.TranslationHelperFunc) inventory.Server
8079
if err != nil {
8180
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
8281
}
83-
return utils.NewToolResultError(fmt.Sprintf("failed to get alert: %s", string(body))), nil, nil
82+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get alert", resp, body), nil, nil
8483
}
8584

8685
r, err := json.Marshal(alert)
@@ -184,7 +183,7 @@ func ListCodeScanningAlerts(t translations.TranslationHelperFunc) inventory.Serv
184183
if err != nil {
185184
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
186185
}
187-
return utils.NewToolResultError(fmt.Sprintf("failed to list alerts: %s", string(body))), nil, nil
186+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list alerts", resp, body), nil, nil
188187
}
189188

190189
r, err := json.Marshal(alerts)

‎pkg/github/dependabot.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func GetDependabotAlert(t translations.TranslationHelperFunc) inventory.ServerTo
8080
if err != nil {
8181
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, err
8282
}
83-
return utils.NewToolResultError(fmt.Sprintf("failed to get alert: %s", string(body))), nil, nil
83+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get alert", resp, body), nil, nil
8484
}
8585

8686
r, err := json.Marshal(alert)
@@ -172,7 +172,7 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
172172
if err != nil {
173173
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, err
174174
}
175-
return utils.NewToolResultError(fmt.Sprintf("failed to list alerts: %s", string(body))), nil, nil
175+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list alerts", resp, body), nil, nil
176176
}
177177

178178
r, err := json.Marshal(alerts)

‎pkg/github/gists.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func ListGists(t translations.TranslationHelperFunc) inventory.ServerTool {
9090
if err != nil {
9191
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
9292
}
93-
return utils.NewToolResultError(fmt.Sprintf("failed to list gists: %s", string(body))), nil, nil
93+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list gists", resp, body), nil, nil
9494
}
9595

9696
r, err := json.Marshal(gists)
@@ -149,7 +149,7 @@ func GetGist(t translations.TranslationHelperFunc) inventory.ServerTool {
149149
if err != nil {
150150
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
151151
}
152-
return utils.NewToolResultError(fmt.Sprintf("failed to get gist: %s", string(body))), nil, nil
152+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get gist", resp, body), nil, nil
153153
}
154154

155155
r, err := json.Marshal(gist)
@@ -248,7 +248,7 @@ func CreateGist(t translations.TranslationHelperFunc) inventory.ServerTool {
248248
if err != nil {
249249
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
250250
}
251-
return utils.NewToolResultError(fmt.Sprintf("failed to create gist: %s", string(body))), nil, nil
251+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create gist", resp, body), nil, nil
252252
}
253253

254254
minimalResponse := MinimalResponse{
@@ -350,7 +350,7 @@ func UpdateGist(t translations.TranslationHelperFunc) inventory.ServerTool {
350350
if err != nil {
351351
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
352352
}
353-
return utils.NewToolResultError(fmt.Sprintf("failed to update gist: %s", string(body))), nil, nil
353+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to update gist", resp, body), nil, nil
354354
}
355355

356356
minimalResponse := MinimalResponse{

‎pkg/github/issues.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ func GetIssue(ctx context.Context, client *github.Client, cache *lockdown.RepoAc
340340
if err != nil {
341341
return nil, fmt.Errorf("failed to read response body: %w", err)
342342
}
343-
return utils.NewToolResultError(fmt.Sprintf("failed to get issue: %s", string(body))), nil
343+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get issue", resp, body), nil
344344
}
345345

346346
if flags.LockdownMode {
@@ -396,7 +396,7 @@ func GetIssueComments(ctx context.Context, client *github.Client, cache *lockdow
396396
if err != nil {
397397
return nil, fmt.Errorf("failed to read response body: %w", err)
398398
}
399-
return utils.NewToolResultError(fmt.Sprintf("failed to get issue comments: %s", string(body))), nil
399+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get issue comments", resp, body), nil
400400
}
401401
if flags.LockdownMode {
402402
if cache == nil {
@@ -455,7 +455,7 @@ func GetSubIssues(ctx context.Context, client *github.Client, cache *lockdown.Re
455455
if err != nil {
456456
return nil, fmt.Errorf("failed to read response body: %w", err)
457457
}
458-
return utils.NewToolResultError(fmt.Sprintf("failed to list sub-issues: %s", string(body))), nil
458+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list sub-issues", resp, body), nil
459459
}
460460

461461
if featureFlags.LockdownMode {
@@ -588,7 +588,7 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool {
588588
if err != nil {
589589
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
590590
}
591-
return utils.NewToolResultError(fmt.Sprintf("failed to list issue types: %s", string(body))), nil, nil
591+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list issue types", resp, body), nil, nil
592592
}
593593

594594
r, err := json.Marshal(issueTypes)
@@ -673,7 +673,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool
673673
if err != nil {
674674
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
675675
}
676-
return utils.NewToolResultError(fmt.Sprintf("failed to create comment: %s", string(body))), nil, nil
676+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create comment", resp, body), nil, nil
677677
}
678678

679679
r, err := json.Marshal(createdComment)
@@ -823,7 +823,7 @@ func AddSubIssue(ctx context.Context, client *github.Client, owner string, repo
823823
if err != nil {
824824
return nil, fmt.Errorf("failed to read response body: %w", err)
825825
}
826-
return utils.NewToolResultError(fmt.Sprintf("failed to add sub-issue: %s", string(body))), nil
826+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add sub-issue", resp, body), nil
827827
}
828828

829829
r, err := json.Marshal(subIssue)
@@ -855,7 +855,7 @@ func RemoveSubIssue(ctx context.Context, client *github.Client, owner string, re
855855
if err != nil {
856856
return nil, fmt.Errorf("failed to read response body: %w", err)
857857
}
858-
return utils.NewToolResultError(fmt.Sprintf("failed to remove sub-issue: %s", string(body))), nil
858+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to remove sub-issue", resp, body), nil
859859
}
860860

861861
r, err := json.Marshal(subIssue)
@@ -904,7 +904,7 @@ func ReprioritizeSubIssue(ctx context.Context, client *github.Client, owner stri
904904
if err != nil {
905905
return nil, fmt.Errorf("failed to read response body: %w", err)
906906
}
907-
return utils.NewToolResultError(fmt.Sprintf("failed to reprioritize sub-issue: %s", string(body))), nil
907+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to reprioritize sub-issue", resp, body), nil
908908
}
909909

910910
r, err := json.Marshal(subIssue)
@@ -1195,7 +1195,7 @@ func CreateIssue(ctx context.Context, client *github.Client, owner string, repo
11951195
if err != nil {
11961196
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil
11971197
}
1198-
return utils.NewToolResultError(fmt.Sprintf("failed to create issue: %s", string(body))), nil
1198+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create issue", resp, body), nil
11991199
}
12001200

12011201
// Return minimal response with just essential information
@@ -1256,7 +1256,7 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4
12561256
if err != nil {
12571257
return nil, fmt.Errorf("failed to read response body: %w", err)
12581258
}
1259-
return utils.NewToolResultError(fmt.Sprintf("failed to update issue: %s", string(body))), nil
1259+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to update issue", resp, body), nil
12601260
}
12611261

12621262
// Use GraphQL API for state updates

‎pkg/github/notifications.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func ListNotifications(t translations.TranslationHelperFunc) inventory.ServerToo
147147
if err != nil {
148148
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
149149
}
150-
return utils.NewToolResultError(fmt.Sprintf("failed to get notifications: %s", string(body))), nil, nil
150+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get notifications", resp, body), nil, nil
151151
}
152152

153153
// Marshal response to JSON
@@ -236,7 +236,7 @@ func DismissNotification(t translations.TranslationHelperFunc) inventory.ServerT
236236
if err != nil {
237237
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
238238
}
239-
return utils.NewToolResultError(fmt.Sprintf("failed to mark notification as %s: %s", state, string(body))), nil, nil
239+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, fmt.Sprintf("failed to mark notification as %s", state), resp, body), nil, nil
240240
}
241241

242242
return utils.NewToolResultText(fmt.Sprintf("Notification marked as %s", state)), nil, nil
@@ -329,7 +329,7 @@ func MarkAllNotificationsRead(t translations.TranslationHelperFunc) inventory.Se
329329
if err != nil {
330330
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
331331
}
332-
return utils.NewToolResultError(fmt.Sprintf("failed to mark all notifications as read: %s", string(body))), nil, nil
332+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to mark all notifications as read", resp, body), nil, nil
333333
}
334334

335335
return utils.NewToolResultText("All notifications marked as read"), nil, nil
@@ -387,7 +387,7 @@ func GetNotificationDetails(t translations.TranslationHelperFunc) inventory.Serv
387387
if err != nil {
388388
return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil
389389
}
390-
return utils.NewToolResultError(fmt.Sprintf("failed to get notification details: %s", string(body))), nil, nil
390+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get notification details", resp, body), nil, nil
391391
}
392392

393393
r, err := json.Marshal(thread)
@@ -481,7 +481,7 @@ func ManageNotificationSubscription(t translations.TranslationHelperFunc) invent
481481

482482
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
483483
body, _ := io.ReadAll(resp.Body)
484-
return utils.NewToolResultError(fmt.Sprintf("failed to %s notification subscription: %s", action, string(body))), nil, nil
484+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, fmt.Sprintf("failed to %s notification subscription", action), resp, body), nil, nil
485485
}
486486

487487
if action == NotificationActionDelete {
@@ -589,7 +589,7 @@ func ManageRepositoryNotificationSubscription(t translations.TranslationHelperFu
589589
// Handle non-2xx status codes
590590
if resp != nil && (resp.StatusCode < 200 || resp.StatusCode >= 300) {
591591
body, _ := io.ReadAll(resp.Body)
592-
return utils.NewToolResultError(fmt.Sprintf("failed to %s repository subscription: %s", action, string(body))), nil, nil
592+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, fmt.Sprintf("failed to %s repository subscription", action), resp, body), nil, nil
593593
}
594594

595595
if action == RepositorySubscriptionActionDelete {

‎pkg/github/projects.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ func GetProject(t translations.TranslationHelperFunc) inventory.ServerTool {
219219
if err != nil {
220220
return nil, nil, fmt.Errorf("failed to read response body: %w", err)
221221
}
222-
return utils.NewToolResultError(fmt.Sprintf("failed to get project: %s", string(body))), nil, nil
222+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get project", resp, body), nil, nil
223223
}
224224

225225
minimalProject := convertToMinimalProject(project)
@@ -423,7 +423,7 @@ func GetProjectField(t translations.TranslationHelperFunc) inventory.ServerTool
423423
if err != nil {
424424
return nil, nil, fmt.Errorf("failed to read response body: %w", err)
425425
}
426-
return utils.NewToolResultError(fmt.Sprintf("failed to get project field: %s", string(body))), nil, nil
426+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get project field", resp, body), nil, nil
427427
}
428428
r, err := json.Marshal(projectField)
429429
if err != nil {
@@ -782,7 +782,7 @@ func AddProjectItem(t translations.TranslationHelperFunc) inventory.ServerTool {
782782
if err != nil {
783783
return nil, nil, fmt.Errorf("failed to read response body: %w", err)
784784
}
785-
return utils.NewToolResultError(fmt.Sprintf("%s: %s", ProjectAddFailedError, string(body))), nil, nil
785+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, ProjectAddFailedError, resp, body), nil, nil
786786
}
787787
r, err := json.Marshal(addedItem)
788788
if err != nil {
@@ -896,7 +896,7 @@ func UpdateProjectItem(t translations.TranslationHelperFunc) inventory.ServerToo
896896
if err != nil {
897897
return nil, nil, fmt.Errorf("failed to read response body: %w", err)
898898
}
899-
return utils.NewToolResultError(fmt.Sprintf("%s: %s", ProjectUpdateFailedError, string(body))), nil, nil
899+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, ProjectUpdateFailedError, resp, body), nil, nil
900900
}
901901
r, err := json.Marshal(updatedItem)
902902
if err != nil {
@@ -988,7 +988,7 @@ func DeleteProjectItem(t translations.TranslationHelperFunc) inventory.ServerToo
988988
if err != nil {
989989
return nil, nil, fmt.Errorf("failed to read response body: %w", err)
990990
}
991-
return utils.NewToolResultError(fmt.Sprintf("%s: %s", ProjectDeleteFailedError, string(body))), nil, nil
991+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, ProjectDeleteFailedError, resp, body), nil, nil
992992
}
993993
return utils.NewToolResultText("project item successfully deleted"), nil, nil
994994
}

0 commit comments

Comments
 (0)