17

Been having a "heated debate" with a colleague about his practice of wrapping most of his functions in a try/catch but the catch has JUST a "throw" in it e.g.

Private sub foo()
    try
        'Do something'
    catch
        throw 'And nothing else!'
    End Try
End Sub

My thought was to not even bother (assuming you don't need to do anything at this point) - the exception would bubble to the next exception handler in a parent member.

The only argument that sounded plausible was that sometimes exceptions weren't caught and your code stopped (in debug mode) with the current line highlighted in green...and that this may be something to do with multiple threads? Best practice does state "an exception handler for each thread" but mostly we work single-threaded.

The good thing may be it could be useful in debug mode to not suddenly pop out to a parent member (yes, Joel!) - you'd move to the "throw" statement and be able to examine your locals. But then your code would be "littered with try/catch/throws" (to quote another thread here)?

And what sort of overhead would be involved in adding try/catch/throws everywhere if no exception occurs (i.e. should you avoid try/catches in tight loops)?

Bob King
  • 25,372
  • 6
  • 54
  • 66
AndrewD
  • 1,083
  • 9
  • 15
  • Sort your VB code out, it looks horrible. – Rob Stevenson-Leggett Oct 20 '08 at 16:13
  • 2
    Try typing with 1 hand and a squirming baby at 11pm! 80/20 - does the job. – AndrewD Oct 21 '08 at 01:29
  • No mention of the benefit in multi-threading - do I assume that it is not relevant then? – AndrewD Oct 21 '08 at 01:44
  • And I assume MSIL would insert at least a line or 2 for a try/catch (MSIL is Greek to me) so the general consensus of "don't do this" is with reason. – AndrewD Oct 21 '08 at 01:44
  • Thanks to all - seems not doing this is best and Jon's Ctrl+Alt+E to break on all is the correct (not sure about best) alternative. Still wondering about the multi-threading bit...but it all fits with the usual std practices. – AndrewD Oct 21 '08 at 01:52
  • You have to be more explicit about your multithreading issue. I'm not aware of problems debugging mulithreaded applications with visual studio. Also breaking at thrown exceptions works fine for me. – Jan Oct 21 '08 at 06:45
  • A little something I discovered recently with "throw" & "throw ex" - the line number reported in the trace will be that of the throw! The only way to get the correct line# is "throw new Exception(ex.Message, ex)". More at https://stackoverflow.com/questions/2493779/wrong-line-number-on-stack-trace – AndrewD Apr 12 '21 at 05:15

11 Answers11

18

Microsoft recommends not to catch an exception when the only thing you do is to rethrow it immediately (i dont remember the source for now). Your code should only catch exceptions that you want to handle for clean up things or similar actions.

So generally its not a good practice to catch and rethrow an exception.

Reasons for catching and replacing it with another exception might be

  • Logging
  • Hiding sensitive information from the caller (Stacktrace, exception details)

And for debugging you might want to change your "Break when an exception is:"-Handler (Press Ctrl+Alt+e) the value "thrown" on selected CLR Exceptions.

You might want to take a look at the entlib exception handler block (EHB), with which you can establish a pattern on how to deal with exceptions in your code.

Regarding your question on performance i think its not a propblem to have many try/catch blocks in your code but you will get performance hits when your code raises and catches many exceptions.

Jan
  • 15,802
  • 5
  • 35
  • 59
  • Thanks for the Ctrl+Alt+e tip (you need an active code pane though)...can't find that in the main menu system (I'd expect it under Tools->Options...Debugging). – AndrewD Oct 21 '08 at 01:50
  • Normally you'll find it under menu Debug->Exceptions, when a project is loaded and a code window is active. – Jan Oct 21 '08 at 06:43
  • VS2005 doesn't seem to have that...but it does the Ctrl+Alt+E!? Must upgrade :-( – AndrewD Oct 21 '08 at 07:35
  • Cant tell you, because i have deinstalled VS2005 :) – Jan Oct 22 '08 at 16:01
17

The reason you have a lone throw inside a catch rather than throwing a new exception is because this causes the original stack trace/exception data to be preserved. And one reason you might do this is because you can now set a break-point there for debugging.

Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
  • 1
    Another reason to rethrow is to clean up existing state - it's like saying, "OK, I know something bad happened. *I* can't handle it, but I'll clean up before I leave it to someone else. Sometimes finally is NOT the right place for this. – plinth Oct 20 '08 at 15:25
  • 1
    While a lone throw will preserve state, so will not catching the exception at all. If you want to set a breakpoint for debugging, set a breakpoint from Debug > Exceptions. A catch clause is totally unnecessary. – Dour High Arch Oct 20 '08 at 20:07
  • @DHA: Please explain how you'd catch an exception at a precise level in a call stack which may be 20 levels deep then. – Jon Skeet Oct 20 '08 at 20:20
  • He means, you should enable automatic breaks at any occurence of a given exception, which you can enable in the debug->exceptions menu. – Jan Oct 20 '08 at 20:24
  • You can break when it's thrown, but it can then be a pain to backtrack up the stack the right amount. I'm not saying it's common to want to do this, but it does happen. – Jon Skeet Oct 20 '08 at 20:35
  • (And the other thing is that the same exception may be thrown from many other places.) – Jon Skeet Oct 20 '08 at 20:59
