From 9549ff2c29f9f41f963a47cee017a16b571b3c5d Mon Sep 17 00:00:00 2001 From: ecency Date: Fri, 17 Jul 2026 05:28:09 +0000 Subject: [PATCH] fix: fail over on malformed 200 responses from Hive RPC nodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A node can return 200 with valid JSON carrying neither an error nor a usable result. That response was recorded as a success, so the health tracker kept the misbehaving node ranked first while every dependent call (token validation, account/global-props fetches) failed for the whole window — observed twice as multi-hour bursts of 401s on authenticated endpoints. Call() now takes an optional result-shape validator; the typed helpers require an array from get_accounts and an object from get_dynamic_global_properties. A failed check marks the node failed and advances immediately, and the surfaced error names the node, which the previous 'result is not iterable' message could not. --- CLAUDE.md | 2 +- .../EcencyApi.Tests/HiveRpcFailoverTests.cs | 54 +++++++++++++++++++ .../EcencyApi/Infrastructure/HiveRpcClient.cs | 23 ++++++-- 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8142ce3e..c38ec7b1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,7 +70,7 @@ Handlers are `public static async Task Name(HttpContext ctx)` methods on static `NodeHealthTracker` (adopted from the vision-next SDK) holds per-node health state: 429 responses park a node for `Retry-After` (or an escalating window), recent failures deprioritize it, and a latency EWMA orders the pool best-first with config order as tiebreak. Two clients build on it: -- `HiveRpcClient` (Hive JSON-RPC): RPC-level errors (JSON `error` field) surface immediately without failover — they're application errors, not node health. +- `HiveRpcClient` (Hive JSON-RPC): RPC-level errors (JSON `error` field) surface immediately without failover — they're application errors, not node health. The typed helpers additionally validate the result *shape* (`get_accounts` → array, `get_dynamic_global_properties` → object): a 200 with valid JSON but no usable result is a node failure that fails over — without this, a node serving malformed 200s is recorded as healthy and stays ranked first (observed in production as multi-hour windows of token-validation 401s). - `EngineRpcClient` (Hive-Engine): one instance per pool — the `/contracts` RPC pool and the history-API pool. The portfolio `Find` calls are fixed-shape queries that always yield a `result` array on a healthy node, so an error payload or non-JSON body *is* a node failure and rolls over to the next node. The raw passthroughs (`engine-api`, `engine-account-history`) fail over only on transport errors and 429/5xx; other responses belong to the caller's query and pipe as-is. Covered by `HiveRpcFailoverTests` and `EngineFailoverTests`; keep new failover behavior test-backed. diff --git a/dotnet/EcencyApi.Tests/HiveRpcFailoverTests.cs b/dotnet/EcencyApi.Tests/HiveRpcFailoverTests.cs index 326eb244..b8628a36 100644 --- a/dotnet/EcencyApi.Tests/HiveRpcFailoverTests.cs +++ b/dotnet/EcencyApi.Tests/HiveRpcFailoverTests.cs @@ -63,6 +63,13 @@ private async Task Loop() "{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":[{\"name\":\"served-by\",\"port\":\"" + Url + "\"}]}"); status = 200; } + else if (status == -3) + { + // Malformed 200: valid JSON, no error field, no usable result — + // the shape observed from misbehaving production nodes. + body = Encoding.UTF8.GetBytes("{\"jsonrpc\":\"2.0\",\"id\":1}"); + status = 200; + } else { body = Encoding.UTF8.GetBytes("rate limited"); @@ -171,6 +178,53 @@ public async Task ProvenSlowNode_IsDemotedByLatencyEwma() Assert.True(fast.Hits >= 1); } + [Fact] + public async Task MalformedResultNode_FailsOverWithoutRetry() + { + await using var malformed = new StubNode(() => -3); // 200, valid JSON, no result + await using var good = new StubNode(() => 200); + + var client = new HiveRpcClient(new[] { malformed.Url, good.Url }, timeoutMs: 1500); + + var accounts = await client.GetAccounts(new[] { "good-karma" }); + + Assert.NotNull(accounts); + Assert.Equal("served-by", accounts![0]!["name"]!.GetValue()); + // Validation failure advances immediately — one hit, no same-node retry. + Assert.Equal(1, malformed.Hits); + Assert.True(good.Hits >= 1); + } + + [Fact] + public async Task MalformedResultNode_IsDemotedOnSubsequentCalls() + { + await using var malformed = new StubNode(() => -3); + await using var good = new StubNode(() => 200); + + var client = new HiveRpcClient(new[] { malformed.Url, good.Url }, timeoutMs: 1500); + + await client.GetAccounts(new[] { "good-karma" }); + var malformedAfterFirst = malformed.Hits; + await client.GetAccounts(new[] { "good-karma" }); + + Assert.Equal(malformedAfterFirst, malformed.Hits); // recent failure: not tried again + Assert.True(good.Hits >= 2); + } + + [Fact] + public async Task AllNodesMalformed_ThrowsNamingTheNode() + { + await using var bad1 = new StubNode(() => -3); + await using var bad2 = new StubNode(() => -3); + + var client = new HiveRpcClient(new[] { bad1.Url, bad2.Url }, timeoutMs: 1500); + + var ex = await Assert.ThrowsAnyAsync(() => + client.GetAccounts(new[] { "good-karma" })); + Assert.Contains("unusable get_accounts result", ex.Message); + Assert.Contains("http://127.0.0.1:", ex.Message); // node URL for diagnosability + } + [Fact] public async Task AllNodesDown_Throws() { diff --git a/dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs b/dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs index 57938bf3..4d556c44 100644 --- a/dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs +++ b/dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs @@ -44,7 +44,14 @@ public RpcException(string message) : base(message) { } // ---- calls ------------------------------------------------------------- - public async Task Call(string api, string method, JsonNode @params) + /// Optional shape check for the RPC result. A node + /// can return 200 with valid JSON that carries neither an error nor a usable + /// result (observed in production as multi-hour windows of get_accounts + /// yielding no array); without validation that response counts as a SUCCESS, + /// so the health tracker keeps the poisoned node ranked first for the whole + /// window. A failed check is treated as node failure and fails over. + public async Task Call(string api, string method, JsonNode @params, + Func? validateResult = null) { var request = new JsonObject { @@ -69,6 +76,14 @@ public RpcException(string message) : base(message) { } try { var result = await CallNode(node, body); + if (validateResult != null && !validateResult(result)) + { + // Don't burn a same-node retry: a node serving malformed + // 200s keeps serving them (observed for hours at a time). + throw new NodeUnavailableException( + $"RPC node {node} returned unusable {method} result", + advanceImmediately: true); + } _health.RecordSuccess(nodeIndex, NowMs - started); return result; } @@ -204,12 +219,14 @@ public NodeUnavailableException(string message, bool advanceImmediately, // get_accounts([null]) is empty — so this distinction is load-bearing. nameArr.Add(n is null ? null : JsonValue.Create(n)); } - var result = await Call("condenser_api", "get_accounts", new JsonArray(nameArr)); + var result = await Call("condenser_api", "get_accounts", new JsonArray(nameArr), + r => r is JsonArray); return result as JsonArray; } public Task GetDynamicGlobalProperties() => - Call("condenser_api", "get_dynamic_global_properties", new JsonArray()); + Call("condenser_api", "get_dynamic_global_properties", new JsonArray(), + r => r is JsonObject); } ///