fix(siteruntime): M2.12 review nits — observe logger fault + meaningful source fallback (#25)
Replace bare task-discard with ContinueWith(OnlyOnFaulted|ExecuteSynchronously) so a
faulted ISiteEventLogger is logged and swallowed rather than going to the unobserved-task
firehose. Replace the "ScriptRuntimeContext" class-name fallback with the meaningful
"InstanceScript:{instanceName}" identifier (matching the site-event-log source convention).
Update the method doc-comment to state the best-effort contract explicitly. Pin the new
fallback value in the shape-precision test.
This commit is contained in:
@@ -250,14 +250,38 @@ public class ScriptRuntimeContext
|
||||
/// <summary>
|
||||
/// M2.12 (#25): fire-and-forget emission of a <c>script</c> Error site event
|
||||
/// for a recursion-limit violation. Mirrors the call shape used by
|
||||
/// <c>ScriptExecutionActor</c>'s catch blocks (WP-32 / M1.8). The returned
|
||||
/// task is intentionally discarded by callers (<c>_ =</c>) so the event log
|
||||
/// never blocks or faults the throw that follows. A null logger is a no-op.
|
||||
/// <c>ScriptExecutionActor</c>'s catch blocks (WP-32 / M1.8). A fault from
|
||||
/// the site-event logger is observed-and-dropped (best-effort) via
|
||||
/// <c>ContinueWith(OnlyOnFaulted)</c> — it never blocks or faults the
|
||||
/// <c>_logger.LogError</c> + throw path that follows. A null logger is a no-op.
|
||||
/// </summary>
|
||||
private Task EmitRecursionLimitEventAsync(string msg) =>
|
||||
_siteEventLogger?.LogEventAsync(
|
||||
"script", "Error", _instanceName, _sourceScript ?? "ScriptRuntimeContext", msg)
|
||||
?? Task.CompletedTask;
|
||||
private void EmitRecursionLimitEventAsync(string msg)
|
||||
{
|
||||
if (_siteEventLogger == null)
|
||||
return;
|
||||
|
||||
var source = string.IsNullOrEmpty(_instanceName)
|
||||
? "recursion-guard"
|
||||
: $"InstanceScript:{_instanceName}";
|
||||
|
||||
var logTask = _siteEventLogger.LogEventAsync("script", "Error", _instanceName, source, msg);
|
||||
if (!logTask.IsCompleted)
|
||||
{
|
||||
logTask.ContinueWith(
|
||||
t => _logger.LogWarning(t.Exception,
|
||||
"Site event log write failed for recursion-limit violation on instance '{Instance}'",
|
||||
_instanceName),
|
||||
CancellationToken.None,
|
||||
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
|
||||
TaskScheduler.Default);
|
||||
}
|
||||
else if (logTask.IsFaulted)
|
||||
{
|
||||
_logger.LogWarning(logTask.Exception,
|
||||
"Site event log write failed for recursion-limit violation on instance '{Instance}'",
|
||||
_instanceName);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Gets the current value of an attribute from the Instance Actor.
|
||||
@@ -333,7 +357,7 @@ public class ScriptRuntimeContext
|
||||
$"CallScript('{scriptName}') rejected at depth {nextDepth}.";
|
||||
_logger.LogError(msg);
|
||||
// M2.12 (#25): emit to site event log in addition to ILogger; fire-and-forget.
|
||||
_ = EmitRecursionLimitEventAsync(msg);
|
||||
EmitRecursionLimitEventAsync(msg);
|
||||
throw new InvalidOperationException(msg);
|
||||
}
|
||||
|
||||
@@ -498,7 +522,7 @@ public class ScriptRuntimeContext
|
||||
_logger.LogError(msg);
|
||||
// M2.12 (#25): emit to site event log via the parent context's
|
||||
// helper — single emission path, fire-and-forget.
|
||||
_ = _context.EmitRecursionLimitEventAsync(msg);
|
||||
_context.EmitRecursionLimitEventAsync(msg);
|
||||
throw new InvalidOperationException(msg);
|
||||
}
|
||||
|
||||
|
||||
@@ -143,6 +143,7 @@ public class RecursionLimitSiteEventTests
|
||||
string? capturedCategory = null;
|
||||
string? capturedSeverity = null;
|
||||
string? capturedInstance = null;
|
||||
string? capturedSource = null;
|
||||
string? capturedMessage = null;
|
||||
|
||||
var loggerMock = new Mock<ISiteEventLogger>();
|
||||
@@ -157,6 +158,7 @@ public class RecursionLimitSiteEventTests
|
||||
capturedCategory = cat;
|
||||
capturedSeverity = sev;
|
||||
capturedInstance = inst;
|
||||
capturedSource = src;
|
||||
capturedMessage = msg;
|
||||
})
|
||||
.Returns(Task.CompletedTask);
|
||||
@@ -169,6 +171,8 @@ public class RecursionLimitSiteEventTests
|
||||
Assert.Equal("script", capturedCategory);
|
||||
Assert.Equal("Error", capturedSeverity);
|
||||
Assert.Equal(InstanceName, capturedInstance);
|
||||
// Source fallback: no sourceScript wired, so the helper uses "InstanceScript:{instanceName}".
|
||||
Assert.Equal($"InstanceScript:{InstanceName}", capturedSource);
|
||||
Assert.NotNull(capturedMessage);
|
||||
Assert.Contains("CallScript", capturedMessage);
|
||||
Assert.Contains("2", capturedMessage); // maxCallDepth in the message
|
||||
|
||||
Reference in New Issue
Block a user