- Notifications
You must be signed in to change notification settings - Fork1.2k
Enable use of error middleware#573
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes fromall commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
# Error Handling | ||
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. | ||
## Overview | ||
The GitHub MCP Server implements a custom error handling approach that serves two primary purposes: | ||
1. **Tool Response Generation**: Return appropriate MCP tool error responses to clients | ||
2. **Middleware Inspection**: Store detailed error information in the request context for middleware analysis | ||
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. | ||
## Error Types | ||
### GitHubAPIError | ||
Used for REST API errors from the GitHub API: | ||
```go | ||
type GitHubAPIError struct { | ||
Message string `json:"message"` | ||
Response *github.Response `json:"-"` | ||
Err error `json:"-"` | ||
} | ||
``` | ||
### GitHubGraphQLError | ||
Used for GraphQL API errors from the GitHub API: | ||
```go | ||
type GitHubGraphQLError struct { | ||
Message string `json:"message"` | ||
Err error `json:"-"` | ||
} | ||
``` | ||
## Usage Patterns | ||
### For GitHub REST API Errors | ||
Instead of directly returning `mcp.NewToolResultError()`, use: | ||
```go | ||
return ghErrors.NewGitHubAPIErrorResponse(ctx, message, response, err), nil | ||
``` | ||
This function: | ||
- Creates a `GitHubAPIError` with the provided message, response, and error | ||
- Stores the error in the context for middleware inspection | ||
- Returns an appropriate MCP tool error response | ||
### For GitHub GraphQL API Errors | ||
```go | ||
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, message, err), nil | ||
``` | ||
### Context Management | ||
The error handling system uses context to store errors for later inspection: | ||
```go | ||
// Initialize context with error tracking | ||
ctx = errors.ContextWithGitHubErrors(ctx) | ||
// Retrieve errors for inspection (typically in middleware) | ||
apiErrors, err := errors.GetGitHubAPIErrors(ctx) | ||
graphqlErrors, err := errors.GetGitHubGraphQLErrors(ctx) | ||
``` | ||
## Design Principles | ||
### User-Actionable vs. Developer Errors | ||
- **User-actionable errors** (authentication failures, rate limits, 404s) should be returned as failed tool calls using the error response functions | ||
- **Developer errors** (JSON marshaling failures, internal logic errors) should be returned as actual Go errors that bubble up through the MCP framework | ||
### Context Limitations | ||
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. | ||
### Graceful Error Handling | ||
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. | ||
## Benefits | ||
1. **Observability**: Middleware can inspect the specific types of GitHub API errors occurring | ||
2. **Debugging**: Detailed error information is preserved without exposing potentially sensitive data in logs | ||
3. **Validation**: Remote servers can use error types and HTTP status codes to validate that changes don't break functionality | ||
4. **Privacy**: Error inspection can be done programmatically using `errors.Is` checks without logging PII | ||
## Example Implementation | ||
```go | ||
func GetIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { | ||
return mcp.NewTool("get_issue", /* ... */), | ||
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { | ||
owner, err := RequiredParam[string](request, "owner") | ||
if err != nil { | ||
return mcp.NewToolResultError(err.Error()), nil | ||
} | ||
client, err := getClient(ctx) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to get GitHub client: %w", err) | ||
} | ||
issue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber) | ||
if err != nil { | ||
return ghErrors.NewGitHubAPIErrorResponse(ctx, | ||
"failed to get issue", | ||
resp, | ||
err, | ||
), nil | ||
} | ||
return MarshalledTextResult(issue), nil | ||
} | ||
} | ||
``` | ||
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -12,6 +12,7 @@ import ( | ||
"strings" | ||
"syscall" | ||
"github.com/github/github-mcp-server/pkg/errors" | ||
"github.com/github/github-mcp-server/pkg/github" | ||
mcplog "github.com/github/github-mcp-server/pkg/log" | ||
"github.com/github/github-mcp-server/pkg/raw" | ||
@@ -90,6 +91,13 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) { | ||
hooks := &server.Hooks{ | ||
OnBeforeInitialize: []server.OnBeforeInitializeFunc{beforeInit}, | ||
OnBeforeAny: []server.BeforeAnyHookFunc{ | ||
func(ctx context.Context, _ any, _ mcp.MCPMethod, _ any) { | ||
// Ensure the context is cleared of any previous errors | ||
// as context isn't propagated through middleware | ||
errors.ContextWithGitHubErrors(ctx) | ||
SamMorrowDrums marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
}, | ||
}, | ||
} | ||
ghServer := github.NewServer(cfg.Version, server.WithHooks(hooks)) | ||
@@ -222,7 +230,8 @@ func RunStdioServer(cfg StdioServerConfig) error { | ||
loggedIO := mcplog.NewIOLogger(in, out, logrusLogger) | ||
in, out = loggedIO, loggedIO | ||
} | ||
// enable GitHub errors in the context | ||
ctx := errors.ContextWithGitHubErrors(ctx) | ||
errC <- stdioServer.Listen(ctx, in, out) | ||
}() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
package errors | ||
import ( | ||
"context" | ||
"fmt" | ||
"github.com/google/go-github/v72/github" | ||
"github.com/mark3labs/mcp-go/mcp" | ||
) | ||
type GitHubAPIError struct { | ||
Message string `json:"message"` | ||
Response *github.Response `json:"-"` | ||
Err error `json:"-"` | ||
} | ||
// NewGitHubAPIError creates a new GitHubAPIError with the provided message, response, and error. | ||
func newGitHubAPIError(message string, resp *github.Response, err error) *GitHubAPIError { | ||
return &GitHubAPIError{ | ||
Message: message, | ||
Response: resp, | ||
Err: err, | ||
} | ||
} | ||
func (e *GitHubAPIError) Error() string { | ||
return fmt.Errorf("%s: %w", e.Message, e.Err).Error() | ||
} | ||
type GitHubGraphQLError struct { | ||
Message string `json:"message"` | ||
Err error `json:"-"` | ||
} | ||
func newGitHubGraphQLError(message string, err error) *GitHubGraphQLError { | ||
return &GitHubGraphQLError{ | ||
Message: message, | ||
Err: err, | ||
} | ||
} | ||
func (e *GitHubGraphQLError) Error() string { | ||
return fmt.Errorf("%s: %w", e.Message, e.Err).Error() | ||
} | ||
type GitHubErrorKey struct{} | ||
type GitHubCtxErrors struct { | ||
api []*GitHubAPIError | ||
graphQL []*GitHubGraphQLError | ||
} | ||
// ContextWithGitHubErrors updates or creates a context with a pointer to GitHub error information (to be used by middleware). | ||
func ContextWithGitHubErrors(ctx context.Context) context.Context { | ||
if ctx == nil { | ||
ctx = context.Background() | ||
} | ||
if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok { | ||
// If the context already has GitHubCtxErrors, we just empty the slices to start fresh | ||
val.api = []*GitHubAPIError{} | ||
val.graphQL = []*GitHubGraphQLError{} | ||
} else { | ||
// If not, we create a new GitHubCtxErrors and set it in the context | ||
ctx = context.WithValue(ctx, GitHubErrorKey{}, &GitHubCtxErrors{}) | ||
} | ||
return ctx | ||
} | ||
// GetGitHubAPIErrors retrieves the slice of GitHubAPIErrors from the context. | ||
func GetGitHubAPIErrors(ctx context.Context) ([]*GitHubAPIError, error) { | ||
if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok { | ||
return val.api, nil // return the slice of API errors from the context | ||
} | ||
return nil, fmt.Errorf("context does not contain GitHubCtxErrors") | ||
} | ||
// GetGitHubGraphQLErrors retrieves the slice of GitHubGraphQLErrors from the context. | ||
func GetGitHubGraphQLErrors(ctx context.Context) ([]*GitHubGraphQLError, error) { | ||
if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok { | ||
return val.graphQL, nil // return the slice of GraphQL errors from the context | ||
} | ||
return nil, fmt.Errorf("context does not contain GitHubCtxErrors") | ||
} | ||
func NewGitHubAPIErrorToCtx(ctx context.Context, message string, resp *github.Response, err error) (context.Context, error) { | ||
apiErr := newGitHubAPIError(message, resp, err) | ||
if ctx != nil { | ||
_, _ = addGitHubAPIErrorToContext(ctx, apiErr) // Explicitly ignore error for graceful handling | ||
} | ||
return ctx, nil | ||
} | ||
func addGitHubAPIErrorToContext(ctx context.Context, err *GitHubAPIError) (context.Context, error) { | ||
if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok { | ||
val.api = append(val.api, err) // append the error to the existing slice in the context | ||
return ctx, nil | ||
} | ||
return nil, fmt.Errorf("context does not contain GitHubCtxErrors") | ||
} | ||
func addGitHubGraphQLErrorToContext(ctx context.Context, err *GitHubGraphQLError) (context.Context, error) { | ||
if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok { | ||
val.graphQL = append(val.graphQL, err) // append the error to the existing slice in the context | ||
return ctx, nil | ||
} | ||
return nil, fmt.Errorf("context does not contain GitHubCtxErrors") | ||
} | ||
// NewGitHubAPIErrorResponse returns an mcp.NewToolResultError and retains the error in the context for access via middleware | ||
func NewGitHubAPIErrorResponse(ctx context.Context, message string, resp *github.Response, err error) *mcp.CallToolResult { | ||
apiErr := newGitHubAPIError(message, resp, err) | ||
if ctx != nil { | ||
_, _ = addGitHubAPIErrorToContext(ctx, apiErr) // Explicitly ignore error for graceful handling | ||
} | ||
return mcp.NewToolResultErrorFromErr(message, err) | ||
} | ||
// NewGitHubGraphQLErrorResponse returns an mcp.NewToolResultError and retains the error in the context for access via middleware | ||
func NewGitHubGraphQLErrorResponse(ctx context.Context, message string, err error) *mcp.CallToolResult { | ||
graphQLErr := newGitHubGraphQLError(message, err) | ||
if ctx != nil { | ||
_, _ = addGitHubGraphQLErrorToContext(ctx, graphQLErr) // Explicitly ignore error for graceful handling | ||
} | ||
return mcp.NewToolResultErrorFromErr(message, err) | ||
} |
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.