This project has moved. For the latest updates, please go here.

Does not handle derived exceptions properly

Aug 24, 2009 at 5:04 PM

I think there is a serious limitation with this component in that the logging of exceptions (which presumably is its primary purpose) is very basic.

This is because it appears to treat all exceptions as if the only relevant information was the Exception Type, Message and Stack Trace. However there are a number of derived exceptions which have extra properties containing important information.

For example, 

   SqlException contains properties for LineNumber, Source and Procedure

  FileNotFoundException contains FileName ,

  ExternalException contains ErrorCode 

  FileWebRequest contains RequestUri, Credentials, Proxy, Headers and a whole load more

etc

None of these properties will be included in the exception report.

In my view, it would be better if reflection was used determine at runtime which properties and fields are available and include them in the log/email.

 


Coordinator
Aug 25, 2009 at 1:12 AM
Edited Aug 25, 2009 at 7:29 AM

Hi,

I've seen what you're describing. That is, an Exception derived class that contains other exceptions, embedded as miscellaneous properties in a derived class. Microsoft is the only one I know who does it - which might explain a lot.

BTW I wouldn't call it a "serious limitation" because this product has been around for something like 5 years, with no one requesting or mentioning it. So let's just call it a "limitation".

ExceptionReporter is for reporting exceptions to people - not necessarily for "logging" as you say. Maybe there's a misunderstanding here.

In any case, I'm not sure I agree with the underlying principle. When an Exception is logged (with Message/Inner Exceptions and StackTrace) - it should contain everything to aid in resolving the issue. I would question the design of an Exception that hides important information within custom properties and fails to include that information in the base properties (even if just in the form of a JSON-like string) .

At this stage, I think it follows that the onus is on the developer who is dealing with such custom exceptions, to catch and parse them before passing them in. ExceptionReporter supports multiple exceptions so you could throw everything into an array.

I'm not convinced that it's the job of a generic Exception tool to deal with strange designs like this.

But maybe it's a neat feature? I'm open to any more opinions.

I can't help feeling that an exception catching strategy that doesn't already handle such custom exceptions, and wrap them up in business knowledge can't be a good idea anyway....  eg Shouldn't you be catching SqlExceptions at the point of occurrence, grabbing the properties you're interested and logging it or wrapping it up and/or re-throwing it from there?

Aug 25, 2009 at 11:10 AM

 

Well I disagree with a lot of what you say.
> I've seen what you’re describing. That is, an Exception derived class that contains other exceptions, embedded as miscellaneous properties in a derived class.
Not exactly, the properties I am describing are not other exceptions, but are details relating to the error. For example reporting an ExternalException or Win32Exception wfthout including the integer property ErrorCode is losing information information
> BTW I wouldn’t call ft a “serious limftation’ because this product has been around for something like 5 years, wfth no one requesting or mentioning ft. So let’sjust call ft a “limftation’.
Fair enough. Lets say ft could have been a serious limftation to me. But since the source is available (thanks) I shouldn’t complain.
> Microsoft is the only one I know who does ft - which might explain a lot
If by ‘it’ you mean embedding other exceptions as properties, then you might be right, but they are certainly not the only people that define there own exceptions to provide extra information as properties.
> ExceptionReporter is for reporting exceptions to people - not necessarily for “logging” as you say. Maybe there’s a misunderstanding here.
Well, the purpose is to report errors to the developers who will want to know as much information as possible.
> In any case, I'm not sure I agree wfth the underlying principle. When an Exception is logged (wfth Message/Inner Exceptions and StackTrace) - ft should contain everything to aid in resolving the issue. I would question the design of an Exception that hides important information within custom properties and fails to include that information in the base properties (even ifjust in the form of a JSON-like string) Aqain I disaqree. ft qoes aqainst the whole principle of object-orientated desiqn to create properties to store every possible piece of information in the base class. And a JSON-like strinq is (IMHO) a last resort option.
Again I disagree, ft goes against the whole principle of object-orientated design to create properties to store every possible piece of information in the base class. And a JSON-like string is (IMHO) a last resort option.
> At this stage, I think ft follows that the onus is on the developer who is dealing wfth such custom exceptions, to catch and parse them before passing them in. ExceptionReporter supports muftiple exceptions so you could throw everything into an array.
Unless you are designed a library or component then you have have to ‘deal’ wfth such custom exceptions, whether they come from the third party components, the Framework or the operating system. And because the developer can not possibly know what type exception will be thrown, then only way they can deal wfth ft is to use reflection to read the properties and fields. If a developer is going to go to that trouble. and given that the source code of this component is available, ft would make more sense to do the reflection inside this component rather than do ft outside and then have to wrap all this information up in an object and pass to this component and then un-wrap the details inside the component and include in the email.
> I’m not convinced that ft’s the job of a generic Exception tool to deal wfth strange designs like this.
Disagree totally. Firstly, I don’t think ft is strange design or even bad design, but even if ft was, because ft is Microsoft’s design and has become the de-facto standard.
> I cant help feeling that an exception catching strategy that doesn’t already handle such custom exceptions, and wrap them up in business knowledge cant be a good idea anyway.... eg Shouldnt you be catching SqlExceptions at the point of occurrence, grabbing the properties you’re interested and logging ft or wrapping ft up and/or re-throwing ft from there?
Here I sort of agree, but if I need to added extra information to an exception, I would expect to define my own Exception Class based on the original exception and pass this object to the logging , reporting tool.
For example. I use the DevExpress components, which have qufte of few custom exceptions class defined. Now I can’t recall ever getting any of this errors and certainly don’t want to have a lot of code to ‘handle’ them , but if they ever did occur to the point where I needed to contain DevExpress for support, then they would want to know the full details of the exception. After all that is the whole point to defining this custom exceptions
I guess this basically comes down to : You think this stuff should be outside the component whereas I think ft should be inside.
 