8

I would only ever do this while debugging an issue - and I'd remove the code again before checking in. It can occasionally be handy to put a breakpoint in to stop at a particular stack level if an exception is thrown. Beyond that though - no.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Definitely don't code it like that by default. But if I need to add that during a debugging session, I'd probably leave it in. If I needed it once I likely will again, and it's a subtle way to tell other developers that the included code may be more difficult than it appears. – Joel Coehoorn Oct 20 '08 at 15:23
  • 1
    I'd put that in a comment then, rather than leaving ugly and distracting code. Most of the time when that code needs to be read, it probably *won't* be the same sort of situation - so don't make it more complex for that case. – Jon Skeet Oct 20 '08 at 15:24
  • I do not think it is a good idea to test something, then change the code "before checking in". – Dour High Arch Oct 20 '08 at 20:09
  • 1
    I wouldn't do it *just* before checking in. I would use the debugger to work out what was going wrong; write a failing unit test; take out the try/catch/throw and prove it's still going wrong; fix it and see it all go green; submit for code review; check in. Do you never remove anything from code? – Jon Skeet Oct 20 '08 at 20:15
  • Man after my own heart, Dour High Arch! Banging heads with a young 'un now about that...any change is a new risk!! Conditional IF's I find useful but just as dangerous. – AndrewD Oct 21 '08 at 01:47
6

In practice, my thought is, if you don't intend to handle the error, don't catch it.

Geoff
  • 9,340
  • 7
  • 38
  • 48
2

One effect of using a catch and immediate rethrow is that any inner "Finally" blocks will run before the "Catch" occurs (which will in turn be before the exception propagates). This is relevant in two scenarios:

  1. If an exception is ultimately unhandled, it's possible that the unhandled-exception trap may quit the application without running any "finally" blocks. Doing a catch and immediate rethrow will ensure that all "finally" blocks within the catch will execute, even if the exception ends up ultimately being unhandled.
  2. It is possible for code in vb.net, and possibly other languages, to act upon an exception before any finally blocks are run. Using a "try" block with a catch-and-immediate-rethrow will cause the "finally" blocks within that catch block to run before any outer "try" blocks get their first look at the exception.

An additional caveat with catch-and-immediate-rethrow: for some reason, a catch and immediate rethrow will trash the stack trace's line number for the function call that caused the exception. I don't know why the current function's entry in the stack trace can't be left alone in that case, but it isn't. If one isn't using a .pdb file to get line-number information, this isn't an issue, but if one wants to use such information, it can be annoying.

Generally, the effects mentioned above aren't desirable, but there are occasions when one or both of the first two effects they may be useful, and the third effect tolerable. In those cases, a catch with immediate rethrow may be appropriate, though the reason for it should be documented.

supercat
  • 77,689
  • 9
  • 166
  • 211
1

Doing it always by default looks like bad design. But there might be reasons for catching and throwing, for example it you want to throw a different exception.

Fabian Buch
  • 831
  • 2
  • 9
  • 18
  • If you're throwing a different exception, it's not the situation described by the OP. He's specifically talking about the "bare throw" situation. – Jon Skeet Oct 20 '08 at 15:25
  • Then I don't know a reason for it, since for debugging there are debuggers. – Fabian Buch Oct 20 '08 at 15:39
1

Yes, it's handy for putting a breakpoint in the catch.

An alternate and cleaner way is to breakpoint in the constructor of the object you're throwing. You're seeing the program state at a point closer to the source of the error.

Mark Ransom
  • 299,747
  • 42
  • 398
  • 622
1

You do not need a catch clause to catch exceptions in the Visual Studio debugger. Choose Debug > Exceptions, and select which exceptions you want to catch, all of them if necessary.

Dour High Arch
  • 21,513
  • 29
  • 75
  • 90
1

If you catch an exception and replace it with another exception, you should typically wrap the original exception in the new one. This is usually done by passing the old exception into the new one's constructor. That way you can dig in as much as necessary to figure out what happened. The main case when you wouldn't is when you need to hide data for security reasons. In these cases, you should try to log the exception data before you clear it out.

The rationale I have seen for wrapping exceptions with new ones, rather than just letting them bubble up the stack, is that exceptions should be at the same symantic level as the methods they are coming from. If I call AuthenticateUser, I don't want to see an SQL exception. Instead, I should see some exception whose name tells me the authentication task could not be completed. If I dig into this exception's inner exceptions, I could then find the SQL exception. Personally, I am still weighing the pros and cons of doing this.

Neil
  • 7,227
  • 5
  • 42
  • 43
0

Since there is zero error handling, this catch is useless. If there was logging or some cleanup done sure, but in this situation I'd get rid of the try/catch.

Bryan
  • 81
  • 1
  • 2
0

This can also be useful if you need to inspect something about the exception, and do something for one circumstance or throw it for other circumstances. For instance, if you need to inspect the error number in a SQLException. You can perform a certain action if the error number is one you're prepared to handle. For others you can simply "throw" it so the stack trace is preserved, as mentioned above.

DCNYAM
  • 11,966
  • 8
  • 53
  • 70