fix: return isError instead of throwing InternalError on chart failures

[blackwell-systems]

Summary

• When chart rendering fails (e.g., due to invalid/empty data), the catch block in callTool.ts threw McpError(InternalError) which maps to MCP error -32603
• MCP clients treat -32603 as a server crash, so agents receive raw stack traces and cannot recover or self-correct
• Changed to return { content: [...], isError: true } with a descriptive error message
• McpError and ValidateError are still thrown as before (they produce proper MCP error responses)

Tools fixed

All 9 tools from #291: generate_fishbone_diagram, generate_mind_map, generate_organization_chart, generate_flow_diagram, generate_network_graph, generate_funnel_chart, generate_venn_chart, generate_district_map, generate_radar_chart

Before/After

Before: Agent receives -32603 with a raw stack trace:

internal error: MCP error -32603: Failed to generate chart: Cannot read properties of undefined (reading 'category')
TypeError: Cannot read properties of undefined (reading 'category')
    at _callee$ (/var/task/code/font.ts:40:2)
    ...

After: Agent receives isError: true with actionable message:

{
  "content": [{"type": "text", "text": "Failed to generate chart: Cannot read properties of undefined (reading 'category'). Please check that the data matches the expected format for this chart type."}],
  "isError": true
}

Test plan

• tsc --noEmit passes
• biome lint passes (ran via pre-commit hook)
• Verified with mcp-assert

Fixes #291

[gemini-code-assist]

Code Review

This pull request updates the error handling in callTool.ts to return a structured error response instead of throwing an InternalError, which prevents MCP client crashes and allows agents to self-correct. Feedback was provided to improve the robustness of error message extraction by accounting for cases where the error might be a string.

[hustcc]

How to verified with mcp-assert？ it is a GitHub action?

[blackwell-systems]

mcp-assert is both a CLI tool and a GitHub Action.

CLI (quickest way to try it):

# Install
brew install blackwell-systems/tap/mcp-assert
# or: npx @blackwell-systems/mcp-assert
# or: pip install mcp-assert

# Generate assertion stubs for all your tools
mcp-assert generate --server "npx -y @antv/mcp-server-chart" --output evals/

# Run them
mcp-assert run --suite evals/

GitHub Action (for CI):

- uses: blackwell-systems/mcp-assert-action@v1
  with:
    suite: evals/

The assertion YAML files in this PR's linked issue (#291) were generated with mcp-assert generate and then customized with expected values.

Docs: https://github.com/blackwell-systems/mcp-assert

[hustcc]

you can help to add the action into ci.

[blackwell-systems]

Happy to add the CI integration. I'll submit a follow-up PR after this one merges that adds:

1. An evals/ directory with assertion YAML files for all 25 tools
2. A GitHub Actions workflow using blackwell-systems/mcp-assert-action@v1

The CI job will catch regressions on every push and PR. Once this fix lands, all 25 tools should pass (the 9 that currently crash will return isError: true instead).

Want me to wait for this PR to merge first, or submit both in parallel?

[blackwell-systems]

Hi @hustcc, following up on your request to add CI integration. Happy to submit the follow-up PR with the evals/ directory and GitHub Actions workflow whenever this one is ready to merge. Let me know if there's anything needed on this PR to move it forward.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.94%. Comparing base (7563a99) to head (433b2a4).

Files with missing lines	Patch %	Lines
src/utils/callTool.ts	0.00%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #292      +/-   ##
==========================================
- Coverage   82.10%   81.94%   -0.17%     
==========================================
  Files          43       43              
  Lines        2040     2044       +4     
  Branches       34       34              
==========================================
  Hits         1675     1675              
- Misses        363      367       +4     
  Partials        2        2              

Files with missing lines	Coverage Δ
src/utils/callTool.ts	39.21% <0.00%> (-1.61%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[hustcc]

I feel this is still an exception and should throw an error. The best solution is to avoid this error at the source, for example: solve the problem at the rendering layer, or restrict the input requirements in the JSON schema.

[blackwell-systems]

Agree that fixing the source is ideal. Stricter JSON Schema validation and rendering-layer guards would prevent most of these errors from happening in the first place.

But the two approaches complement each other. Even with perfect validation, unexpected errors can still occur (network issues, resource limits, edge cases in chart data). The MCP protocol distinguishes between these intentionally:

• isError: true means "the tool understood the request but couldn't complete it." The agent reads the message and can retry with different input or ask the user for help.
• -32603 (Internal Error) means "the server crashed." The agent treats the tool as broken and has no recovery path.

This PR converts unhandled exceptions into isError responses, which is the defensive layer. It doesn't prevent you from also adding input validation or rendering fixes on top. Most production MCP servers (Anthropic's reference servers, Microsoft Playwright, Mozilla Firefox DevTools) use both: validate input where possible, catch everything else and return isError.

Happy to also help add stricter JSON Schema constraints for the tool inputs in a follow-up if that's useful.