fix(witan): the concurrency probe reported "Server returned an error response" and nothing else - #212
Merged
Merged
Conversation
…d nothing else That sentence is `MCPError.__str__`, which is its `message` alone. It is equally true of an expired credential, a queued request cancelled by a gateway, a backend crash and a 502 HTML page, so a failing worker's record said nothing about which of those happened. Tracing the write ceiling on the deployed CI service therefore had to be done from cluster logs, re-deriving what the exception had been carrying all along. Record the JSON-RPC `code`, any `data`, an HTTP status if the transport kept the response, and the causal chain. The code is read off whichever link CARRIES it rather than off the outermost exception, because witan wraps the transport fault in a RemoteUnreachable whose prose was all the reader ever saw and which has no code of its own; and the walk follows `.exceptions` as well as `__cause__`, since anyio re-raises transport faults through an ExceptionGroup that `__cause__` alone stops at. With that in place the same run reports `error_code=-32603` — a server-side internal error, not the transport failure its wrapper claimed. Also record what the 2026-08-09 run found, since the docstring's whole point is that a later run has something to differ from: the vMCP OOMKill is gone (0 restarts across ~150 sessions where 32 concurrent previously killed it), A/B/C pass at 4/12/6, and the remaining ceiling is 36 concurrent WRITES being cancelled into 502s — a deadline, not a crash. And note the probe's own local limit: 36 separate interpreters cannot connect inside the default 20s lead, so that FAIL is a verdict on this machine, not on the server. Refs: tk-deployed-witan-saturates-at-24-32-concurrent-con-8e4afc Refs: tk-concurrent-writes-to-the-deployed-witan-are-canc-02abbd Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UsPuQ4Qu44oqxnHMKD6uQY
Contributor
There was a problem hiding this comment.
Pull request overview
Improves concurrency-probe diagnostics by exposing structured exception details and causal chains.
Changes:
- Captures JSON-RPC codes, error data, HTTP statuses, and exception chains.
- Prints structured diagnostics for failed workers.
- Adds focused error-detail tests and operational findings.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
mcp/servers/witan/witan/scripts/concurrency_probe.py |
Adds detailed failure reporting and updated probe findings. |
mcp/servers/witan/tests/test_concurrency_probe_errors.py |
Tests structured exception extraction. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…TP burst to this probe Both from review on #212, both correct. ── the walk followed one branch and called it "every reachable failure" ── `_error_detail` stepped into `exceptions[0]` and, having found a member, never looked at the group's own `__cause__`. A task group that loses several workers reports them in completion order, so the member carrying the JSON-RPC code is routinely not the first; and anyio raises the group *from* the fault that triggered it, putting the code under the group rather than inside it. Either shape reported no detail at all, which reads as "the exception had nothing to give" — the precise failure this helper was added to stop. `witan_core.remote.proxy._chain` already walks every member AND every cause/context, and is what the proxy classifies faults with. Use it instead of a second, worse traversal. Two tests cover the shapes the hand-rolled walk missed. ── and the operational numbers were not this probe's ── The docstring credited "36 concurrent memory_store" and "36 concurrent memory_search" to a 24-writer/12-reader run. This probe cannot produce either: one store per writer, and per reader `n_reads` SEQUENTIAL reads, so that run is 24 concurrent writes and at most 12 simultaneous reads. The counts came from a separate raw-HTTP burst — N connections, one initialize plus one tools/call each — which is why they mean what they say. Labelled as a separate experiment with its shape described, so the baseline can be reproduced rather than assumed. Noted there too: check the response BODY, not the status. A wrong tool name returns HTTP 200 carrying a JSON-RPC error, and an early burst "passed" at N=64 while calling a tool the deployed vMCP does not expose — it exposes them unprefixed. Also pinned the local-ceiling note to what was actually observed (18 workers made the 20s lead, 36 did not) rather than a vague threshold, and the ~150 sessions to the window they were counted over. Refs: tk-concurrent-writes-to-the-deployed-witan-are-canc-02abbd Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UsPuQ4Qu44oqxnHMKD6uQY
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
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.
What
The concurrency probe's failure record was
str(exc)— and forMCPErrorthatis its
messagealone: the sentence "Server returned an error response". Thatsentence is equally true of an expired credential, a request cancelled by a
gateway, a backend crash and a 502 HTML page, so a failing worker's row said
nothing about which had happened.
This records what the exception was already carrying: the JSON-RPC
code, anydata, an HTTP status when the transport kept the response, and the causalchain.
Two details are load-bearing:
exception — witan wraps the transport fault in a
RemoteUnreachablewhoseprose was all the reader ever saw, and which has no code of its own;
.exceptionsas well as__cause__, because anyiore-raises transport faults through an
ExceptionGroupthat__cause__alone stops at.
With that in place the same failing run reports
error_code=-32603— aserver-side internal error, not the transport failure its wrapper claimed.
Why now
Tracing the deployed service's write ceiling had to be done from cluster logs,
re-deriving facts the client already had. Findings from that session are also
recorded in the module docstring, whose stated purpose is that a later run has
something to differ from:
concurrent previously killed the aggregator (ol-infrastructure #5320);
with nothing restarting — a deadline, not a crash;
inside the default 20s lead, so that
FAILis a verdict on the machinerunning it, not on the server. Raise
--lead.Testing
just test-witan-council— 727 passed, including 5 new cases over_error_detail. Each new case is a shape the deployed stack actually producedon 2026-08-09.
Refs:
tk-deployed-witan-saturates-at-24-32-concurrent-con-8e4afc,tk-concurrent-writes-to-the-deployed-witan-are-canc-02abbd🤖 Generated with Claude Code
https://claude.ai/code/session_01UsPuQ4Qu44oqxnHMKD6uQY