Skip to content

Commit 5608a1e

Browse files
omgitsadsomgitsads
committed
Return concrete error types for API errors
1 parent 7e026fc commit 5608a1e

File tree

11 files changed

+440
-78
lines changed

11 files changed

+440
-78
lines changed

‎internal/ghmcp/server.go‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import (
1414

1515
"github.com/github/github-mcp-server/pkg/github"
1616
mcplog "github.com/github/github-mcp-server/pkg/log"
17+
"github.com/github/github-mcp-server/pkg/toolsets"
18+
ts "github.com/github/github-mcp-server/pkg/toolsets"
1719
"github.com/github/github-mcp-server/pkg/translations"
1820
gogithub "github.com/google/go-github/v69/github"
1921
"github.com/mark3labs/mcp-go/mcp"
@@ -45,6 +47,9 @@ type MCPServerConfig struct {
4547

4648
// Translator provides translated text for the server tooling
4749
Translator translations.TranslationHelperFunc
50+
51+
// ToolHandlerWrapper is a function that can be used to wrap tool handlers
52+
ToolHandlerWrapper toolsets.ToolHandlerWrapper
4853
}
4954

5055
func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
@@ -120,6 +125,12 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
120125
getGQLClient,
121126
cfg.Translator,
122127
)
128+
129+
if cfg.ToolHandlerWrapper != nil {
130+
// If a custom tool handler wrapper is provided, use it
131+
toolsets.ToolHandlerWrapper = cfg.ToolHandlerWrapper
132+
}
133+
123134
if err != nil {
124135
return nil, fmt.Errorf("failed to initialize toolsets: %w", err)
125136
}
@@ -129,11 +140,11 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
129140

130141
// Register the tools with the server
131142
toolsets.RegisterTools(ghServer)
132-
context.RegisterTools(ghServer)
143+
context.RegisterTools(ghServer, ts.DefaultToolsetHandler)
133144

134145
if cfg.DynamicToolsets {
135146
dynamic := github.InitDynamicToolset(ghServer, toolsets, cfg.Translator)
136-
dynamic.RegisterTools(ghServer)
147+
dynamic.RegisterTools(ghServer, ts.DefaultToolsetHandler)
137148
}
138149

139150
return ghServer, nil

‎pkg/errors/error.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package errors
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/google/go-github/v69/github"
7+
)
8+
9+
type GitHubAPIError struct {
10+
Message string `json:"message"`
11+
Response *github.Response `json:"-"`
12+
Err error `json:"-"`
13+
}
14+
15+
func NewGitHubAPIError(message string, resp *github.Response, err error) *GitHubAPIError {
16+
return &GitHubAPIError{
17+
Message: message,
18+
Response: resp,
19+
Err: err,
20+
}
21+
}
22+
23+
func (e *GitHubAPIError) Error() string {
24+
return fmt.Errorf("%s: %w", e.Message, e.Err).Error()
25+
}
26+
27+
type GitHubGraphQLError struct {
28+
Message string `json:"message"`
29+
Err error `json:"-"`
30+
}
31+
32+
func NewGitHubGraphQLError(message string, err error) *GitHubGraphQLError {
33+
return &GitHubGraphQLError{
34+
Message: message,
35+
Err: err,
36+
}
37+
}
38+
39+
func (e *GitHubGraphQLError) Error() string {
40+
return fmt.Errorf("%s: %w", e.Message, e.Err).Error()
41+
}

‎pkg/github/code_scanning.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"io"
88
"net/http"
99

10+
ghErrors "github.com/github/github-mcp-server/pkg/errors"
1011
"github.com/github/github-mcp-server/pkg/translations"
1112
"github.com/google/go-github/v69/github"
1213
"github.com/mark3labs/mcp-go/mcp"
@@ -54,7 +55,11 @@ func GetCodeScanningAlert(getClient GetClientFn, t translations.TranslationHelpe
5455

5556
alert, resp, err := client.CodeScanning.GetAlert(ctx, owner, repo, int64(alertNumber))
5657
if err != nil {
57-
return nil, fmt.Errorf("failed to get alert: %w", err)
58+
return nil, ghErrors.NewGitHubAPIError(
59+
"failed to get alert",
60+
resp,
61+
err,
62+
)
5863
}
5964
defer func() { _ = resp.Body.Close() }()
6065

@@ -138,7 +143,11 @@ func ListCodeScanningAlerts(getClient GetClientFn, t translations.TranslationHel
138143
}
139144
alerts, resp, err := client.CodeScanning.ListAlertsForRepo(ctx, owner, repo, &github.AlertListOptions{Ref: ref, State: state, Severity: severity, ToolName: toolName})
140145
if err != nil {
141-
return nil, fmt.Errorf("failed to list alerts: %w", err)
146+
return nil, ghErrors.NewGitHubAPIError(
147+
"failed to list alerts",
148+
resp,
149+
err,
150+
)
142151
}
143152
defer func() { _ = resp.Body.Close() }()
144153

‎pkg/github/issues.go‎

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strings"
1010
"time"
1111

12+
ghErrors "github.com/github/github-mcp-server/pkg/errors"
1213
"github.com/github/github-mcp-server/pkg/translations"
1314
"github.com/go-viper/mapstructure/v2"
1415
"github.com/google/go-github/v69/github"
@@ -58,7 +59,11 @@ func GetIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool
5859
}
5960
issue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber)
6061
if err != nil {
61-
return nil, fmt.Errorf("failed to get issue: %w", err)
62+
return nil, ghErrors.NewGitHubAPIError(
63+
fmt.Sprintf("failed to get issue with number '%d'", issueNumber),
64+
resp,
65+
err,
66+
)
6267
}
6368
defer func() { _ = resp.Body.Close() }()
6469

