Skip to content

Improve error handling to use MCP tool-level errors#55

Merged
wesleyjellis merged 1 commit intomainfrom
wesley/better-errors
Jun 12, 2025
Merged

Improve error handling to use MCP tool-level errors#55
wesleyjellis merged 1 commit intomainfrom
wesley/better-errors

Conversation

@wesleyjellis
Copy link
Copy Markdown
Contributor

@wesleyjellis wesleyjellis commented Jun 10, 2025

🤖 Generated with Claude Code

Problem

I noticed sometimes we return nil when something goes wrong, but there's a specific error response we can (and I think should) use

Solution

Checklist

  • I have run this code, and it appears to resolve the stated issue.
  • This PR has no user interface changes or has already received approval from product management to change the interface.
  • Make a changie entry that explains the customer facing outcome of this change.

…tegration

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@wesleyjellis wesleyjellis requested a review from Copilot June 10, 2025 20:01
Copy link
Copy Markdown

Copilot AI 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 improves error handling by wrapping native errors into MCP tool-level errors for consistent CLI responses.

  • Updates newToolResult to return CallToolResult error objects instead of raw errors
  • Replaces direct nil, err returns in Cobra commands with mcp.NewToolResultError or mcp.NewToolResultErrorFromErr
  • Adds explicit parameter-required error messages for req.RequireString calls
Comments suppressed due to low confidence (2)

src/cmd/root.go:97

  • [nitpick] The error message "operation failed" is very generic; consider adding context about which operation failed or using a more descriptive message.
return mcp.NewToolResultErrorFromErr("operation failed", err), nil

src/cmd/root.go:94

  • Add unit tests for newToolResult to verify both success and error branches produce the expected CallToolResult instances with correct error wrapping.
func newToolResult(obj any, err error) (*mcp.CallToolResult, error) {

Comment thread src/cmd/root.go
Copy link
Copy Markdown
Contributor

@derek-etherton-opslevel derek-etherton-opslevel left a comment

Choose a reason for hiding this comment

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

makes sense to me, thanks for the QoL improvements

@wesleyjellis wesleyjellis merged commit 0e924f4 into main Jun 12, 2025
2 checks passed
@wesleyjellis wesleyjellis deleted the wesley/better-errors branch June 12, 2025 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants