Skip to content

Commit c8db92b

Browse files
committed
Extract methods that call user-provided functions
There are three reasons for this change. 1. The code is easier to read 2. The [SuppressMessage] attribute is narrower and won't accidentally suppress another `catch (Exception)` 3. It works around a bug in Stryker.NET, see stryker-mutator/stryker-net#1845
1 parent a2b0375 commit c8db92b

File tree

1 file changed

+44
-21
lines changed

1 file changed

+44
-21
lines changed

src/Log4NetTextFormatter.cs

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,6 @@ private static string LogLevel(LogEventLevel level)
197197
/// <param name="properties">The collection of properties to write.</param>
198198
/// <param name="machineNameProperty">The machine name property to write or <see langword="null"/> if doesn't exist.</param>
199199
/// <remarks>https://github.com/apache/logging-log4net/blob/rel/2.0.8/src/Layout/XmlLayout.cs#L262-L286</remarks>
200-
[SuppressMessage("Microsoft.Design", "CA1031", Justification = "Protecting from user-provided code which might throw anything")]
201200
private void WriteProperties(LogEvent logEvent, XmlWriter writer, IEnumerable<KeyValuePair<string, LogEventPropertyValue>> properties, LogEventPropertyValue? machineNameProperty)
202201
{
203202
WriteStartElement(writer, "properties");
@@ -209,17 +208,7 @@ private void WriteProperties(LogEvent logEvent, XmlWriter writer, IEnumerable<Ke
209208
}
210209
foreach (var (propertyName, propertyValue) in properties.Select(e => (e.Key, e.Value)))
211210
{
212-
bool includeProperty;
213-
try
214-
{
215-
includeProperty = _options.FilterProperty(logEvent, propertyName);
216-
}
217-
catch (Exception filterException)
218-
{
219-
Debugging.SelfLog.WriteLine($"[{GetType().FullName}] An exception was thrown while filtering property '{propertyName}'. Including the property in the log4net event.\n{filterException}");
220-
includeProperty = true;
221-
}
222-
if (includeProperty)
211+
if (IncludeProperty(logEvent, propertyName))
223212
{
224213
WriteProperty(writer, propertyName, propertyValue);
225214
}
@@ -228,6 +217,27 @@ private void WriteProperties(LogEvent logEvent, XmlWriter writer, IEnumerable<Ke
228217
writer.WriteEndElement();
229218
}
230219

220+
/// <summary>
221+
/// Determine whether the property should be included with the user-provided <see cref="PropertyFilter"/>.
222+
/// </summary>
223+
/// <param name="logEvent">The log event.</param>
224+
/// <param name="propertyName">The property name.</param>
225+
/// <returns><see langword="true"/> if the property must be included in the log4net properties, <see langword="false"/> otherwise.</returns>
226+
/// <remarks>If the property filter throws an exception, a self-log is emitted and the property is included.</remarks>
227+
[SuppressMessage("Microsoft.Design", "CA1031", Justification = "Protecting from user-provided code which might throw anything")]
228+
private bool IncludeProperty(LogEvent logEvent, string propertyName)
229+
{
230+
try
231+
{
232+
return _options.FilterProperty(logEvent, propertyName);
233+
}
234+
catch (Exception filterException)
235+
{
236+
Debugging.SelfLog.WriteLine($"[{GetType().FullName}] An exception was thrown while filtering property '{propertyName}'. Including the property in the log4net event.\n{filterException}");
237+
return true;
238+
}
239+
}
240+
231241
/// <summary>
232242
/// Render a <see cref="LogEventPropertyValue"/> as <see cref="string"/>, without quotes if the value is a <see cref="ScalarValue"/> containing a <see cref="string"/>.
233243
/// This is appropriate to use for an XML attribute.
@@ -362,28 +372,41 @@ private void WriteMessage(LogEvent logEvent, XmlWriter writer)
362372
/// <param name="logEvent">The log event.</param>
363373
/// <param name="writer">The XML writer.</param>
364374
/// <remarks>https://github.com/apache/logging-log4net/blob/rel/2.0.8/src/Layout/XmlLayout.cs#L288-L295</remarks>
365-
[SuppressMessage("Microsoft.Design", "CA1031", Justification = "Protecting from user-provided code which might throw anything")]
366375
private void WriteException(LogEvent logEvent, XmlWriter writer)
367376
{
368377
var exception = logEvent.Exception;
369378
if (exception == null)
370379
{
371380
return;
372381
}
373-
string formattedException;
382+
var formattedException = FormatException(exception);
383+
if (formattedException != null)
384+
{
385+
var elementName = UsesLog4JCompatibility ? "throwable" : "exception";
386+
WriteContent(writer, elementName, formattedException);
387+
}
388+
}
389+
390+
/// <summary>
391+
/// Formats the given <paramref name="exception"/> with the user-provided <see cref="ExceptionFormatter"/>.
392+
/// </summary>
393+
/// <param name="exception">The exception to format.</param>
394+
/// <returns>The formatted exception.</returns>
395+
/// <remarks>
396+
/// If the exception formatter throws an exception, a self-log is emitted and
397+
/// the default formatting is applied, i.e. <c>exception.ToString()</c>.
398+
/// </remarks>
399+
[SuppressMessage("Microsoft.Design", "CA1031", Justification = "Protecting from user-provided code which might throw anything")]
400+
private string? FormatException(Exception exception)
401+
{
374402
try
375403
{
376-
formattedException = _options.FormatException(exception);
404+
return _options.FormatException(exception);
377405
}
378406
catch (Exception formattingException)
379407
{
380408
Debugging.SelfLog.WriteLine($"[{GetType().FullName}] An exception was thrown while formatting an exception. Using the default exception formatter.\n{formattingException}");
381-
formattedException = exception.ToString();
382-
}
383-
if (formattedException != null)
384-
{
385-
var elementName = UsesLog4JCompatibility ? "throwable" : "exception";
386-
WriteContent(writer, elementName, formattedException);
409+
return exception.ToString();
387410
}
388411
}
389412

0 commit comments

Comments
 (0)