@@ -132,7 +137,11 @@ func AddIssueComment(getClient GetClientFn, t translations.TranslationHelperFunc
132137
}
133138
createdComment, resp, err := client.Issues.CreateComment(ctx, owner, repo, issueNumber, comment)
134139
if err != nil {
135-
return nil, fmt.Errorf("failed to create comment: %w", err)
140+
return nil, ghErrors.NewGitHubAPIError(
141+
fmt.Sprintf("failed to create comment on issue '%d'", issueNumber),
142+
resp,
143+
err,
144+
)
136145
}
137146
defer func() { _ = resp.Body.Close() }()
138147

@@ -220,7 +229,11 @@ func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (
220229
}
221230
result, resp, err := client.Search.Issues(ctx, query, opts)
222231
if err != nil {
223-
return nil, fmt.Errorf("failed to search issues: %w", err)
232+
return nil, ghErrors.NewGitHubAPIError(
233+
"failed to search issues",
234+
resp,
235+
err,
236+
)
224237
}
225238
defer func() { _ = resp.Body.Close() }()
226239

@@ -342,7 +355,11 @@ func CreateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
342355
}
343356
issue, resp, err := client.Issues.Create(ctx, owner, repo, issueRequest)
344357
if err != nil {
345-
return nil, fmt.Errorf("failed to create issue: %w", err)
358+
return nil, ghErrors.NewGitHubAPIError(
359+
"failed to create issue",
360+
resp,
361+
err,
362+
)
346363
}
347364
defer func() { _ = resp.Body.Close() }()
348365

@@ -464,7 +481,11 @@ func ListIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (to
464481
}
465482
issues, resp, err := client.Issues.ListByRepo(ctx, owner, repo, opts)
466483
if err != nil {
467-
return nil, fmt.Errorf("failed to list issues: %w", err)
484+
return nil, ghErrors.NewGitHubAPIError(
485+
"failed to list issues",
486+
resp,
487+
err,
488+
)
468489
}
469490
defer func() { _ = resp.Body.Close() }()
470491

@@ -610,7 +631,11 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
610631
}
611632
updatedIssue, resp, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueRequest)
612633
if err != nil {
613-
return nil, fmt.Errorf("failed to update issue: %w", err)
634+
return nil, ghErrors.NewGitHubAPIError(
635+
"failed to update issue",
636+
resp,
637+
err,
638+
)
614639
}
615640
defer func() { _ = resp.Body.Close() }()
616641

@@ -693,7 +718,11 @@ func GetIssueComments(getClient GetClientFn, t translations.TranslationHelperFun
693718
}
694719
comments, resp, err := client.Issues.ListComments(ctx, owner, repo, issueNumber, opts)
695720
if err != nil {
696-
return nil, fmt.Errorf("failed to get issue comments: %w", err)
721+
return nil, ghErrors.NewGitHubAPIError(
722+
"failed to get issue comments",
723+
resp,
724+
err,
725+
)
697726
}
698727
defer func() { _ = resp.Body.Close() }()
699728