Well I disagree with a lot of what you say.

> I've seen what you're describing. That is, an Exception derived class that contains other exceptions, embedded as miscellaneous properties in a derived class

Not exactly, the properties I am describing are not other exceptions, but are details relating to the error. For example reporting an ExternalException or Win32Exception without including the integer property ErrorCode is losing information information

Microsoft is the only one I know who does it - which might explain a lot.

If by ‘it’ you mean embedding other exceptions as properties, then you might be right, but they are certainly not the only people that define their own exceptions to provide extra information as properties.

> BTW I wouldn't call it a "serious limitation" because this product has been around for something like 5 years, with no one requesting or mentioning it. So let's just call it a "limitation".

Fair enough. Lets say it could have been a serious limitation to me. But since the source is available I shouldn’t complain. (btw should say thanks to you and Simon

> ExceptionReporter is for reporting exceptions to people - not necessarily for "logging" as you say. Maybe there's a misunderstanding here.

Well, the purpose is to report errors to the developers who will want to know as much information as possible.

> In any case, I'm not sure I agree with the underlying principle. When an Exception is logged (with Message/Inner Exceptions and StackTrace) - it should contain everything to aid in resolving the issue. I would question the design of an Exception that hides important information within custom properties and fails to include that information in the base properties (even if just in the form of a JSON-like string) .

Again I disagree. It goes against the whole principle of object-orientated design to create properties to store every possible piece of information in the base class. And a JSON-like string is (IMHO) a last resort option.

> At this stage, I think it follows that the onus is on the developer who is dealing with such custom exceptions, to catch and parse them before passing them in. ExceptionReporter supports multiple exceptions so you could throw everything into an array.

Unless you are designed a library or component then you have have to ‘deal’ with such custom exceptions, whether they come from the third party components, the Framework or the operating system. And because the developer can not possibly know what type exception will be thrown, then only way they can deal with them is to use reflection to read the properties and fields. If a developer is going to go to that trouble. and given that the source code of this component is available, it would make more sense to do the reflection inside this component rather than do it outside and then have to wrap all this information up in an object and pass to this component and then un-wrap the details inside the component and include in the email.

> I'm not convinced that it's the job of a generic Exception tool to deal with strange designs like this.

Disagree totally. Firstly, I don’t think it is strange design or even bad design, but even if it was, because it is Microsoft’s design and has become the de-facto standard , and generic Exception tool should not ignore it.

> I can't help feeling that an exception catching strategy that doesn't already handle such custom exceptions, and wrap them up in business knowledge can't be a good idea anyway....  eg Shouldn't you be catching SqlExceptions at the point of occurrence, grabbing the properties you're interested and logging it or wrapping it up and/or re-throwing it from there?

Here I sort of agree, but if I need to added extra information to an exception, I would define my own Exception Class (based on the original exception with properties to store the extra information) and pass this object to the logging/reporting tool.

Another example. I use the DevExpress components, which have quite of few custom exceptions class defined. Now I can’t recall ever getting any of this errors and certainly don’t want to have a lot of code to ‘handle’ them , but if they ever did occur and if I  needed to contain DevExpress for support, then they would want to know the full details of the exception. After all that is the whole point to defining this custom exceptions. IMHO, a reporting tool should do this by default

I guess this basically comes down to : You think this stuff should be outside the component whereas I think it should be inside.

 

 

Developer
Aug 26, 2009 at 12:06 AM

Ok i think we should log all exception info.

I also agree with Peter (Panda) that, ideally, is should not be our job to do this.

I was hoping that things like SQLException would override ToString to include all extra info, unfortunatly this is not the case.

So since MS has not given us an easy way to do it, and it should not be the job of the consuming developer, I think we should resort to reflection.

What do u think Peter?

 

Coordinator
Aug 26, 2009 at 4:45 AM

Log4Net and all logging libraries do this as well.

log.Error(new Exception("hello"), "failed while trying to update report");

 

It takes an Exception argument but won't use reflection - just StackTrace, Message and InnerExceptions and then logs away.

Show me any logging/exception tool that takes an Exception as an argument, and that tries to predict what the developer wants by reflecting on custom properties, and I might change my mind ;-)

