views:

1168

answers:

4

Hello,

I have started to make heavy use of exceptions, I'm sure I'll grow out of it as a learn the advantages and disadvantages the hard way, but until I've become an exception guru I'd like to know if this technique is acceptable.

I intend to wrap, say a database exception in my own 'SorryFailedToSaveYourData' exception and then recursively moved through the exception displaying the messages, kinda like this:

Try
    DoSomeWork
Catch
    BuildErrorMessage(lblError,ex)
End Try

Public Sub BuildErrorMessage(ByVal lbl As Label, ByVal ex As Exception)
    lbl.Text += "<br />" & ex.Message
    While Not ex.InnerException Is Nothing
        BuildErrorMessage(lbl, ex.InnerException)
    End While
End Sub

Is this practice useful or I have I completely missed the boat when it comes to handling exceptions? I know you can create your own exceptions but it seems like overkill for the size of the projects we are working on.

Thanks

A: 

You're building a label inside of an exception, presumably because you want to display the message to the user. If your exception spans more than one tier of the application (you mentioned wrapping your own data exception), then this is a terrible idea as it will violate encapsulation and force your exception handling to be using UI elements.

Generally, though, you'll want to not display error messages to the user. What does a user care that a stack overflow happened? He only cares that the application is broken and that an error report was logged and sent to the appropriate team.

Generally you want to have exceptions if and only if the application is in a state in which it can no longer run. If it's something that just happens conditionally you'll want to handle it explicitly with conditions to check for it rather than using the exception mechanism.

For example, if you know users will be connecting from home and you can't guarantee there will be an Internet connection, you check for an Internet connection and display a message saying the user needs to connect, instead of just trying to use an Internet connection and creating an exception if none exists.

Welbog
A: 

Edit

I misread your question at first, you probally don't want to do this if your displaying the information to the end user. You will want to do what I am suggesting if your going to log the exception for you to troubleshoot latter.

====

This is standard although I would also capture the stack trace and any items in the Data property (Although I've not seen any framework code use this you never know and its useful if your going throw your own exceptions or rethrow an exception.

Although your actual code has an error in it your probally end up in an infinite loop since your outer exception will always have an inner exception.

Public Sub BuildErrorMessage(ByVal lbl As Label, ByVal ex As Exception)   
 lbl.Text += "<br />" & ex.Message    
 If Not ex.InnerException Is Nothing     Then
    BuildErrorMessage(lbl, ex.InnerException)    
  End If 
End Sub
JoshBerke
+2  A: 

ex.ToString() will build the exception text formatted, you should use that rather than the .Message as it might not give you the full information you will need for debugging.

I would also recommend passing a stringbuilder down the recursion instead of a label using AppendLine(), this way you can also use that text to send email notifications, write the event viewer, as well as a log file. For displaying in your UI (it looks like you want html) simply set the labels text value to

StringBuilder.ToString().Replace(Environment.NewLine, "<br />")
Tom Anderson
+2  A: 

I wouldn't display each and every inner exception message to the user of your application. I would catch the real exception (ie SqlException) and rethrow it as your own exception (ie throw new YouCantSaveThisTwiceException("This customer ID already exists") ). Then just display that hand typed message to the user on a nice pretty display screen. Send yourself an email / log file / etc with the full-on stack trace.

This serves two purposes:

1) You get to see the real stack trace which makes it easier for you to find and fix the problem.

2) The users do not have to be shown a big scary stack trace. Instead, they read a nice message written by you which reassures them that although there is a problem, they did not "break" the application.

This works well for exceptions which you don't expect a user to be able to fix with different input. For exceptions which validate user input, often you can catch those in the presentation layer and display your nice message back to them on a label without leaving the form.

Only (ONLY!!!) use this technique if you know exactly what you are going to be catching and you give the users clear instruction on how to fix it. Otherwise, you end up with the worst of both worlds. The users don't know what to do, and you are not alerted of the presence of a bug. Never wrap an entire method in a try-catch simply to "catch all" exceptions. That type of handling should be done globally at the application level.

I do all of my global exception handling (emailing, logging, etc) in the application_error handler method within Global.asax. You could also create an HttpModule which handles application errors. Then you can carry that functionality with you from project to project.

Chad Braun-Duin