fix(esg): park buffered calls that fail deterministically with ArgumentException (path template/verb) instead of retrying to exhaustion (plan R2-06 T3)

2026-07-13 09:51:30 -04:00
parent 9668600c38
commit cc94b3a68c
2 changed files with 56 additions and 1 deletions
@@ -166,7 +166,11 @@ public class ExternalSystemClient : IExternalSystemClient
    /// Delivers a buffered ExternalSystem call during a store-and-forward
    /// retry sweep. Returns true on success, false on permanent failure (the message
    /// is parked); throws <see cref="TransientExternalSystemException"/> on a
-    /// transient failure so the engine retries.
+    /// transient failure so the engine retries. Deterministic-poison failures against
+    /// the CURRENT method definition — malformed JSON, or an
+    /// <see cref="ArgumentException"/> from an unresolvable <c>{param}</c> path
+    /// template or an unsupported HTTP verb — return false (park immediately) rather
+    /// than retry, because re-running the same stored payload can never succeed.
    /// </summary>
    /// <param name="message">The buffered message to deliver.</param>
    /// <param name="cancellationToken">Cancellation token.</param>
@@ -221,6 +225,21 @@ public class ExternalSystemClient : IExternalSystemClient
            _logger.LogError(ex, "Buffered call to '{System}' failed permanently; parking.", payload.SystemName);
            return false;
        }
+        catch (ArgumentException ex)
+        {
+            // N2: same poison-message shape as the JsonException catch above. BuildUrl's
+            // {param} path-template substitution (placeholder with no matching/non-null
+            // parameter) and ValidateHttpMethod both throw ArgumentException
+            // deterministically for the SAME stored payload — the method definition
+            // changed underneath the buffered call, and retrying can never succeed.
+            // Return false so the S&F engine parks the message instead of counting the
+            // throw as transient and burning MaxRetries.
+            _logger.LogError(
+                ex,
+                "Buffered call to '{System}'/'{Method}' fails deterministically against the current method definition; parking.",
+                payload.SystemName, payload.MethodName);
+            return false;
+        }
        // TransientExternalSystemException propagates — the S&F engine retries.
    }

@@ -1341,4 +1341,40 @@ public class ExternalSystemClientTests
            () => client.CallAsync("TestAPI", "getRecipe"));
        Assert.Contains("id", ex.Message);
    }
+
+    [Fact]
+    public async Task DeliverBuffered_PathTemplateNowUnresolvable_ReturnsFalseSoMessageParks()
+    {
+        // Arch-review R2 N2: a method whose path gained a {param} placeholder AFTER
+        // calls were buffered throws ArgumentException deterministically for every
+        // already-buffered message — it must park immediately (permanent), not burn
+        // MaxRetries "transient" attempts (the S&F sweep's catch-all retries any throw).
+        var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
+        var method = new ExternalSystemMethod("getRecipe", "GET", "/recipes/{id}") { Id = 1, ExternalSystemDefinitionId = 1 };
+        StubResolution(system, method);
+        _httpClientFactory.CreateClient(Arg.Any<string>())
+            .Returns(new HttpClient(new MockHttpMessageHandler(HttpStatusCode.OK, "{}")));
+        var client = new ExternalSystemClient(
+            _httpClientFactory, _repository, NullLogger<ExternalSystemClient>.Instance);
+
+        // BufferedCall carries Parameters:null — the {id} placeholder has no value.
+        var delivered = await client.DeliverBufferedAsync(BufferedCall("TestAPI", "getRecipe"));
+
+        Assert.False(delivered);
+    }
+
+    [Fact]
+    public async Task DeliverBuffered_UnsupportedVerbOnStoredMethod_ReturnsFalseSoMessageParks()
+    {
+        // ValidateHttpMethod throws the same deterministic ArgumentException shape.
+        var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
+        var method = new ExternalSystemMethod("oddVerb", "FOO", "/p") { Id = 1, ExternalSystemDefinitionId = 1 };
+        StubResolution(system, method);
+        var client = new ExternalSystemClient(
+            _httpClientFactory, _repository, NullLogger<ExternalSystemClient>.Instance);
+
+        var delivered = await client.DeliverBufferedAsync(BufferedCall("TestAPI", "oddVerb"));
+
+        Assert.False(delivered);
+    }
 }