mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-26 21:47:13 +00:00
fix(analyze): break self-referencing loop in correlation allMatches list
Previous fix slimmed evidence + added a size guard, deployed (47ffb348), did NOT solve the real problem. Re-queueing the poison message after deploy hit the same StoreRuleResultAsync false return — but the App Insights exception trail this time exposed the actual underlying error: Newtonsoft.Json.JsonSerializationException: Self referencing loop detected with type 'System.Collections.Generic.Dictionary<String,Object>'. Path 'enforcement_error_resolved.allMatches'. Pre-existing latent bug in EvaluateEventCorrelationCondition since v1: matchedPairs[0] IS primary by reference. primary["allMatches"] = matchedPairs (or a Take(N).ToList() slice that still contains the same reference) creates primary -> allMatches[0] -> primary, which Newtonsoft refuses to serialize. JsonConvert.SerializeObject throws inside StoreRuleResultAsync, the catch returns false, and the queue handler propagates a retry — eventually hitting poison after 5 attempts. Why this never fired before: correlation rules typically produce 0 or 1 match, and the if (matchedPairs.Count > 1) branch was dead code in production. The 12-day stuck session that surfaced this was the first real session with multiple correlated app_install_failed/completed pairs. Fix: clone each pair into a new Dictionary<string, object> when populating allMatches. The clones are independent instances with the same data, so Newtonsoft sees them as distinct objects and the cycle is broken. Pair flat-clone is sufficient — pairs only contain primitive values + small strings, no nested dicts. Also kept: the slim evidence (no message, no errorDetail) and the 60KB size guard from47ffb348. Both still good defense-in-depth — the size guard never got a chance to trigger before because serialize threw first. Test: RuleEngineCorrelationEvidenceSlimTests.Correlation_with_multiple_pairs_serializes_without_self_referencing_loop runs JsonConvert.SerializeObject(MatchedConditions) end-to-end and fails fast if the loop sneaks back in. 774/774 backend pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1 +1 @@
|
||||
444
|
||||
445
|
||||
|
||||
@@ -1 +1 @@
|
||||
1205
|
||||
1206
|
||||
|
||||
+33
@@ -3,6 +3,7 @@ using AutopilotMonitor.Shared.DataAccess;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Moq;
|
||||
using Newtonsoft.Json;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Tests;
|
||||
|
||||
@@ -76,6 +77,38 @@ public class RuleEngineCorrelationEvidenceSlimTests
|
||||
Assert.True((bool)primary["matchesTruncated"]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Correlation_with_multiple_pairs_serializes_without_self_referencing_loop()
|
||||
{
|
||||
// Regression pin: production session d434a3ba (2026-04-28) hit
|
||||
// "Self referencing loop detected with type 'Dictionary<String,Object>'.
|
||||
// Path 'enforcement_error_resolved.allMatches'."
|
||||
// because matchedPairs[0] WAS primary by reference, and primary["allMatches"] = matchedPairs
|
||||
// looped primary -> allMatches[0] -> primary. Cloning the dicts in allMatches breaks the
|
||||
// cycle. This test fails fast if a future refactor re-introduces the shared reference.
|
||||
var rule = BuildCorrelationRule();
|
||||
var baseTime = new DateTime(2026, 4, 28, 12, 0, 0, DateTimeKind.Utc);
|
||||
var events = new List<EnrollmentEvent>();
|
||||
for (var i = 0; i < 5; i++)
|
||||
{
|
||||
events.Add(BuildEvent($"evt-A-{i}", "app_install_failed", appId: $"app-{i}", sequence: i * 2, timestamp: baseTime.AddSeconds(i * 10)));
|
||||
events.Add(BuildEvent($"evt-B-{i}", "app_install_completed", appId: $"app-{i}", sequence: i * 2 + 1, timestamp: baseTime.AddSeconds(i * 10 + 5)));
|
||||
}
|
||||
|
||||
var (engine, _) = SutWithRule(rule, events);
|
||||
var outcome = await engine.AnalyzeSessionAsync(TenantId, SessionId);
|
||||
|
||||
Assert.Single(outcome.Results);
|
||||
|
||||
// The actual production failure was JsonConvert.SerializeObject throwing
|
||||
// JsonSerializationException at the StoreRuleResultAsync persist step. Reproduce that
|
||||
// exact serialize call here — if the loop sneaks back, this throws.
|
||||
var json = JsonConvert.SerializeObject(outcome.Results[0].MatchedConditions);
|
||||
Assert.False(string.IsNullOrEmpty(json));
|
||||
Assert.Contains("allMatches", json);
|
||||
Assert.Contains("totalMatches", json);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Correlation_with_few_pairs_does_not_truncate()
|
||||
{
|
||||
|
||||
@@ -668,21 +668,23 @@ namespace AutopilotMonitor.Functions.Services
|
||||
// Cap protects against runaway evidence size — sessions with hundreds of correlated
|
||||
// pairs would otherwise produce MatchedConditionsJson > 64KB and fail Table Storage
|
||||
// persistence (see also defense-in-depth check in TableStorageService.StoreRuleResultAsync).
|
||||
//
|
||||
// CRITICAL: allMatches MUST be a list of cloned dictionaries, not the matchedPairs
|
||||
// list itself. matchedPairs[0] IS primary by reference — if we assigned the raw list,
|
||||
// primary["allMatches"][0] would point back to primary and Newtonsoft.Json would
|
||||
// throw "Self referencing loop detected" on the StoreRuleResultAsync UpsertEntity
|
||||
// serialize step. The flat clone via `new Dictionary<string, object>(p)` snapshots
|
||||
// each pair's data BEFORE primary["allMatches"] is set, so the clones never contain
|
||||
// the back-reference. Pre-existing latent bug from before slim-down — never fired
|
||||
// in production until a 12-day stuck session matched multiple correlation pairs.
|
||||
const int MaxAllMatches = 10;
|
||||
var primary = matchedPairs[0];
|
||||
primary["totalMatches"] = matchedPairs.Count;
|
||||
if (matchedPairs.Count > 1)
|
||||
{
|
||||
if (matchedPairs.Count > MaxAllMatches)
|
||||
{
|
||||
primary["allMatches"] = matchedPairs.Take(MaxAllMatches).ToList();
|
||||
primary["matchesTruncated"] = true;
|
||||
}
|
||||
else
|
||||
{
|
||||
primary["allMatches"] = matchedPairs;
|
||||
primary["matchesTruncated"] = false;
|
||||
}
|
||||
var capped = matchedPairs.Take(MaxAllMatches);
|
||||
primary["allMatches"] = capped.Select(p => new Dictionary<string, object>(p)).ToList();
|
||||
primary["matchesTruncated"] = matchedPairs.Count > MaxAllMatches;
|
||||
}
|
||||
return (true, primary);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user