Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 51 additions & 1 deletion pkg/errors/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
stderrors "errors"
"fmt"
"net/http"
"strings"
"time"

"github.com/github/github-mcp-server/pkg/utils"
Expand Down Expand Up @@ -191,7 +192,56 @@ func NewGitHubAPIErrorResponse(ctx context.Context, message string, resp *github
"%s: GitHub secondary rate limit exceeded. Wait before retrying.", message))
}

return utils.NewToolResultErrorFromErr(message, err)
return utils.NewToolResultErrorFromErr(message, formattedGitHubAPIError(err))
}

// formattedGitHubAPIError unwraps a github.ErrorResponse so tool results include
// nested validation messages (for example repository ruleset violations) instead
// of go-github's compact 422 dump.
func formattedGitHubAPIError(err error) error {
var ghErr *github.ErrorResponse
if !stderrors.As(err, &ghErr) {
return err
}

var parts []string
switch {
case ghErr.Response != nil && ghErr.Response.StatusCode != 0 && ghErr.Message != "":
parts = append(parts, fmt.Sprintf("HTTP %d %s", ghErr.Response.StatusCode, ghErr.Message))
case ghErr.Response != nil && ghErr.Response.StatusCode != 0:
parts = append(parts, fmt.Sprintf("HTTP %d", ghErr.Response.StatusCode))
case ghErr.Message != "":
parts = append(parts, ghErr.Message)
}

for _, item := range ghErr.Errors {
detail := strings.TrimSpace(item.Message)
if detail == "" {
var bits []string
if item.Resource != "" {
bits = append(bits, item.Resource)
}
if item.Field != "" {
bits = append(bits, item.Field)
}
if item.Code != "" {
bits = append(bits, item.Code)
}
detail = strings.Join(bits, " ")
}
if detail != "" {
parts = append(parts, detail)
}
}

if ghErr.DocumentationURL != "" {
parts = append(parts, "See "+ghErr.DocumentationURL)
}

if len(parts) == 0 {
return err
}
return stderrors.New(strings.Join(parts, "\n"))
}

// NewGitHubGraphQLErrorResponse returns an mcp.NewToolResultError and retains the error in the context for access via middleware
Expand Down
47 changes: 47 additions & 0 deletions pkg/errors/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -687,3 +687,50 @@ func TestNewGitHubAPIErrorResponse_RateLimits(t *testing.T) {
assert.Contains(t, text, "validation failed")
})
}

func TestNewGitHubAPIErrorResponse_ValidationMessages(t *testing.T) {
t.Run("ruleset ErrorResponse includes nested validation messages", func(t *testing.T) {
ctx := ContextWithGitHubErrors(context.Background())

originalErr := &github.ErrorResponse{
Response: &http.Response{StatusCode: http.StatusUnprocessableEntity},
Message: "Validation Failed",
Errors: []github.Error{
{
Resource: "GitRef",
Field: "ref",
Code: "custom",
Message: "ref name does not match the required pattern 'feature/*'",
},
},
DocumentationURL: "https://docs.github.com/rest/git/refs#create-a-reference",
}

result := NewGitHubAPIErrorResponse(ctx, "failed to create branch", nil, originalErr)

text := requireErrorText(t, result)
assert.Contains(t, text, "failed to create branch")
assert.Contains(t, text, "HTTP 422 Validation Failed")
assert.Contains(t, text, "ref name does not match the required pattern 'feature/*'")
assert.Contains(t, text, "See https://docs.github.com/rest/git/refs#create-a-reference")
assert.NotContains(t, text, "Resource:")
})

t.Run("wrapped ErrorResponse is still unwrapped", func(t *testing.T) {
ctx := ContextWithGitHubErrors(context.Background())

originalErr := fmt.Errorf("create ref: %w", &github.ErrorResponse{
Response: &http.Response{StatusCode: http.StatusUnprocessableEntity},
Message: "Validation Failed",
Errors: []github.Error{
{Message: "Changes must be made through a pull request."},
},
})

result := NewGitHubAPIErrorResponse(ctx, "failed to create branch", nil, originalErr)

text := requireErrorText(t, result)
assert.Contains(t, text, "Changes must be made through a pull request.")
assert.NotContains(t, text, "create ref:")
})
}
30 changes: 30 additions & 0 deletions pkg/github/repositories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,36 @@ func Test_CreateBranch(t *testing.T) {
expectError: true,
expectedErrMsg: "failed to create branch",
},
{
name: "create branch surfaces ruleset validation details",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposGitRefByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockSourceRef),
"GET /repos/owner/repo/git/ref/heads/main": mockResponse(t, http.StatusOK, mockSourceRef),
PostReposGitRefsByOwnerByRepo: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusUnprocessableEntity)
_, _ = w.Write([]byte(`{
"message": "Validation Failed",
"documentation_url": "https://docs.github.com/rest/git/refs#create-a-reference",
"errors": [
{
"resource": "GitRef",
"field": "ref",
"code": "custom",
"message": "ref name does not match the required pattern 'feature/*'"
}
]
}`))
},
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"branch": "hotfix",
"from_branch": "main",
},
expectError: true,
expectedErrMsg: "ref name does not match the required pattern 'feature/*'",
},
}

for _, tc := range tests {
Expand Down