Compiling all Exception messages into a string

Question

Below is a recursive function I've written that's responsible for compiling a string consisting of a Exception's message, and also each of that Exception's InnerException messages.

I've tried to condense everything to one line, but am unsure if this is the most efficient way of approaching this problem.

public static string GetExceptionMessages(Exception exception, int msgCount = 1)
{
    return exception != null ? string.Format("{0}: {1}\n{2}", msgCount, exception.Message, GetExceptionMessages(exception.InnerException, ++msgCount)) : string.Empty;
}

And here is code showing the above method in action:

public static void Main(string[] args)
{
    try
    {
        Throw1();
    }
    catch (Exception e)
    {
        string messages = GetExceptionMessages(e);
        Console.WriteLine(messages);
    }
    Console.ReadLine();
}
public static void Throw1()
{
    try
    {
        Throw2();
    }
    catch (Exception e)
    {
        throw new Exception("Throw1", e);
    }
}
public static void Throw2()
{
    throw new Exception("Throw2");
}
public static string GetExceptionMessages(Exception exception, int msgCount = 1)
{
    return exception != null ? string.Format("{0}: {1}\n{2}", msgCount, exception.Message, GetExceptionMessages(exception.InnerException, ++msgCount)) : string.Empty;
}

Answer 1

Readability first

I've tried to condense everything to one line, but am unsure if this is the most efficient way of approaching this problem.

Your primary goal when writing code is readability and maintainability and not squeezing everything into single lines.

---

Can you read this?

return exception != null ? string.Format("{0}: {1}\n{2}", msgCount, exception.Message, GetExceptionMessages(exception.InnerException, ++msgCount)) : string.Empty;

I cannot. This line is way too long even on my large monitor.

How about this? A little bit better, but still, the third line is too long even with string interpolation instead of string.Format

return
    exception is null
        ? string.Empty
        : $"{messageCount}: {exception.Message}{Environment.NewLine}{GetExceptionMessages(exception.InnerException, ++messageCount)}";

---

Separation

We need to change the strategy because your method is doing two things at a time:

• it's looking for exceptions and
• it's building a string

We need to separate them by implementing an extension that will enumerate exceptions:

public static IEnumerable<Exception> DescendantsAndSelf(this Exception exception)
{
    do
    {
        yield return exception;
        exception = exception.InnerException;
    } while (!(exception is null));
}

Then we rewrite the string building method as an extension of the first one:

public static string StringifyMessages(this IEnumerable<Exception> exceptions)
{
    var messages = exceptions.Select((e, i) => $"{i + 1}: {e.Message}")
    return string.Join(Environment.NewLine, messages);
}

So, finally you can chain them like this:

catch (Exception e)
{
    Console.WriteLine(e.DescendantsAndSelf().StringifyMessages());
}

---

Advantages

This means more code now but this code is testable, modular and can be reused for other things and extensions.

Answer 2

Review

• If the purpose is to output each of the descendant inner exceptions, you are missing all except the first inner exception in case of an AggregateException. And this is a common exception.
• Don't use \n as new line unless you specifically need to comply to this style of new line. Prefer Environment.NewLine.
• I am not convinced of the exception != null -> string.Empty clause. This seems to be coded just so you can make a one-liner method implementation. I would check for exception.InnerException == null to short-circuit the return value.
• msgCount is not hierarchical, so it does not make much sense when dealing with exception trees (see AggregateException). Consider using a count tree ("1.1.4", etc..) or not using a count at all. Perhaps an indentation suits the layout better.
• Does exception.ToString() provide sufficient information for you? It handles inner exceptions and inner exception trees.

Alternative

This is your code reworked to take into account all the above.

public static string GetExceptionMessages(Exception exception, string indent = "\t")
{
    exception = exception ?? throw new ArgumentNullException(nameof(exception));
    indent = indent ?? string.Empty;
    var builder = new StringBuilder();
    GetExceptionMessages(exception, builder, indent, new Stack<string>());
    return builder.ToString();
}

private static void GetExceptionMessages(
    Exception exception, StringBuilder builder, string indent, Stack<string> currentIndent)
{
    if (exception == null) return;
    builder.AppendLine($"{string.Join(string.Empty, currentIndent)}{exception.Message}");
    currentIndent.Push(indent);
    if (exception is AggregateException aggregateException)
    {
        foreach (var innerException in aggregateException.InnerExceptions)
        {
            GetExceptionMessages(innerException, builder, indent, currentIndent);
        }
    }
    else
    {
        GetExceptionMessages(exception.InnerException, builder, indent, currentIndent);
    }
    currentIndent.Pop();
}

Answer 3

Note that some messages from exceptions expand to more lines (e.g. ArgumentNullException), so consider to replace new line characters while formatting the message so each exception message is written neatly in one line:

exception.Message.Replace(Environment.NewLine, " ")

---

My version would look something like:

public static string GetMessageTree(this Exception ex, string indent = "")
{
  if (ex == null) return string.Empty;

  if (ex is AggregateException aggregate)
    return aggregate.GetMessageTree(indent);

  return string.Format("{0}{1}{3}{2}", 
    indent, 
    ex.Message.Replace(Environment.NewLine, ": "), 
    ex.InnerException.GetMessageTree($"{indent}    "), 
    Environment.NewLine);
}

public static string GetMessageTree(this AggregateException ex, string indent = "")
{
  if (ex == null) return string.Empty;

  StringBuilder builder = new StringBuilder();
  builder.AppendLine($"{indent}{ex.Message} ->");

  foreach (Exception childEx in ex.InnerExceptions)
  {
    builder
      .AppendLine(
        childEx
        .GetMessageTree($"{indent}    ")
        .TrimEnd(Environment.NewLine.ToCharArray()));
  }

  return builder.ToString();
}

Because the Polymorphism doesn't seem to work in extension methods it is necessary to check for AggregateException in the bass class extension