8000 move to new approach and update testing · github/github-mcp-server@ffd99aa · GitHub
[go: up one dir, main page]
Skip to content

Commit ffd99aa

Browse files
SamMorrowDrumsSamMorrowDrums
committed
move to new approach and update testing
1 parent bedcde7 commit ffd99aa

19 files changed

+904
-253
lines changed

‎docs/error-handling.md

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
# Error Handling
2+
3+
This document describes the error handling patterns used in the GitHub MCP Server, specifically how we handle GitHub API errors and avoid direct use of mcp-go error types.
4+
5+
## Overview
6+
7+
The GitHub MCP Server implements a custom error handling approach that serves two primary purposes:
8+
9+
1. **Tool Response Generation**: Return appropriate MCP tool error responses to clients
10+
2. **Middleware Inspection**: Store detailed error information in the request context for middleware analysis
11+
12+
This dual approach enables better observability and debugging capabilities, particularly for remote server deployments where understanding the nature of failures (rate limiting, authentication, 404s, 500s, etc.) is crucial for validation and monitoring.
13+
14+
## Error Types
15+
16+
### GitHubAPIError
17+
18+
Used for REST API errors from the GitHub API:
19+
20+
```go
21+
type GitHubAPIError struct {
22+
Message string `json:"message"`
23+
Response *github.Response `json:"-"`
24+
Err error `json:"-"`
25+
}
26+
```
27+
28+
### GitHubGraphQLError
29+
30+
Used for GraphQL API errors from the GitHub API:
31+
32+
```go
33+
type GitHubGraphQLError struct {
34+
Message string `json:"message"`
35+
Err error `json:"-"`
36+
}
37+
```
38+
39+
## Usage Patterns
40+
41+
### For GitHub REST API Errors
42+
43+
Instead of directly returning `mcp.NewToolResultError()`, use:
44+
45+
```go
46+
return ghErrors.NewGitHubAPIErrorResponse(ctx, message, response, err), nil
47+
```
48+
49+
This function:
50+
- Creates a `GitHubAPIError` with the provided message, response, and error
51+
- Stores the error in the context for middleware inspection
52+
- Returns an appropriate MCP tool error response
53+
54+
### For GitHub GraphQL API Errors
55+
56+
```go
57+
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, message, err), nil
58+
```
59+
60+
### Context Management
61+
62+
The error handling system uses context to store errors for later inspection:
63+
64+
```go
65+
// Initialize context with error tracking
66+
ctx = errors.ContextWithGitHubErrors(ctx)
67+
68+
// Retrieve errors for inspection (typically in middleware)
69+
apiErrors, err := errors.GetGitHubAPIErrors(ctx)
70+
graphqlErrors, err := errors.GetGitHubGraphQLErrors(ctx)
71+
```
72+
73+
## Design Principles
74+
75+
### User-Actionable vs. Developer Errors
76+
77+
- **User-actionable errors** (authentication failures, rate limits, 404s) should be returned as failed tool calls using the error response functions
78+
- **Developer errors** (JSON marshaling failures, internal logic errors) should be returned as actual Go errors that bubble up through the MCP framework
79+
80+
### Context Limitations
81+
82+
This approach was designed to work around current limitations in mcp-go where context is not propagated through each step of request processing. By storing errors in context values, middleware can inspect them without requiring context propagation.
83+
84+
### Graceful Error Handling
85+
86+
Error storage operations in context are designed to fail gracefully - if context storage fails, the tool will still return an appropriate error response to the client.
87+
88+
## Benefits
89+
90+
1. **Observability**: Middleware can inspect the specific types of GitHub API errors occurring
91+
2. **Debugging**: Detailed error information is preserved without exposing potentially sensitive data in logs
92+
3. **Validation**: Remote servers can use error types and HTTP status codes to validate that changes don't break functionality
93+
4. **Privacy**: Error inspection can be done programmatically using `errors.Is` checks without logging PII
94+
95+
## Example Implementation
96+
97+
```go
98+
func GetIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
99+
return mcp.NewTool("get_issue", /* ... */),
100+
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
101+
owner, err := RequiredParam[string](request, "owner")
102+
if err != nil {
103+
return mcp.NewToolResultError(err.Error()), nil
104+
}
105+
106+
client, err := getClient(ctx)
107+
if err != nil {
108+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
109+
}
110+
111+
issue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber)
112+
if err != nil {
113+
return ghErrors.NewGitHubAPIErrorResponse(ctx,
114+
"failed to get issue",
115+
resp,
116+
err,
117+
), nil
118+
}
119+
120+
return MarshalledTextResult(issue), nil
121+
}
122+
}
123+
```
124+
125+
This approach ensures that both the client receives an appropriate error response and any middleware can inspect the underlying GitHub API error for monitoring and debugging purposes.

