Skip to content

Commit 49ce1d7

Browse files
Copilotmo-esmp
Copilot
and
mo-esmp
authored
Fix KeyNotFoundException in enrichers when accessing httpContext.Items (#56)
* Fix KeyNotFoundException in all enrichers by using TryGetValue instead of direct dictionary access ------ Co-authored-by: Mohsen Esmailpour <[email protected]>
1 parent 7d79cb7 commit 49ce1d7

File tree

6 files changed

+162
-120
lines changed

6 files changed

+162
-120
lines changed

‎src/Serilog.Enrichers.ClientInfo/Enrichers/ClientHeaderEnricher.cs‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,16 @@
44

55
namespace Serilog.Enrichers;
66

7-
/// <inheritdoc/>
7+
/// <inheritdoc />
88
public class ClientHeaderEnricher : ILogEventEnricher
99
{
1010
private readonly string _clientHeaderItemKey;
11-
private readonly string _propertyName;
12-
private readonly string _headerKey;
1311
private readonly IHttpContextAccessor _contextAccessor;
12+
private readonly string _headerKey;
13+
private readonly string _propertyName;
1414

1515
/// <summary>
16-
/// Initializes a new instance of the <see cref="ClientHeaderEnricher"/> class.
16+
/// Initializes a new instance of the <see cref="ClientHeaderEnricher" /> class.
1717
/// </summary>
1818
/// <param name="headerKey">The key of the header.</param>
1919
/// <param name="propertyName">The name of the property.</param>
@@ -25,9 +25,7 @@ public ClientHeaderEnricher(string headerKey, string propertyName)
2525
internal ClientHeaderEnricher(string headerKey, string propertyName, IHttpContextAccessor contextAccessor)
2626
{
2727
_headerKey = headerKey;
28-
_propertyName = string.IsNullOrWhiteSpace(propertyName)
29-
? headerKey.Replace("-", "")
30-
: propertyName;
28+
_propertyName = string.IsNullOrWhiteSpace(propertyName) ? headerKey.Replace("-", "") : propertyName;
3129
_clientHeaderItemKey = $"Serilog_{headerKey}";
3230
_contextAccessor = contextAccessor;
3331
}
@@ -37,25 +35,23 @@ internal ClientHeaderEnricher(IHttpContextAccessor contextAccessor)
3735
_contextAccessor = contextAccessor;
3836
}
3937

40-
/// <inheritdoc/>
38+
/// <inheritdoc />
4139
public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory)
4240
{
43-
var httpContext = _contextAccessor.HttpContext;
44-
if (httpContext == null)
45-
{
46-
return;
47-
}
41+
HttpContext httpContext = _contextAccessor.HttpContext;
42+
if (httpContext == null) return;
4843

49-
if (httpContext.Items[_clientHeaderItemKey] is LogEventProperty logEventProperty)
44+
if (httpContext.Items.TryGetValue(_clientHeaderItemKey, out object value) &&
45+
value is LogEventProperty logEventProperty)
5046
{
5147
logEvent.AddPropertyIfAbsent(logEventProperty);
5248
return;
5349
}
5450

55-
var headerValue = httpContext.Request.Headers[_headerKey].ToString();
51+
string headerValue = httpContext.Request.Headers[_headerKey].ToString();
5652
headerValue = string.IsNullOrWhiteSpace(headerValue) ? null : headerValue;
5753

58-
var logProperty = new LogEventProperty(_propertyName, new ScalarValue(headerValue));
54+
LogEventProperty logProperty = new(_propertyName, new ScalarValue(headerValue));
5955
httpContext.Items.Add(_clientHeaderItemKey, logProperty);
6056

6157
logEvent.AddPropertyIfAbsent(logProperty);

‎src/Serilog.Enrichers.ClientInfo/Enrichers/ClientIpEnricher.cs‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
using Microsoft.AspNetCore.Http;
1+
using System.Runtime.CompilerServices;
2+
using Microsoft.AspNetCore.Http;
23
using Serilog.Core;
34
using Serilog.Events;
4-
using System.Runtime.CompilerServices;
55

66
[assembly: InternalsVisibleTo("Serilog.Enrichers.ClientInfo.Tests")]
77

88
namespace Serilog.Enrichers;
99

10-
/// <inheritdoc/>
10+
/// <inheritdoc />
1111
public class ClientIpEnricher : ILogEventEnricher
1212
{
1313
private const string IpAddressPropertyName = "ClientIp";
@@ -16,7 +16,7 @@ public class ClientIpEnricher : ILogEventEnricher
1616
private readonly IHttpContextAccessor _contextAccessor;
1717

1818
/// <summary>
19-
/// Initializes a new instance of the <see cref="ClientIpEnricher"/> class.
19+
/// Initializes a new instance of the <see cref="ClientIpEnricher" /> class.
2020
/// </summary>
2121
public ClientIpEnricher() : this(new HttpContextAccessor())
2222
{
@@ -27,23 +27,19 @@ internal ClientIpEnricher(IHttpContextAccessor contextAccessor)
2727
_contextAccessor = contextAccessor;
2828
}
2929

30-
/// <inheritdoc/>
30+
/// <inheritdoc />
3131
public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory)
3232
{
33-
var httpContext = _contextAccessor.HttpContext;
34-
if (httpContext == null)
35-
{
36-
return;
37-
}
33+
HttpContext httpContext = _contextAccessor.HttpContext;
34+
if (httpContext == null) return;
3835

39-
var ipAddress = httpContext.Connection.RemoteIpAddress?.ToString() ?? "unknown";
36+
string ipAddress = httpContext.Connection.RemoteIpAddress?.ToString() ?? "unknown";
4037

41-
if (httpContext.Items[IpAddressItemKey] is LogEventProperty logEventProperty)
38+
if (httpContext.Items.TryGetValue(IpAddressItemKey, out object value) &&
39+
value is LogEventProperty logEventProperty)
4240
{
4341
if (!((ScalarValue)logEventProperty.Value).Value.ToString()!.Equals(ipAddress))
44-
{
4542
logEventProperty = new LogEventProperty(IpAddressPropertyName, new ScalarValue(ipAddress));
46-
}
4743

4844
logEvent.AddPropertyIfAbsent(logEventProperty);
4945
return;

‎src/Serilog.Enrichers.ClientInfo/Enrichers/CorrelationIdEnricher.cs‎

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,28 @@
1-
using Microsoft.AspNetCore.Http;
1+
using System;
2+
using Microsoft.AspNetCore.Http;
3+
using Microsoft.Extensions.Primitives;
24
using Serilog.Core;
35
using Serilog.Events;
4-
using System;
56

67
namespace Serilog.Enrichers;
78

8-
/// <inheritdoc/>
9+
/// <inheritdoc />
910
public class CorrelationIdEnricher : ILogEventEnricher
1011
{
1112
private const string CorrelationIdItemKey = "Serilog_CorrelationId";
1213
private const string PropertyName = "CorrelationId";
13-
private readonly string _headerKey;
1414
private readonly bool _addValueIfHeaderAbsence;
1515
private readonly IHttpContextAccessor _contextAccessor;
16+
private readonly string _headerKey;
1617

1718
/// <summary>
18-
/// Initializes a new instance of the <see cref="CorrelationIdEnricher"/> class.
19+
/// Initializes a new instance of the <see cref="CorrelationIdEnricher" /> class.
1920
/// </summary>
2021
/// <param name="headerKey">
21-
/// The header key used to retrieve the correlation ID from the HTTP request or response headers.
22+
/// The header key used to retrieve the correlation ID from the HTTP request or response headers.
2223
/// </param>
2324
/// <param name="addValueIfHeaderAbsence">
24-
/// Determines whether to add a new correlation ID value if the header is absent.
25+
/// Determines whether to add a new correlation ID value if the header is absent.
2526
/// </param>
2627
public CorrelationIdEnricher(string headerKey, bool addValueIfHeaderAbsence)
2728
: this(headerKey, addValueIfHeaderAbsence, new HttpContextAccessor())
@@ -35,44 +36,34 @@ internal CorrelationIdEnricher(string headerKey, bool addValueIfHeaderAbsence, I
3536
_contextAccessor = contextAccessor;
3637
}
3738

38-
/// <inheritdoc/>
39+
/// <inheritdoc />
3940
public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory)
4041
{
41-
var httpContext = _contextAccessor.HttpContext;
42-
if (httpContext == null)
43-
{
44-
return;
45-
}
42+
HttpContext httpContext = _contextAccessor.HttpContext;
43+
if (httpContext == null) return;
4644

47-
if (httpContext.Items[CorrelationIdItemKey] is LogEventProperty logEventProperty)
45+
if (httpContext.Items.TryGetValue(CorrelationIdItemKey, out object value) &&
46+
value is LogEventProperty logEventProperty)
4847
{
4948
logEvent.AddPropertyIfAbsent(logEventProperty);
5049
return;
5150
}
5251

53-
var requestHeader = httpContext.Request.Headers[_headerKey];
54-
var responseHeader = httpContext.Response.Headers[_headerKey];
52+
StringValues requestHeader = httpContext.Request.Headers[_headerKey];
53+
StringValues responseHeader = httpContext.Response.Headers[_headerKey];
5554

5655
string correlationId;
5756

5857
if (!string.IsNullOrWhiteSpace(requestHeader))
59-
{
6058
correlationId = requestHeader;
61-
}
6259
else if (!string.IsNullOrWhiteSpace(responseHeader))
63-
{
6460
correlationId = responseHeader;
65-
}
6661
else if (_addValueIfHeaderAbsence)
67-
{
6862
correlationId = Guid.NewGuid().ToString();
69-
}
7063
else
71-
{
7264
correlationId = null;
73-
}
7465

75-
var correlationIdProperty = new LogEventProperty(PropertyName, new ScalarValue(correlationId));
66+
LogEventProperty correlationIdProperty = new(PropertyName, new ScalarValue(correlationId));
7667
logEvent.AddOrUpdateProperty(correlationIdProperty);
7768

7869
httpContext.Items.Add(CorrelationIdItemKey, correlationIdProperty);

‎test/Serilog.Enrichers.ClientInfo.Tests/ClientHeaderEnricherTests.cs‎

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using Microsoft.AspNetCore.Http;
22
using NSubstitute;
3+
using Serilog.Core;
34
using Serilog.Events;
45
using System;
56
using Xunit;
@@ -12,7 +13,7 @@ public class ClientHeaderEnricherTests
1213

1314
public ClientHeaderEnricherTests()
1415
{
15-
var httpContext = new DefaultHttpContext();
16+
DefaultHttpContext httpContext = new();
1617
_contextAccessor = Substitute.For<IHttpContextAccessor>();
1718
_contextAccessor.HttpContext.Returns(httpContext);
1819
}
@@ -21,15 +22,15 @@ public ClientHeaderEnricherTests()
2122
public void EnrichLogWithClientHeader_WhenHttpRequestContainHeader_ShouldCreateNamedHeaderValueProperty()
2223
{
2324
// Arrange
24-
var headerKey = "RequestId";
25-
var propertyName = "HttpRequestId";
26-
var headerValue = Guid.NewGuid().ToString();
25+
string headerKey = "RequestId";
26+
string propertyName = "HttpRequestId";
27+
string headerValue = Guid.NewGuid().ToString();
2728
_contextAccessor.HttpContext!.Request!.Headers[headerKey] = headerValue;
2829

29-
var clientHeaderEnricher = new ClientHeaderEnricher(headerKey, propertyName, _contextAccessor);
30+
ClientHeaderEnricher clientHeaderEnricher = new(headerKey, propertyName, _contextAccessor);
3031

3132
LogEvent evt = null;
32-
var log = new LoggerConfiguration()
33+
Logger log = new LoggerConfiguration()
3334
.Enrich.With(clientHeaderEnricher)
3435
.WriteTo.Sink(new DelegatingSink(e => evt = e))
3536
.CreateLogger();
@@ -48,14 +49,14 @@ public void EnrichLogWithClientHeader_WhenHttpRequestContainHeader_ShouldCreateN
4849
public void EnrichLogWithClientHeader_WhenHttpRequestContainHeader_ShouldCreateHeaderValueProperty()
4950
{
5051
// Arrange
51-
var headerKey = "RequestId";
52-
var headerValue = Guid.NewGuid().ToString();
52+
string headerKey = "RequestId";
53+
string headerValue = Guid.NewGuid().ToString();
5354
_contextAccessor!.HttpContext!.Request!.Headers[headerKey] = headerValue;
5455

55-
var clientHeaderEnricher = new ClientHeaderEnricher(headerKey, propertyName: string.Empty, _contextAccessor);
56+
ClientHeaderEnricher clientHeaderEnricher = new(headerKey, string.Empty, _contextAccessor);
5657

5758
LogEvent evt = null;
58-
var log = new LoggerConfiguration()
59+
Logger log = new LoggerConfiguration()
5960
.Enrich.With(clientHeaderEnricher)
6061
.WriteTo.Sink(new DelegatingSink(e => evt = e))
6162
.CreateLogger();
@@ -71,20 +72,21 @@ public void EnrichLogWithClientHeader_WhenHttpRequestContainHeader_ShouldCreateH
7172
}
7273

7374
[Fact]
74-
public void EnrichLogWithMultipleClientHeaderEnricher_WhenHttpRequestContainHeaders_ShouldCreateHeaderValuesProperty()
75+
public void
76+
EnrichLogWithMultipleClientHeaderEnricher_WhenHttpRequestContainHeaders_ShouldCreateHeaderValuesProperty()
7577
{
7678
// Arrange
77-
var headerKey1 = "Header1";
78-
var headerKey2 = "User-Agent";
79-
var headerValue1 = Guid.NewGuid().ToString();
80-
var headerValue2 = Guid.NewGuid().ToString();
79+
string headerKey1 = "Header1";
80+
string headerKey2 = "User-Agent";
81+
string headerValue1 = Guid.NewGuid().ToString();
82+
string headerValue2 = Guid.NewGuid().ToString();
8183
_contextAccessor!.HttpContext!.Request!.Headers[headerKey1] = headerValue1;
8284
_contextAccessor!.HttpContext!.Request!.Headers[headerKey2] = headerValue2;
83-
var clientHeaderEnricher1 = new ClientHeaderEnricher(headerKey1, propertyName: string.Empty, _contextAccessor);
84-
var clientHeaderEnricher2 = new ClientHeaderEnricher(headerKey2, propertyName: string.Empty, _contextAccessor);
85+
ClientHeaderEnricher clientHeaderEnricher1 = new(headerKey1, string.Empty, _contextAccessor);
86+
ClientHeaderEnricher clientHeaderEnricher2 = new(headerKey2, string.Empty, _contextAccessor);
8587

8688
LogEvent evt = null;
87-
var log = new LoggerConfiguration()
89+
Logger log = new LoggerConfiguration()
8890
.Enrich.With(clientHeaderEnricher1)
8991
.Enrich.With(clientHeaderEnricher2)
9092
.WriteTo.Sink(new DelegatingSink(e => evt = e))
@@ -106,11 +108,11 @@ public void EnrichLogWithMultipleClientHeaderEnricher_WhenHttpRequestContainHead
106108
public void EnrichLogWithClientHeader_WhenHttpRequestNotContainHeader_ShouldCreateHeaderValuePropertyWithNoValue()
107109
{
108110
// Arrange
109-
var headerKey = "RequestId";
110-
var clientHeaderEnricher = new ClientHeaderEnricher(headerKey, propertyName: string.Empty, _contextAccessor);
111+
string headerKey = "RequestId";
112+
ClientHeaderEnricher clientHeaderEnricher = new(headerKey, string.Empty, _contextAccessor);
111113

112114
LogEvent evt = null;
113-
var log = new LoggerConfiguration()
115+
Logger log = new LoggerConfiguration()
114116
.Enrich.With(clientHeaderEnricher)
115117
.WriteTo.Sink(new DelegatingSink(e => evt = e))
116118
.CreateLogger();
@@ -125,17 +127,40 @@ public void EnrichLogWithClientHeader_WhenHttpRequestNotContainHeader_ShouldCrea
125127
Assert.Null(evt.Properties[headerKey].LiteralValue());
126128
}
127129

130+
[Fact]
131+
public void EnrichLogWithClientIp_WhenKeyNotInItems_ShouldWorkCorrectly()
132+
{
133+
// Arrange
134+
string headerKey = "x-dummy-header";
135+
ClientHeaderEnricher clientHeaderEnricher = new(headerKey, string.Empty, _contextAccessor);
136+
137+
LogEvent evt = null;
138+
Logger log = new LoggerConfiguration()
139+
.Enrich.With(clientHeaderEnricher)
140+
.WriteTo.Sink(new DelegatingSink(e => evt = e))
141+
.CreateLogger();
142+
143+
log.Information("Testing log enricher.");
144+
145+
// Act - This should work without throwing any exceptions
146+
Exception exception = Record.Exception(() => log.Information("Test log message"));
147+
148+
// Assert
149+
Assert.Null(exception);
150+
Assert.NotNull(evt);
151+
}
152+
128153
[Fact]
129154
public void WithRequestHeader_ThenLoggerIsCalled_ShouldNotThrowException()
130155
{
131156
// Arrange
132-
var logger = new LoggerConfiguration()
157+
Logger logger = new LoggerConfiguration()
133158
.Enrich.WithRequestHeader("HeaderName")
134159
.WriteTo.Sink(new DelegatingSink(_ => { }))
135160
.CreateLogger();
136161

137162
// Act
138-
var exception = Record.Exception(() => logger.Information("LOG"));
163+
Exception exception = Record.Exception(() => logger.Information("LOG"));
139164

140165
// Assert
141166
Assert.Null(exception);

0 commit comments

Comments
 (0)