Degrade transient optional list failures in Modern enumeration - #6348
Degrade transient optional list failures in Modern enumeration#6348lorenzozanee wants to merge 5 commits into
Conversation
A rate-limited Modern backend that answers an optional capability enumeration (resources/list, resources/templates/list or prompts/list) with a transient HTTP 429 was misclassified as ErrBackendUnavailable, flipping a reachable backend to unhealthy/degraded and aggregate Ready=false even though initialize and tools/list succeeded. Degrade a transient failure on an optional list to an empty result instead of failing the whole enumeration. tools/list stays fatal; the -32601 not-implemented degradation is unchanged. Fixes stacklok#6347 Signed-off-by: lorenzozanee <wyz0707@proton.me>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6348 +/- ##
==========================================
+ Coverage 77.75% 77.77% +0.02%
==========================================
Files 750 750
Lines 72601 72610 +9
==========================================
+ Hits 56448 56474 +26
+ Misses 16148 16131 -17
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hi @lorenzozanee, thanks for submitting this PR! Mind fixing the linting error? |
Extract the three nearly-identical optional-list degradation switches in modernEnumerate into a single modernListOptional helper, dropping the function's cyclomatic complexity back under the gocyclo threshold (15). Behavior is unchanged: a transient list failure degrades with a WARN, a -32601 degrades only for resources/templates/list (mirroring the Legacy path), and tools/list stays fatal.
|
Hi @reyortiz3 — thanks for the heads-up. The gocyclo lint error is fixed in Behavior is unchanged — a transient list failure still degrades with a WARN, I verified locally with the same golangci-lint version CI pins (v2.12.2): full-repo One note: the branch merged |
2 participants
Summary
A reachable Modern backend can be marked unavailable when an optional capability enumeration (resources/templates/list, resources/list or prompts/list) returns a transient HTTP 429. The 429 is classified as transient in the Modern layer, but the enumeration treated every error other than -32601 as fatal, so the transient failure surfaced as ErrBackendUnavailable and flipped the backend to unhealthy/degraded even though initialize and tools/list had succeeded. With several VirtualMCPServers probing the same rate-limited backend, aggregate Ready flips to False (BackendsDegraded).
A transient failure on an optional list now degrades to an empty result (with a WARN) instead of failing the whole enumeration. tools/list stays fatal: a backend that cannot list its tools is genuinely unavailable. The existing -32601 not-implemented degradation is unchanged.
Fixes #6347
Type of change
Test plan
task test)task test-e2e)task lint-fix)The regression tests fail on the base commit with the exact error from the report ("backend unavailable: failed to list resource templates ... modern backend returned a transient error: HTTP 429") and pass with the fix; the affected vmcp packages are green.
Special notes for reviewers
This fix targets the Modern path reported in #6347. The Legacy path still treats a 429 on optional enumeration as fatal; detecting the transient class there would require reaching into go-sdk internals and is left for a follow-up. This is orthogonal to #6246 (partialFailureMode wiring), which does not touch error classification.