‎internal/ghmcp/server.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"strings"
1313
"syscall"
1414

15+
"github.com/github/github-mcp-server/pkg/errors"
1516
"github.com/github/github-mcp-server/pkg/github"
1617
mcplog "github.com/github/github-mcp-server/pkg/log"
1718
"github.com/github/github-mcp-server/pkg/raw"
@@ -90,6 +91,13 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
9091

9192
hooks := &server.Hooks{
9293
OnBeforeInitialize: []server.OnBeforeInitializeFunc{beforeInit},
94+
OnBeforeAny: []server.BeforeAnyHookFunc{
95+
func(ctx context.Context, _ any, _ mcp.MCPMethod, _ any) {
96+
// Ensure the context is cleared of any previous errors
97+
// as context isn't propagated through middleware
98+
errors.ContextWithGitHubErrors(ctx)
99+
},
100+
},
93101
}
94102

95103
ghServer := github.NewServer(cfg.Version, server.WithHooks(hooks))
@@ -222,7 +230,8 @@ func RunStdioServer(cfg StdioServerConfig) error {
222230
loggedIO := mcplog.NewIOLogger(in, out, logrusLogger)
223231
in, out = loggedIO, loggedIO
224232
}
225-
233+
// enable GitHub errors in the context
234+
ctx := errors.ContextWithGitHubErrors(ctx)
226235
errC <- stdioServer.Listen(ctx, in, out)
227236
}()
228237

‎pkg/errors/error.go

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package errors
22

33
import (
4+
"context"
45
"fmt"
56

67
"github.com/google/go-github/v72/github"
8+
"github.com/mark3labs/mcp-go/mcp"
79
)
810

