fix(grpc): keep the RpcException off the failover warning so no sink can render Status.Detail
Review follow-ups: the warning now carries only the two endpoint URIs and the status code — attaching the exception object would let any real sink render its ToString(), which for RpcException includes server-authored Status.Detail. The leak pin test now also asserts no exception rides on the warning, via a new WarningExceptions capture on RecordingLogger. Plus two cosmetic nits: unified ThrowIfNull idiom, documented that FallbackEndpoints mutation after Validate is unsupported. Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
This commit is contained in:
+6
@@ -175,6 +175,12 @@ public sealed class FailoverSecretsHubReaderTests
|
||||
}
|
||||
|
||||
Assert.Equal(1, _logger.WarningCount);
|
||||
|
||||
// The exception object must not ride along either: sinks render an attached exception's
|
||||
// ToString(), which for RpcException includes Status.Detail — the very text the message
|
||||
// template deliberately excludes. A clean message with the exception attached would still
|
||||
// leak in production, so the warning must carry NO exception at all.
|
||||
Assert.All(_logger.WarningExceptions, Assert.Null);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
@@ -130,6 +130,14 @@ internal sealed class RecordingLogger<TCategory> : ILogger<TCategory>
|
||||
/// <summary>Formatted messages logged at <see cref="LogLevel.Warning"/>.</summary>
|
||||
public List<string> Warnings { get; } = [];
|
||||
|
||||
/// <summary>
|
||||
/// The exception object attached to each Warning entry, index-aligned with
|
||||
/// <see cref="Warnings"/>. Captured separately from the formatted message because real sinks
|
||||
/// ALSO render an attached exception's <c>ToString()</c> — for <c>RpcException</c> that includes
|
||||
/// <c>Status.Detail</c> — so a leak test that only scans the message would miss that channel.
|
||||
/// </summary>
|
||||
public List<Exception?> WarningExceptions { get; } = [];
|
||||
|
||||
/// <summary>Count of information entries.</summary>
|
||||
public int InformationCount => Information.Count;
|
||||
|
||||
@@ -161,6 +169,7 @@ internal sealed class RecordingLogger<TCategory> : ILogger<TCategory>
|
||||
break;
|
||||
case LogLevel.Warning:
|
||||
Warnings.Add(message);
|
||||
WarningExceptions.Add(exception);
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
|
||||
Reference in New Issue
Block a user