Repository navigation
feat(activity): classify tool-call failures with error_class and fault_domain (Spec 113-c) - #1502
Merged
Merged
Conversation
…t_domain (Spec 113-c) Every failed upstream tool call now carries error_class (network, timeout, http, jsonrpc, tool_error, session_terminated, auth, proxy_policy, proxy_internal, cancelled), fault_domain (upstream, proxy, client) and, when known, upstream_http_status, on the activity record, the REST activity API, the SSE completion payload and mcpproxy activity list|show -o json|yaml. - internal/callerr: pure classifier (Classify) plus a per-call context note and Outcome.AuditClass, a total mapping onto the frozen Spec 107 audit vocabulary so the audit line and the activity record agree. - internal/transport: per-call CallRecorder round tripper, outermost layer of every upstream HTTP/SSE client, so the response status survives mcp-go flattening non-2xx into an untyped string. - core.Client marks the call dispatched when it hands it to mcp-go and returns a typed timeout error that keeps its message. - ActivityRecord gains three omitempty fields; old records decode unchanged. isError results keep status error and are told apart by tool_error. - ActivityFilter, REST query params and CLI --error-class/--fault-domain. - docs and OAS regenerated.
Deploying mcpproxy-docs with
|
| Latest commit: |
9f24b0e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3fa72169.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://113-c-call-error-taxonomy.mcpproxy-docs.pages.dev |
… status on network, ignore reply POSTs in recorder, always install recorder on OAuth SSE, stamp rejected SSE and output-policy blocks
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 37616353564 --repo smart-mcp-proxy/mcpproxy-go
|
…nomy # Conflicts: # internal/upstream/core/client.go
Dumbris
enabled auto-merge (squash)
October 6, 2026 17:24
…nomy # Conflicts: # oas/docs.go
…Disconnect logs The debug/info log lines read the flag without its mutex, racing with acquireListToolsContext (DATA RACE on the Windows unit-test lane).
…ted after re-init ErrSessionReestablished (Spec 113-e) did not match transport.ErrSessionTerminated, so the call-error classifier (Spec 113-c) fell through to class http for a non-read-only call that hit a terminated session. The not-repeated error now matches both sentinels; its message is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A tool call that fails shows only
status=errorplus prose in the Activity log. Nobody can tell whether the network dropped, the upstream returned HTTP 502, the upstream answered with a JSON-RPC error, the tool reported a business error (isError:true), the session expired, auth failed, or mcpproxy itself refused the call. Gap G6 of the Anthropic MCP-proxy talk audit (Spec 113, slice 113-c).What changed
internal/callerr: a pure classifierClassify(result, err, facts)returningerror_class(network, timeout, http, jsonrpc, tool_error, session_terminated, auth, proxy_policy, proxy_internal, cancelled),fault_domain(upstream, proxy, client) andupstream_http_status. Rules follow FR-041 in order.Outcome.AuditClass()is a total mapping onto the frozen Spec 107 audit vocabulary;auditToolCalluses it, so the audit line and the activity record agree.internal/transport: a per-callCallRecorderround tripper, installed as the outermost layer inupstreamRoundTripper(covers everyCreateHTTPClientbranch and SSE). mcp-go flattens non-2xx responses into an untyped string, so the status is recorded at the transport. Requests without a recorder in their context are untouched.core.Client.CallToolmarks the call dispatched at the exact point it hands it to mcp-go (every transport) and returns a typed timeout error with an unchanged message (it was a barefmt.Errorfthat dropped the chain).storage.ActivityRecordgainserror_class,fault_domain,upstream_http_status, allomitempty. A pre-113 JSON fixture round-trips byte-identical.isErrorresults keepstatus=errorand are told apart bytool_error.call_tool_*, legacycall_tool, direct surface,code_executionsub-calls, REST/api/v1/tools/call); limiter sheds are stampedproxy_policy/proxy.contracts.ActivityRecord, SSE completion payload (forwarded verbatim by/events),error_class/fault_domainquery filters on list and export.make swaggerrun,oas/committed. CLI:activity list|export --error-class --fault-domain,activity showprints anError classline,-o json|yamlcarry the fields. Docs updated.Tests run
go test -race(no skips needed): callerr, transport, storage, contracts, httpapi, audit, upstream/core, cmd/... all green.go test -race -tags serverwith the CI skip regex: runtime, upstream/..., serveredition/... green;internal/servergreen when run alone (1150 s). A combined run with runtime hit the 20 m timeout from CPU contention on the shared machine.isError, 502 with untyped error,-32001after a recorded 200, narrow fallback list),AuditClassmapping, recorder installed in every client branch and concurrency isolation, record JSON compatibility, runtime emit->persist, and anhttptestStreamable HTTP upstream integration test (SC-004) asserting persisted record andGET /api/v1/activityJSON for isError, 502 HTML, JSON-RPC -32001, connection reset, timeout, 404 session, 401, pre-dispatch validation and success, plus direct / code_execution / REST write paths.--build-tags server): no findings in touched files (remaining findings are pre-existing, in untouched files)../scripts/test-api-e2e.shon an isolated port with unique results/log paths (another session was running the same script concurrently; shared/tmpfiles and fixture port 39933 made the stock run flaky on both this branch andorigin/main): 65/66; the one failure,Audit log: server-edition binary present, also fails onorigin/main(no server-edition binary built).Assumptions
network/upstream; an error completion that no path classified isproxy_internal/proxy; ablockedcompletion with no noted outcome isproxy_policy/proxy.connection refused|reset,no such host,broken pipe,unexpected eof,i/o timeout,Client.Timeout exceeded,context deadline exceeded, consulted only when the call never left the proxy or HTTP requests went out with no response, so an upstream's own message is never reclassified.*transport.Errorwrapper counts asnetworkonly after the recorded non-2xx rule, because mcp-go wraps HTTP errors in it (found by the integration test).cmd/generate-typesdoes not generateActivityRecord, so the hand-writtenfrontend/src/types/api.tsinterface got the three optional fields instead.call_tool_*wrapper records (internal_tool_call) and activity replay are not stamped; the canonicaltool_callrecord is.internal/health/**andinternal/upstream/managed/client.gountouched.Spec: #1495
Review: opencode github-copilot/gpt-6.1-sol, 2 rounds, clean; follow-ups: none (round 1 rejected: *transport.Error-before-http ordering is an intentional documented deviation)