911
type GitHubAPIError struct {
@@ -12,7 +14,8 @@ type GitHubAPIError struct {
1214
Err error `json:"-"`
1315
}
1416

15-
func NewGitHubAPIError(message string, resp *github.Response, err error) *GitHubAPIError {
17+
// NewGitHubAPIError creates a new GitHubAPIError with the provided message, response, and error.
18+
func newGitHubAPIError(message string, resp *github.Response, err error) *GitHubAPIError {
1619
return &GitHubAPIError{
1720
Message: message,
1821
Response: resp,
@@ -29,7 +32,7 @@ type GitHubGraphQLError struct {
2932
Err error `json:"-"`
3033
}
3134

32-
func NewGitHubGraphQLError(message string, err error) *GitHubGraphQLError {
35+
func newGitHubGraphQLError(message string, err error) *GitHubGraphQLError {
3336
return &GitHubGraphQLError{
3437
Message: message,
3538
Err: err,
@@ -39,3 +42,84 @@ func NewGitHubGraphQLError(message string, err error) *GitHubGraphQLError {
3942
func (e *GitHubGraphQLError) Error() string {
4043
return fmt.Errorf("%s: %w", e.Message, e.Err).Error()
4144
}
45+
46+
type GitHubErrorKey struct{}
47+
type GitHubCtxErrors struct {
48+
api []*GitHubAPIError
49+
graphQL []*GitHubGraphQLError
50+
}
51+
52+
// ContextWithGitHubErrors updates or creates a context with a pointer to GitHub error information (to be used by middleware).
53+
func ContextWithGitHubErrors(ctx context.Context) context.Context {
54+
if ctx == nil {
55+
ctx = context.Background()
56+
}
57+
if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok {
58+
// If the context already has GitHubCtxErrors, we just empty the slices to start fresh
59+
val.api = []*GitHubAPIError{}
60+
val.graphQL = []*GitHubGraphQLError{}
61+
} else {
62+
// If not, we create a new GitHubCtxErrors and set it in the context
63+
ctx = context.WithValue(ctx, GitHubErrorKey{}, &GitHubCtxErrors{})
64+
}
65+
66+
return ctx
67+
}
68+
69+
// GetGitHubAPIErrors retrieves the slice of GitHubAPIErrors from the context.
70+
func GetGitHubAPIErrors(ctx context.Context) ([]*GitHubAPIError, error) {
71+
if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok {
72+
return val.api, nil // return the slice of API errors from the context
73+
}
74+
return nil, fmt.Errorf("context does not contain GitHubCtxErrors")
75+
}
76+
77+
// GetGitHubGraphQLErrors retrieves the slice of GitHubGraphQLErrors from the context.
78+
func GetGitHubGraphQLErrors(ctx context.Context) ([]*GitHubGraphQLError, error) {
79+
if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok {
80+
return val.graphQL, nil // return the slice of GraphQL errors from the context
81+
}
82+
return nil, fmt.Errorf("context does not contain GitHubCtxErrors")
83+
}
84+
85+
func NewGitHubAPIErrorToCtx(ctx context.Context, message string, resp *github.Response, err error) (context.Context, error) {
86+
apiErr := newGitHubAPIError(message, resp, err)
87+
if ctx != nil {
88+
_, _ = addGitHubAPIErrorToContext(ctx, apiErr) // Explicitly ignore error for graceful handling
89+
}
90+
return ctx, nil
91+
}
92+
93+
func addGitHubAPIErrorToContext(ctx context.Context, err *GitHubAPIError) (context.Context, error) {
94+
if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok {
95+
val.api = append(val.api, err) // append the error to the existing slice in the context
96+
return ctx, nil
97+
}
98+
return nil, fmt.Errorf("context does not contain GitHubCtxErrors")
99+
}
100+
101+
func addGitHubGraphQLErrorToContext(ctx context.Context, err *GitHubGraphQLError) (context.Context, error) {
102+
if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok {
103+
val.graphQL = append(val.graphQL, err) // append the error to the existing slice in the context
104+
return ctx, nil
105+
}
106+
return nil, fmt.Errorf("context does not contain GitHubCtxErrors")
107+
}
108+
109+
// NewGitHubAPIErrorResponse returns an mcp.NewToolResultError and retains the error in the context for access via middleware
110+
func NewGitHubAPIErrorResponse(ctx context.Context, message string, resp *github.Response, err error) *mcp.CallToolResult {
111+
apiErr := newGitHubAPIError(message, resp, err)
112+
if ctx != nil {
113+
_, _ = addGitHubAPIErrorToContext(ctx, apiErr) // Explicitly ignore error for graceful handling
114+
}
115+
return mcp.NewToolResultErrorFromErr(message, err)
116+
}
117+
118+
// NewGitHubGraphQLErrorResponse returns an mcp.NewToolResultError and retains the error in the context for access via middleware
119+
func NewGitHubGraphQLErrorResponse(ctx context.Context, message string, err error) *mcp.CallToolResult {
120+
graphQLErr := newGitHubGraphQLError(message, err)
121+
if ctx != nil {
122+
_, _ = addGitHubGraphQLErrorToContext(ctx, graphQLErr) // Explicitly ignore error for graceful handling
123+
}
124+
return mcp.NewToolResultErrorFromErr(message, err)
125+
}

0 commit comments

Comments
 (0)
0