Fixes to the Log4NetAdapter to prevent StackOverflowExceptions during…

… logging when AggregateExceptions being logged contain recursive inner exeptions. The maximum recursive level is 5.
zinofi · Mar 5, 2020 · 787b307 · 787b307
1 parent 51ba310
commit 787b307
Showing 1 changed file with 68 additions and 50 deletions.
diff --git a/Logging/src/Umbrella.Extensions.Logging.Log4Net/Log4NetAdapter.cs b/Logging/src/Umbrella.Extensions.Logging.Log4Net/Log4NetAdapter.cs
@@ -39,62 +39,80 @@ public bool IsEnabled(LogLevel logLevel)
             }
         }
 
-        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
-        {
-            if (!IsEnabled(logLevel))
-                return;
-
-            if (formatter == null)
-                throw new InvalidOperationException();
+		public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
+			=> LogInner(logLevel, eventId, state, exception, formatter);
 
-            //If the eventId is 0 check if the Name has a value as we have hijacked this to allow for recursive calls
-            //to this method to use the same id for correlating messages.
-            string messageId = eventId.Id == 0
-                ? string.IsNullOrWhiteSpace(eventId.Name) ? "Correlation Id: " + DateTime.UtcNow.Ticks.ToString() : eventId.Name
-                : eventId.Id.ToString();
+		private void LogInner<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter, int recursiveLevel = 0)
+		{
+			// Checking how many levels deep we are here to prevent a possible StackOverflowException.
+			if (recursiveLevel > 5)
+			{
+				if (IsEnabled(LogLevel.Warning))
+					m_Logger.Warn("Whilst logging an exception the recursion level exceeded 5. This indicates a problem with the way the AggregateException has been constructed.");
 
-            var messageBuider = new StringBuilder()
-                .AppendLine(messageId)
-                .Append(formatter(state, exception));
+				return;
+			}
 
-            string message = messageBuider.ToString();
+			if (!IsEnabled(logLevel))
+				return;
 
-            switch (logLevel)
-            {
-                case LogLevel.Trace:
-                case LogLevel.Debug:
-                    m_Logger.Debug(message, exception);
-                    break;
-                case LogLevel.Information:
-                    m_Logger.Info(message, exception);
-                    break;
-                case LogLevel.Warning:
-                    m_Logger.Warn(message, exception);
-                    break;
-                case LogLevel.Error:
-                    m_Logger.Error(message, exception);
-                    break;
-                case LogLevel.Critical:
-                    m_Logger.Fatal(message, exception);
-                    break;
-                default:
-                    m_Logger.Warn($"Encountered unknown log level {logLevel}, writing out as Info.");
-                    m_Logger.Info(message, exception);
-                    break;
-            }
+			if (formatter == null)
+				throw new InvalidOperationException();
 
-            // Log4Net doesn't seem to log AggregateExceptions properly so handling them manually here
-            if (!(exception is AggregateException aggregate))
-                aggregate = exception?.InnerException as AggregateException;
+			// If the eventId is 0 check if the Name has a value as we have hijacked this to allow for recursive calls
+			// to this method to use the same id for correlating messages.
+			string messageId = eventId.Id == 0
+				? string.IsNullOrWhiteSpace(eventId.Name) ? "Correlation Id: " + DateTime.UtcNow.Ticks.ToString() : eventId.Name
+				: eventId.Id.ToString();
 
-            if (aggregate?.InnerExceptions?.Count > 0)
-            {
-                foreach(Exception innerException in aggregate.InnerExceptions)
-                {
-                    Log(logLevel, new EventId(0, messageId), state, exception, formatter);
-                }
-            }
-        }
+			var messageBuider = new StringBuilder()
+				.AppendLine(messageId)
+				.Append(formatter(state, exception));
+
+			string message = messageBuider.ToString();
+
+			switch (logLevel)
+			{
+				case LogLevel.Trace:
+				case LogLevel.Debug:
+					m_Logger.Debug(message, exception);
+					break;
+				case LogLevel.Information:
+					m_Logger.Info(message, exception);
+					break;
+				case LogLevel.Warning:
+					m_Logger.Warn(message, exception);
+					break;
+				case LogLevel.Error:
+					m_Logger.Error(message, exception);
+					break;
+				case LogLevel.Critical:
+					m_Logger.Fatal(message, exception);
+					break;
+				default:
+					m_Logger.Warn($"Encountered unknown log level {logLevel}, writing out as Info.");
+					m_Logger.Info(message, exception);
+					break;
+			}
+
+			// Log4Net doesn't seem to log AggregateExceptions properly so handling them manually here
+			if (exception != null)
+			{
+				if (!(exception is AggregateException aggregate))
+					aggregate = exception?.InnerException as AggregateException;
+
+				if (aggregate?.InnerExceptions?.Count > 0)
+				{
+					foreach (Exception innerException in aggregate.InnerExceptions)
+					{
+						LogInner(logLevel, new EventId(0, messageId), state, exception, formatter, ++recursiveLevel);
+
+						// The call returned here so we can reduce by 1 to indicate we have moved back up.
+						recursiveLevel--;
+					}
+				}
+			}
+		}
         #endregion
     }
 }