And if you find it, maybe we'll steal the code ;-)

Aug 26, 2009 at 9:45 AM

Simon,

Here is a quick way of doing this without resorting to reflection.
Because all exceptions are derived from Exception and Exception implements ISerializable you can do the following:

        class Program
	{
		static void Main(string[] args)
		{
			try
			{
				throw new COMException("My error", 100443);
			}
			catch (Exception ex)
			{
				ShowExceptionData(ex);
			}

			try
			{
				throw new FileNotFoundException("File not found", "My Missing File.txt");
			}
			catch (Exception ex)
			{
				ShowExceptionData(ex);
			}

			Console.Read();
		}

		private static void ShowExceptionData(Exception ex)
		{
			var serializationInfo = new SerializationInfo(ex.GetType(), new FormatterConverter());
			ex.GetObjectData(serializationInfo, new StreamingContext());

			Console.Out.WriteLine("-------- {0} -------", ex);
			foreach (var serializationEntry in serializationInfo)
			{
				Console.Out.WriteLine("{0}({1})={2}", serializationEntry.Name, serializationEntry.ObjectType, serializationEntry.Value);
			}
			Console.Out.WriteLine("-----------------------");
		}
	}

This works and you don't need reflection :)

Cheers
John

Aug 26, 2009 at 9:50 AM

Panda,

Just because other logging frameworks do not implement this functionality, I don't think you should not try, I think the more info you log the more the chances you have to reproduce and fix the error.

Cheers
John

 

Aug 26, 2009 at 12:27 PM

@johnsimons

An interesting approach to getting the exception's details.  However, your solution doesn't seem to walk through the inner exception's details.  I've used your code in the below, which walks the exceptions...

This gives an output like:

 

-------------------------------------------
Level 1:
System.ArgumentNullException: argX ---> System.IO.FileNotFoundException: File not found
File name: 'My Missing File.txt' ---> System.NotImplementedException: This is not implemeneted.
   --- End of inner exception stack trace ---
-------------------------------------------
  ClassName(System.String)=System.ArgumentNullException
  Message(System.String)=argX
  Data(System.Collections.IDictionary)=
  InnerException(System.Exception)=System.IO.FileNotFoundException: File not found
File name: 'My Missing File.txt' ---> System.NotImplementedException: This is not implemeneted.
  HelpURL(System.String)=
  StackTraceString(System.String)=
  RemoteStackTraceString(System.String)=
  RemoteStackIndex(System.Int32)=0
  ExceptionMethod(System.String)=
  HResult(System.Int32)=-2147467261
  Source(System.String)=
  ParamName(System.String)=
-------------------------------------------
Level 2:
System.IO.FileNotFoundException: File not found
File name: 'My Missing File.txt' ---> System.NotImplementedException: This is not implemeneted.
-------------------------------------------
  ClassName(System.String)=System.IO.FileNotFoundException
  Message(System.String)=File not found
  Data(System.Collections.IDictionary)=
  InnerException(System.Exception)=System.NotImplementedException: This is not implemeneted.
  HelpURL(System.String)=
  StackTraceString(System.String)=
  RemoteStackTraceString(System.String)=
  RemoteStackIndex(System.Int32)=0
  ExceptionMethod(System.String)=
  HResult(System.Int32)=-2147024894
  Source(System.String)=
  FileNotFound_FileName(System.String)=My Missing File.txt
  FileNotFound_FusionLog(System.String)=
-------------------------------------------
Level 3:
System.NotImplementedException: This is not implemeneted.
-------------------------------------------
  ClassName(System.String)=System.NotImplementedException
  Message(System.String)=This is not implemeneted.
  Data(System.Collections.IDictionary)=
  InnerException(System.Exception)=
  HelpURL(System.String)=
  StackTraceString(System.String)=
  RemoteStackTraceString(System.String)=
  RemoteStackIndex(System.Int32)=0
  ExceptionMethod(System.String)=
  HResult(System.Int32)=-2147467263
  Source(System.String)=
-------------------------------------------

