Skip to content

Classify MCP tool errors by operational category - #94

Merged
ChiragAgg5k merged 1 commit into
mainfrom
fix/tool-error-categories
Aug 9, 2026
Merged

Classify MCP tool errors by operational category#94
ChiragAgg5k merged 1 commit into
mainfrom
fix/tool-error-categories

Conversation

@ChiragAgg5k

Copy link
Copy Markdown
Member

Summary

  • classify public MCP tool failures as write_confirmation, appwrite_4xx, appwrite_5xx, sdk_validation, or internal
  • emit the bounded error_category attribute alongside the existing error_type telemetry attribute
  • use the same classifier for Sentry capture policy and event tags, suppressing expected write-confirmation and Appwrite 4xx outcomes
  • preserve the existing MCP error response text and is_error=true behavior

Testing

  • uv run --group dev ruff check src tests
  • uv run --group dev black --check src tests
  • uv run --group dev pyright
  • uv run python -m unittest discover -s tests/unit -v
  • docker build -t appwrite-mcp:error-categories .

Follow-up

The MCP dashboard in the separate dashboards repository should group the primary error-distribution panel by error_category; the retained error_type label remains available for drill-down and compatibility.

@greptile-apps

greptile-apps Bot commented Aug 9, 2026

Copy link
Copy Markdown

Greptile Summary

The PR introduces bounded operational categories for public MCP tool failures and uses them in telemetry and Sentry tagging and capture policy.

  • Adds a shared exception-chain classifier for write confirmation, Appwrite HTTP failures, SDK validation failures, and internal errors.
  • Adds error_category to tool-error metrics and Sentry events.
  • Suppresses expected write-confirmation and Appwrite 4xx outcomes and updates unit coverage.

Confidence Score: 4/5

The SDK-validation capture-policy mismatch should be fixed before merging so direct validation failures are not silently excluded from Sentry.

The broad ValueError guard runs before category classification, causing direct Pydantic validation failures to bypass the newly introduced category-based monitoring policy.

Files Needing Attention: src/mcp_server_appwrite/error_monitoring.py and tests/unit/test_error_monitoring.py

Important Files Changed

Filename Overview
src/mcp_server_appwrite/error_classification.py Adds bounded exception classification with defensive chain traversal and status-code normalization.
src/mcp_server_appwrite/error_monitoring.py Uses categories for Sentry tags and suppression, but the earlier ValueError guard still suppresses direct SDK validation failures.
src/mcp_server_appwrite/operator.py Records classified failures and introduces a dedicated write-confirmation exception without changing response text.
src/mcp_server_appwrite/telemetry.py Adds the bounded error_category attribute to failed tool-call metrics with an internal fallback.
tests/unit/test_error_monitoring.py Covers category tags and expected suppression but omits direct Pydantic ValidationError capture behavior.

Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
### Issue 1
src/mcp_server_appwrite/error_monitoring.py:164-167
**SDK validation bypasses category policy**

When a public tool raises a Pydantic `ValidationError` directly, the broad `ValueError` check suppresses it before the new classifier runs, causing an `sdk_validation` telemetry failure to be silently omitted from Sentry.

```suggestion
    category = classify_tool_error(exc)
    if isinstance(exc, ValueError) and category != "sdk_validation":
        return False
    if category in {"write_confirmation", "appwrite_4xx"}:
        return False
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "feat: classify MCP tool errors" | Re-trigger Greptile

Comment on lines 164 to 167
if isinstance(exc, ValueError):
return False
appwrite_error = _find_appwrite_exception(exc)
if appwrite_error is not None and _is_appwrite_client_error(appwrite_error):
if classify_tool_error(exc) in {"write_confirmation", "appwrite_4xx"}:
return False

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 SDK validation bypasses category policy

When a public tool raises a Pydantic ValidationError directly, the broad ValueError check suppresses it before the new classifier runs, causing an sdk_validation telemetry failure to be silently omitted from Sentry.

Suggested change
if isinstance(exc, ValueError):
return False
appwrite_error = _find_appwrite_exception(exc)
if appwrite_error is not None and _is_appwrite_client_error(appwrite_error):
if classify_tool_error(exc) in {"write_confirmation", "appwrite_4xx"}:
return False
category = classify_tool_error(exc)
if isinstance(exc, ValueError) and category != "sdk_validation":
return False
if category in {"write_confirmation", "appwrite_4xx"}:
return False
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mcp_server_appwrite/error_monitoring.py
Line: 164-167

Comment:
**SDK validation bypasses category policy**

When a public tool raises a Pydantic `ValidationError` directly, the broad `ValueError` check suppresses it before the new classifier runs, causing an `sdk_validation` telemetry failure to be silently omitted from Sentry.

```suggestion
    category = classify_tool_error(exc)
    if isinstance(exc, ValueError) and category != "sdk_validation":
        return False
    if category in {"write_confirmation", "appwrite_4xx"}:
        return False
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

@ChiragAgg5k
ChiragAgg5k merged commit f4f9ede into main Aug 9, 2026
5 checks passed
@ChiragAgg5k
ChiragAgg5k deleted the fix/tool-error-categories branch August 9, 2026 14:30
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.

1 participant