Best Practice for Exceptions

I'm not sure why, but I got in the habbit of doing this:
  Catch ex As Exception
   Throw New ApplicationException(ex.ToString)

I'm assuming, that because I'm not creating a New Object, this is better:
  Catch ex As Exception
   Throw ex

Am I correct?

Instead of catching an exception and throwing it again, can't you just not catch the exception?

Hmm..... Deep... Umm... yea... why didn't I think of that...

Ok, I can do that, I'm going to keep the try though.
Like this...
Try
....
Finally
....
End Try

Note that according to best practices, you shouldn't catch Exception and you shouldn't use ApplicationException at all.

If you are throwing a different exception in the catch handler of an exception (which is a perfectly valid approach in many cases, just don't throw ApplicationException), always specify the original exception by using the constructor overload that takes an innerexception.

And if you just want to rethrow the original exception for whatever reason, just saying "Throw" with nothing after it is enough, you don't need to say "Throw ex". And if that's the only statement in the catch block, as Tommy said, better omit it completely.

Ok... (I was taught wrong practices... time to clean 'em out.)

Let's say I have this:
  Catch ex As Exception
   Throw New ApplicationException(debugStr & "-" & Chr(10) & ex.ToString)


How would I use the original exception type?
(No, I DO NOT use debugStr all the time.)

What are you trying to accomplish here?

You shouldn't catch Exception. You don't have a clue what really happened if your in a catch block that caught exception, so how can you possibly recover sensibly. Catch classes derived from Exception, not Exception itself.

And who would catch that ApplicationException? How are they going to know what it was supposed to be about? If there is at least some debate possible about never catching generic exception types, you definitely shouldn't throw them.

I just can't think of a situation where what you're doing here actually makes sense, so please enlighten me.

Well, in a debuggin situation, I get the original exception's message. (When debugging, this included code line numbers.)

debugStr is mine, for more debugging purposes.

I'm willing to take in your point of view, but I'm a bit hazy. Can you tell me what you mean by this:

>> You shouldn't catch Exception. You don't have a clue what really happened if your in a catch block that caught exception, so how can you possibly recover sensibly. Catch classes derived from Exception, not Exception itself.

I'm thinking you're talking about, instead of catching any exception, Catch specific types of exception? (ie, SQLClient.Exception, or System.Threading.Exception)?

You might want to use this instead:

Try

   'do stuff

Catch ex As Exception

   'cleanup/rollback

   Throw

End Try

I believe if you do "throw ex" you lose the call stack.

Thanks,
Nate

 

qwert231 wrote:

I'm not sure why, but I got in the habbit of doing this:
  Catch ex As Exception
   Throw New ApplicationException(ex.ToString)

I'm assuming, that because I'm not creating a New Object, this is better:
  Catch ex As Exception
   Throw ex

Am I correct?

I have a similar bad habit (from the discussion I assume its bad) when writing services. All my services include a logging method that accepts a string and EventLogEntryType (error, warning, information, etc) .... I use catch blocks to catch "Exception" and then write to the eventlog with the method name and exception data as well as anything else i might find useful for debugging problems whenever something goes wrong.

Am I just stuck in some idea that this is useful? Any suggestions on a better way would be welcome as its somewhat tedious atm.

qwert231 wrote:

I'm thinking you're talking about, instead of catching any exception, Catch specific types of exception? (ie, SQLClient.Exception, or System.Threading.Exception)?

That's a good start, yes. The thing is, there is no value catching any exceptions at all unless you can usefully respond to what went wrong. If you catch something as generic as Exception, then you can't possibly know whether or not there was anything you could do about it.

Sven Groot wrote:



I just can't think of a situation where what you're doing here actually makes sense, so please enlighten me.

Perheps if he wants to write a routine that will "email back the execption message/write the message to event log" whenever an exception occurs, it'll be appropiate to catch all the exceptions, email the content, then throw it again?

I generally catch the exceptions I can't do anything else about (i.e. they rely on a call to another app, database on another server, EDI transaction, etc); the rest I generally put in a catch-all that provides a more friendly error and writes the technical goodies to a logfile.

Then again, I'm a geek. I like the technical goodies, even if my users don't.

In our applications, we put a try { ... } catch(Exception ex) { ... } at the very top-level of both our client and server applications. On the server-side, we log the exception. On the client-side, we send the exception to the server (if still possible), and show a friendly error message.

Here's what I try to do in my apps. Any method invoked by a user, get's a try catch. In the catch I do something like this:

Catch ex As Exception
 ReportError(ex as exception, datetime, user (if possible), sessionID (if web project)...) ' Custom logging method.

 labelErrors.Text = "There has been an error, please try again." ' For Web Projects
 MsgBox("There has been an error, please try again.")
End Try

Any other methods, I usuall just send back, but I was shown early on that you should use throw. Now I am realizing, unless I want to do something other than throw, I'll avoid the catch there.

Sometimes I do throw out the extension with some of my own debugging info in there, like so:

Throw New ApplicationException(myDebugString & Chr(10) & ex.ToString)

Thus I get my info, plus the current stack trace.

However, after what has been shown in this thread, I think I can improve performance by getting rid of lines like these:
Throw ex
Throw New ApplicationException(ex.ToString) ' Don't remember why I started doing this.

Handling exceptions at the AppDomain level is good as a last resort for catching unhandled exceptions. However it doesn't replace in-line handling. When you use a generic handler you lose the context of where the exception occurs. You have the call stack, but you can't respond and handle the exception allowing the app to move on unless you handle it inline.

Handle inline? For instance...?