Sean

 

 

   /// <summary>
   /// Writes exception details to a string.
   /// </summary>
   public class ExceptionWriter
   {
      private readonly ExceptionEnumerator enumerator;
      public ExceptionWriter(Exception root)
      {
         enumerator = new ExceptionEnumerator(root);
         var builder = new StringBuilder();
         var level = 0;
         foreach (var exception in enumerator)
         {
            WriteExceptionData(exception, builder, ++level);
         }
         builder.AppendLine("-------------------------------------------");
         Details = builder.ToString();
      }

      /// <summary>
      /// Gets the exception details, including nested exception details.
      /// </summary>
      /// <value>The details.</value>
      public string Details { get; private set; }

      private void WriteExceptionData(Exception ex, StringBuilder builder, int level)
      {
         var serializationInfo = new SerializationInfo(ex.GetType(), new FormatterConverter());
         ex.GetObjectData(serializationInfo, new StreamingContext());
         builder.AppendLine("-------------------------------------------");
         builder.AppendLine(string.Format("Level {0}:", level));
         builder.AppendLine(ex.ToString());
         builder.AppendLine("-------------------------------------------");
         foreach (var serializationEntry in serializationInfo)
         {
            builder.AppendLine(string.Format("  {0}({1})={2}", serializationEntry.Name, serializationEntry.ObjectType, serializationEntry.Value));
         }
      }
   }

   /// <summary>
   /// Enumerates an exception tree through <see cref="Exception.InnerException"/>.
   /// </summary>
   public class ExceptionEnumerator: IEnumerable<Exception>
   {
      private readonly Exception root;
      public ExceptionEnumerator(Exception root)
      {
         this.root = root;
      }

      public IEnumerator<Exception> GetEnumerator()
      {
         return Harvest(root).GetEnumerator();
      }

      private IEnumerable<Exception> Harvest(Exception e)
      {
         if (e == null)
         {
            yield break;
         }

         yield return e;
         foreach (var inner in Harvest(e.InnerException))
         {
            yield return inner;
         }
      }

      IEnumerator IEnumerable.GetEnumerator()
      {
         return GetEnumerator();
      }
   }

Some tests too...

 

   [TestFixture]
   public class Test
   {
      private ExceptionEnumerator enumerator;
      private ExceptionWriter writer;
      private Exception root;
      private Exception child1;
      private Exception child2;

      [SetUp]
      public void Setup()
      {
         child2 = new NotImplementedException("This is not implemeneted.");
         child1 = new FileNotFoundException("File not found", "My Missing File.txt", child2);
         root = new ArgumentNullException("argX", child1);
         enumerator = new ExceptionEnumerator(root);
         writer = new ExceptionWriter(root);

         // Assert that the fixture is set up as expected.
         Assert.IsNotNull(root);
         Assert.AreEqual(child1, root.InnerException);
         Assert.AreEqual(child2, child1.InnerException);
         Assert.IsNotNull(enumerator);
         Assert.IsNotNull(writer);
      }

      [Test]
      public void exception_enumerator_enumerates_the_exception_tree()
      {
         Assert.AreEqual(3, enumerator.Count());
      }

      [Test]
      public void exception_enumerator_enumerates_from_outer_to_inner()
      {
         var exceptions = enumerator.ToArray();
         Assert.AreEqual(root, exceptions[0]);
         Assert.AreEqual(child1, exceptions[1]);
         Assert.AreEqual(child2, exceptions[2]);
      }

      [Test]
      public void exception_writer_writes_a_string()
      {
         Assert.IsNotNullOrEmpty(writer.Details);
         Console.WriteLine(writer.Details); // So we can read the string.
      }
   }
{end}

 

 

 

 

 

 

Aug 26, 2009 at 10:00 PM

I know, and it also doesn't handle complex types like the SqlException has an Errors property that returns SqlErrorCollection.
I'm not sure how to handle complex types yet, anyone?

For inner exceptions, we can just use recursion to do the same we do to the parent exception.

 

Coordinator
Aug 27, 2009 at 2:08 AM
Edited Aug 27, 2009 at 2:10 AM
johnsimons wrote:

Panda,

Just because other logging frameworks do not implement this functionality, I don't think you should not try,

Sorry, that's not my point. The point is that it's widely accepted that handling custom properties on custom exceptions is the responsibility of the implementing developer.

I refer to log4net and all other frameworks as evidence that everyone sees this. The developer who is receiving custom exceptions should catch and wrap them up in a way that can be generically reported.

I love great features, but this just seems like feature creep. It is complicated and of no use to most people. More importantly, even if it would be useful, it would be much simpler for everybody if they just did the work of wrapping the exception themselves. I would argue that an exception handling block that catches custom exceptions and parses them into a meaningful generic structure - would be much better practice anyway.

There are probably complications that arise too eg What if some properties contain private info that users shouldn't see? We'll also need a Custom Exception Property Filter feature...

 

 

 

Aug 27, 2009 at 6:57 AM

Have to say that I agree with Panda on this (and following log4net's lead is standing on the shoulders of a giant!).  

Maybe Panda saying "an exception handling block that catches custom exceptions and parses them into a meaningful generic structure - would be much better practice anyway". is a good starter for an ExceptionReporter.Contrib project?!?