Skip to content

refactor: replace CallToolResult methods with NewToolResult methods for error handling#26

Merged
cfc4n merged 1 commit intomasterfrom
update-toolresult-func
Apr 12, 2025
Merged

refactor: replace CallToolResult methods with NewToolResult methods for error handling#26
cfc4n merged 1 commit intomasterfrom
update-toolresult-func

Conversation

@cfc4n
Copy link
Copy Markdown
Member

@cfc4n cfc4n commented Apr 12, 2025

This pull request involves refactoring error handling and result reporting methods in several service files to use a unified approach. The changes replace custom error and result handling methods with standardized methods from the mcp package.

Error Handling and Result Reporting Standardization:

  • services/browser.go: Replaced bs.CallToolResultErr and bs.CallToolResult with mcp.NewToolResultError and mcp.NewToolResultText respectively in various methods such as handleNavigate, handleScreenshot, handleClick, handleFill, handleSelect, handleHover, and handleEvaluate. [1] [2] [3]
  • services/browser_debugger.go: Updated methods like handleDebugEnable, handleSetBreakpoint, handleRemoveBreakpoint, handlePause, handleResume, and handleGetCallstack to use mcp.NewToolResultError and mcp.NewToolResultText instead of bs.CallToolResultErr and bs.CallToolResult. [1] [2] [3] [4] [5] [6] [7]
  • services/command.go: Modified handleExecuteCommand to replace cs.CallToolResultErr and cs.CallToolResult with mcp.NewToolResultError and mcp.NewToolResultText.
  • services/file_system.go: Updated methods like handleReadFile, handleWriteFile, handleListDirectory, and handleCreateDirectory to use mcp.NewToolResultError and mcp.NewToolResultText instead of fss.CallToolResultErr and fss.CallToolResult. [1] [2] [3] [4] [5] [6] [7]

…or error handling

Signed-off-by: CFC4N <cfc4n.cs@gmail.com>
@cfc4n cfc4n requested a review from Copilot April 12, 2025 15:55
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Apr 12, 2025
@cfc4n cfc4n added the enhancement New feature or request label Apr 12, 2025
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.

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

services/browser.go:385

  • [nitpick] When the type assertion for 'selector' fails, the variable 'selector' is not assigned and may be empty. Consider referencing request.Params.Arguments["selector"] directly to provide a more informative error message.
return mcp.NewToolResultError(fmt.Sprintf("selector must be a string:%v", selector)), nil

services/file_system.go:715

  • [nitpick] There is an inconsistency in error message capitalization compared to other similar messages (e.g., 'Path must be a string'). Standardizing the capitalization can improve clarity and consistency.
return mcp.NewToolResultError("path must be a string"), nil

@cfc4n cfc4n merged commit a7f1235 into master Apr 12, 2025
6 checks passed
@cfc4n cfc4n deleted the update-toolresult-func branch April 12, 2025 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants