Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
SamMorrowDrums wants to merge3 commits intomain
base:main
Choose a base branch
Loading
fromenable-error-middleware

Conversation

SamMorrowDrums
Copy link
Collaborator

Primarily for the remote server, this is the minimal approach@omgitsads and I could find to enable us to store API errors on the context, so that we can use the error types and http status codes etc. to derive the nature of failures, so that we can understand if failed tool calls occur due to:

  • rate limiting
  • authentication issues
  • 404
  • 500s from API etc.

Without this knowledge it's hard for us to validate if changes break things or not.

This is additionally frustrated by the fact mcp-go currently doesn't propagate context through each next step, and so we have concluded that if we create a value pointer on the context, we can edit the values without needing to propagate the context, and we are using this to be able to implement the aforementioned middleware, which can then extract the errors and inspect them without logging them directly (as that would expose PII sometimes), and so it's best to be able to useErrors.Is checks. The middleware is not present in this repo (although we could also provide debug logs on the STDIO server using the server log message protocol, which would be a nice addition and likely a follow-up to this work).

Developer facing changes

Instead of directly returning mcp tool error responses, tool authors will call functions that will both return the tool error response, but will also add the actual errors to the context for later inspection.

returnghErrors.NewGitHubAPIErrorResponse(ctx,"failed to rerun workflow run",resp,err),nil
return ghErrors.NewGitHubGraphQLErrorResponse(ctx,"failed to get latest review for current user",err,), nil

Additional change

Errors that are ones the user should be able to do something about need to be returned as failed tool calls and not as the error value, so I have fixed a few of those in this PR.

I have left things like failures to marhsall response json, as I would consider that a real error and likely something that requires developers to fix.

@CopilotCopilotAI review requested due to automatic review settingsJune 24, 2025 14:07
@SamMorrowDrumsSamMorrowDrums requested a review froma team as acode ownerJune 24, 2025 14:07
Copy link
Contributor

@CopilotCopilotAI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Pull Request Overview

This PR introduces context-based error middleware support by capturing GitHub API and GraphQL errors in the context, exposing them for later inspection rather than returning raw errors. Key changes include:

  • Swapping rawfmt.Errorf returns forNewGitHubAPIErrorResponse/NewGitHubGraphQLErrorResponse to store errors in context.
  • Updating tests to expectresult.IsError and extract error text viagetErrorResult.
  • Initializing and clearing error context in server hooks (ContextWithGitHubErrors).

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
pkg/github/secret_scanning.goReplaced raw error returns withNewGitHubAPIErrorResponse.
pkg/github/search.goSame pattern for search endpoints.
pkg/github/repositories.goWrapped repository errors in context and result responses.
pkg/github/pullrequests.goApplied context error handling to pull request operations.
pkg/github/notifications.goAdded context error responses for notifications API calls.
pkg/github/context_tools.goSwapped user error return for API error response inGetMe.
pkg/github/actions.goAdjusted job log handlers to return and store context errors.
pkg/github/code_scanning.goConsistent context error wrapping for code scanning endpoints.
pkg/errors/error.goIntroducedGitHubAPIError,GitHubGraphQLError, and helpers.
internal/ghmcp/server.goAdded hooks to initialize/clear GitHub error context.
Various_test.go filesUpdated tests to checkresult.IsError and inspect error text.
Comments suppressed due to low confidence (1)

pkg/github/context_tools.go:32

  • [nitpick] Consider renaming the variableres toresp for consistency with other GitHub client calls.
user, res, err := client.Users.Get(ctx, "")

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

Copilot code reviewCopilotCopilot left review comments

@omgitsadsomgitsadsomgitsads approved these changes

@JoannaaKLJoannaaKLJoannaaKL approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@SamMorrowDrums@omgitsads@JoannaaKL

[8]ページ先頭

©2009-2025 Movatter.jp