@@ -824,7 +853,10 @@ func AssignCopilotToIssue(getGQLClient GetGQLClientFn, t translations.Translatio
824853
var query suggestedActorsQuery
825854
err := client.Query(ctx, &query, variables)
826855
if err != nil {
827-
return nil, err
856+
return nil, ghErrors.NewGitHubGraphQLError(
857+
"failed to list suggested actors",
858+
err,
859+
)
828860
}
829861

830862
// Iterate all the returned nodes looking for the copilot bot, which is supposed to have the
@@ -870,7 +902,10 @@ func AssignCopilotToIssue(getGQLClient GetGQLClientFn, t translations.Translatio
870902
}
871903

872904
if err := client.Query(ctx, &getIssueQuery, variables); err != nil {
873-
return mcp.NewToolResultError(fmt.Sprintf("failed to get issue ID: %v", err)), nil
905+
return nil, ghErrors.NewGitHubGraphQLError(
906+
"failed to get issue ID",
907+
err,
908+
)
874909
}
875910

876911
// Finally, do the assignment. Just for reference, assigning copilot to an issue that it is already
@@ -896,7 +931,10 @@ func AssignCopilotToIssue(getGQLClient GetGQLClientFn, t translations.Translatio
896931
},
897932
nil,
898933
); err != nil {
899-
return nil, fmt.Errorf("failed to replace actors for assignable: %w", err)
934+
return nil, ghErrors.NewGitHubGraphQLError(
935+
"failed to replace actors for assignable",
936+
err,
937+
)
900938
}
901939

902940
return mcp.NewToolResultText("successfully assigned copilot to issue"), nil

‎pkg/github/notifications.go‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strconv"
1010
"time"
1111

12+
ghErrors "github.com/github/github-mcp-server/pkg/errors"
1213
"github.com/github/github-mcp-server/pkg/translations"
1314
"github.com/google/go-github/v69/github"
1415
"github.com/mark3labs/mcp-go/mcp"
@@ -118,7 +119,11 @@ func ListNotifications(getClient GetClientFn, t translations.TranslationHelperFu
118119
notifications, resp, err = client.Activity.ListNotifications(ctx, opts)
119120
}
120121
if err != nil {
121-
return nil, fmt.Errorf("failed to get notifications: %w", err)
122+
return nil, ghErrors.NewGitHubAPIError(
123+
"failed to list notifications",
124+
resp,
125+
err,
126+
)
122127
}
123128
defer func() { _ = resp.Body.Close() }()
124129

@@ -187,7 +192,11 @@ func DismissNotification(getclient GetClientFn, t translations.TranslationHelper
187192
}
188193

189194
if err != nil {
190-
return nil, fmt.Errorf("failed to mark notification as %s: %w", state, err)
195+
return nil, ghErrors.NewGitHubAPIError(
196+
fmt.Sprintf("failed to mark notification as %s", state),
197+
resp,
198+
err,
199+
)
191200
}
192201
defer func() { _ = resp.Body.Close() }()
193202

@@ -262,7 +271,11 @@ func MarkAllNotificationsRead(getClient GetClientFn, t translations.TranslationH
262271
resp, err = client.Activity.MarkNotificationsRead(ctx, markReadOptions)
263272
}
264273
if err != nil {
265-
return nil, fmt.Errorf("failed to mark all notifications as read: %w", err)
274+
return nil, ghErrors.NewGitHubAPIError(
275+
"failed to mark all notifications as read",
276+
resp,
277+
err,
278+
)
266279
}
267280
defer func() { _ = resp.Body.Close() }()
268281

@@ -304,7 +317,11 @@ func GetNotificationDetails(getClient GetClientFn, t translations.TranslationHel
304317

305318
thread, resp, err := client.Activity.GetThread(ctx, notificationID)
306319
if err != nil {
307-
return nil, fmt.Errorf("failed to get notification details: %w", err)
320+
return nil, ghErrors.NewGitHubAPIError(
321+
fmt.Sprintf("failed to get notification details for ID '%s'", notificationID),
322+
resp,
323+
err,
324+
)
308325
}
309326
defer func() { _ = resp.Body.Close() }()
310327

@@ -385,7 +402,11 @@ func ManageNotificationSubscription(getClient GetClientFn, t translations.Transl
385402
}
386403

387404
if apiErr != nil {
388-
return nil, fmt.Errorf("failed to %s notification subscription: %w", action, apiErr)
405+
return nil, ghErrors.NewGitHubAPIError(
406+
fmt.Sprintf("failed to %s notification subscription", action),
407+
resp,
408+
apiErr,
409+
)
389410
}
390411
defer func() { _ = resp.Body.Close() }()
391412

@@ -474,7 +495,11 @@ func ManageRepositoryNotificationSubscription(getClient GetClientFn, t translati
474495
}
475496

476497
if apiErr != nil {
477-
return nil, fmt.Errorf("failed to %s repository subscription: %w", action, apiErr)
498+
return nil, ghErrors.NewGitHubAPIError(
499+
fmt.Sprintf("failed to %s repository subscription", action),
500+
resp,
501+
apiErr,
502+
)
478503
}
479504
if resp != nil {
480505
defer func() { _ = resp.Body.Close() }()

0 commit comments

Comments
 (0)