views:

645

answers:

11
+8  Q: 

C# try {} catch {}

hi and thanks for reading. im a newbie in programming and C# and sockets programming. in my code i try and catch problems to provide fault tolarence in my app. the following:

        catch (ArgumentNullException e)
        {
            OnNetworkEvents eventArgs = new OnNetworkEvents("Network Unavailable", e.Message);
            OnUpdateNetworkStatusMessage(this, eventArgs);
        }
        catch (EncoderFallbackException e)
        {
            OnNetworkEvents eventArgs = new OnNetworkEvents("Network Unavailable", e.Message);
            OnUpdateNetworkStatusMessage(this, eventArgs);
        }
        catch (SocketException e)
        {
            OnNetworkEvents eventArgs = new OnNetworkEvents("Network Unavailable", e.Message);
            OnUpdateNetworkStatusMessage(this, eventArgs);
        }
        catch (ArgumentOutOfRangeException e)
        {
            OnNetworkEvents eventArgs = new OnNetworkEvents("Network Unavailable", e.Message);
            OnUpdateNetworkStatusMessage(this, eventArgs);
        }
        catch (ObjectDisposedException e)
        {
            OnNetworkEvents eventArgs = new OnNetworkEvents("Network Unavailable", e.Message);
            OnUpdateNetworkStatusMessage(this, eventArgs);
        }

i was just wondering if i can replace this repetitive code with one single:

catch (Exception e) { handle here}

would this work?

thanks again.

+1  A: 

It would work, but it would also catch "null reference" exceptions and any other random exception that got thrown.

taylonr
+1  A: 

Yes it would work, but it would not do the same, since all exceptions and not only the specific ones would be catched.

This is usually not what you want, as the general rule is to catch only exceptions which you know how to handle. There are some situations where you need such a catch-all to redirect the exception (like accross thread boundaries or in async calls) or do some cleanup work if something - anything - goes wrong (but then you usually re-throw the exception after doing your work).

Lucero
+3  A: 

That will certainly catch all exceptions, but that can be poor practice. There are some exceptions that will be hidden or misrepresented that will make testing and debugging your application much more difficult.

Adam Crossland
+1  A: 

It would catch all exceptions which inherit from Exception class. If they all are handled in the same way you can do it but you shouldn't as you can't handle all Exceptions inherited from Exception in the same way.

Javi
+10  A: 

As a general guideline, don't catch System.Exception. It's a bad idea and a bad code smell indicative of a failure to grasp the purpose of exceptions. I believe it's the result of a dev's belief that exceptions are the error instead of the code that triggered the exception.

I see this failure in production code all the time, and it invariably masks real errors as valid and useful exceptions are caught and swallowed, leaving the application in a bad state. This makes it much trickier to track down the root cause of a failure.

Catching exceptions using the method you've used here is more-or-less the right way to do it. There are a few things you might want to consider improving on, though:

  1. Don't generally treat an ArgumentNullExceptionor ArgumentOutOfRangeException as a runtime network failure (unless it really is). Rather, it should be considered a logic error that causes your program to fail as quickly as possible (so that you can check it out with a debugger as close as possible to the point of failure). This frequently means not catching any ArgumentException at all.

  2. Consider examining the exception hierarchy to find an appropriate base exception that covers the exceptions you're trying to report. E.g., (I haven't looked it up), suppose that three of your exceptions all derived from SocketException. You might save some typing by catching a SocketException instead of the three separate exception. Only do this if you're reporting all socket exceptions, however. (This is basically a more disciplined version of your original attempt to just catch Exception)

  3. Because both lines in every exception handler are identical, you may create a function to do those two lines of work in a single line in the handler. Typical don't-repeat-yourself refactoring. If you want to change the way you report your exception, think about how much easier it would be to change a single function than all those individual handlers.

  4. Just about any significant I/O (network and file come to mind) is going to entail a pretty significant chain of exception handlers just because there's so much that can go wrong. Seeing a lot of error reporting around I/O that may fail isn't an anti-pattern, but it may be a good code smell. Like pine-fresh scent or freshly baked bread. :)

Greg D
+1 for #2, that is an excellent idea.
Frank V
I gave a +1 for a more detailed and less confrontational answer than mine. ;) Sometimes I just think that people need a kick in the pants; we've come a long way since the days of `ON ERROR RESUME NEXT`.
Aaronaught
+2  A: 

You could but you would lose granularity and the ability to handle each issue individually, which isn't the best practice. Usually you would handle different types of exceptions first, just as you have, then if you need to do something with any unhandled exceptions you could add the Exception at the end. Re-throw it once you're done to preserve the stack-trace and let it bubble up.

Keep what you have an add a base case at the end:

catch (ArgumentNullException e) { ... }
catch (EncoderFallbackException e)  { ... }
catch (SocketException e) { ... }
catch (ArgumentOutOfRangeException e)  { ... }
catch (ObjectDisposedException e) { ... }
catch (Exception e)
{
    // not handled by above exceptions, do something if needed
    throw; // after doing what you need, re-throw it
}
Ahmad Mageed
A: 

While you can do that , I'd consider it poor practice and not how you'd want to be structuring your code.

Depending on the exception you may want to do something different. An invalid IP is a different issue than a hardware error and so on. Additionally some errors you might want to notify back to the UI via a delegate or log somewhere using log4net.

But that's just me and I'm far from an expert - so take it for what it's worth

PSU_Kardi
+18  A: 

No, never catch System.Exception. I feel like a broken record today, but I really can't stress this enough. You cannot handle an OutOfMemoryException or AccessViolationException, so don't catch it!

As for the example code, I have strong doubts that an ArgumentNullException or ArgumentOutOfRange exception would correspond to a network error. If it does, it likely means that the component that really should be catching this exception, isn't.

"Fault tolerance" does not mean "eat every exception so the app doesn't crash" - it means actually knowing the things that can go wrong and handling them correctly.

Aaronaught
Depending on why you get those errors there are things you can do. For example what if you get a OutOfMemoryException because you try to declare a byte[] that is 4GB in size. It's bad to make blanket statements. There is typically and edge case that will prove you to the exception and not the rule.
Matthew Whited
While I agree in spirit, I cannot say "never catch System.Exception". I have Windows Services that process large amounts of data, for example analyzing over 100k+ rows, if some of that unstructured data throws an unhandled exception, something I could not expect, I catch this System.Exception and proceed with the rest of the records and issue an alert via our logging mechanism. I'm sorry, but there are places in unattended processes where this catch all is useful especially when working with batches. My 2 cents.
RandomNoob
Also what if you get an exception that is thrown from something else up the stack. It could be some network timeout exception that you didn't think to catch and handle one off. If you know you just want to try resending data if anything blows up then feel free to catch Exception. Just remember to log and/or rethrow the exception if you don't want to do anything with it.
Matthew Whited
@bnkdev: A few years ago I used to do the same thing as you for long, unattended batch processes. I stopped doing it when it started masking exceptions due to race conditions that subsequently caused data corruption. There's (almost) always a more specific exception you can catch, and if there isn't, you may need to rework some of the lower-level components you're using.
Aaronaught
@Matthew Whited: Catching an `OutOfMemoryException` because you tried to declare a 4 GB buffer sounds like an exceptionally bad idea to me. The program is in an undefined state after such an exception - other threads may also have crashed/been corrupted. Better not to try to allocate that kind of memory all at once, or at least check to make sure it is available.
Aaronaught
That was an extreme example. Batch processing, data links, remove file access. They can all have glitches that could abort an entire process instead of just killing one step that can be restarted or flagged for user intervention. There is no reason why you can’t have general exception handlers to process such problems.
Matthew Whited
@Matthew Whited: Sure, it's an extreme example until it happens to you. ;) I hate leaky abstractions too, but unfortunately, in my experience, the only means of reliable exception handling is to understand *specifically* what can go wrong. Almost makes me long for Java's checked exceptions... *almost*. (P.S. Global exception handlers are another story - it's fine to use those in moderation as opposed to simply catching `System.Exception`)
Aaronaught
@Aaronaught: "You cannot handle an OutOfMemoryException". Could you please explain why? I just tried to execute the following line: "ArrayList array = new ArrayList(500000000);" and successfully caught OutOfMemoryException, which merely informed me that I did't have enough memory. Program state was NOT corrupted at all.
Igor Korkhov
@Igor Korhov: Being able to *catch* an `OutOfMemoryException` does not mean you can *handle* it. What will you do with it? And how do you know that the program state was fine? Did you try this in the middle of a running program? Did you have other threads running? The docs for this specifically say: `The exception that is thrown when there is not enough memory to continue the execution of a program.` That says to me that even if the execution can *sometimes* continue, it is not a behaviour you can ever rely on.
Aaronaught
1) Create a smaller buffer. 2) Change to a streaming model. 3) Notify the use and go on to the next one.
Matthew Whited
Also... just because you can't create one buffer of 4GB doesn't mean you can't create several buffer that equal the same volume of space. Memory is allocated based on free memory gaps... not based on total memory useage.
Matthew Whited
@Aaronaught: Documentation also says: "The following Microsoft intermediate (MSIL) instructions throw OutOfMemoryException: box, newarr, newobj". And what that simply means is that CLR is unable to find free block of memory of a given size. It DOES NOT expand or shrink other object's memory in favor of your request. That is why I am telling you that in my example the state was not corrupted.
Igor Korkhov
@Igor and @Matthew: How would you know for certain that an `OutOfMemoryException` came from any particular statement you executed? And how could you know for certain that creating a smaller buffer or changing to a streaming model would actually succeed? How can you guarantee a notification will get sent? I never said that the Framework automatically corrupts your program when an `OutOfMemoryException` occurs, simply that it *could* be corrupted because your process may really be out of memory/address space, and it is not safe to continue executing when this happens.
Aaronaught
First of all, you never now if the state of a program is corrupted or not at any given momemt, at least not in a program written in c# (or any language which doesn't support automated *mathematical* demonstration that the semantics of the program are consistent with the specification for that program). But it has nothing to do with exceptions. OutOfMemoryException just says that N bytes of memory cannot be allocated, just like NullReferenceException says that an *attempt* to dereference null reference was made. And it's up to programmer to decide what to do in each case.
Igor Korkhov
And pretending that there is no OutOfMemoryException at all by not even trying to catch it is a kind of hiding one's head in the sand. And please don't think that I want to say that "catch (Exception e)" is a good style. I just wanted to say that OutOfMemoryException can be catched and, being catched, doesn't generally mean the end of the world.
Igor Korkhov
@Igor Korkhov: Not catching an `OutOfMemoryException` at all is acknowledging that you cannot recover from said exception and **failing fast**. You may be able to concoct some trivial edge-case exception, but 99% of the time, when an `OutOfMemoryException` occurs, you have **no idea** where it really came from; maybe it was your code, maybe it was deep within the framework. So you are going to catch it and then do what - log it? Try a smaller buffer? All of these things involve *allocating more memory* - not the smartest idea when you just got an `OutOfMemoryException`.
Aaronaught
If you wish to continue this argument, I suggest making your own question out of it - "When is it OK to catch an `OutOfMemoryException` and how to handle it?" - see what kind of responses you get from the community here.
Aaronaught
My only issue with your answer is you said "never catch System.Exception". I agree that in general it can be best to catch explict exceptions, but if you are writing extensible batch processing services there are plenty of times when you pretty much have no choice. That is unless you want to have the service drop and be restarted when an add-in faults.
Matthew Whited
@Aaronaught: Done: http://stackoverflow.com/questions/2117142/when-is-it-ok-to-catch-an-outofmemoryexception-and-how-to-handle-it
Igor Korkhov
A: 

IMO:

Catching Exception would be pretty similar to having methods with parameters that are never more specific of a class than Object; Sure you can do it but is it really the best decision 9 out of 10 times?

What you should consider doing to simplify your code some is Catch exceptions at the highest definition that is relevant.

For instance, in java I would

try{...}
catch( IOException e ) {...}

to catch all IO related exceptions.

There are too many things that can throw too many different types of exceptions, you typically should avoid using the catch-all that is Exception.

instanceofTom
+2  A: 

I actually just read a blog post on this very topic this morning.

You could do something like mentioned in the comments of the post:

catch(Exception e){
    if(    e is ArgumentNullException 
        || e is EncoderFallbackException 
        || e is ...whatever 
    ){
      OnNetworkEvents eventArgs = new OnNetworkEvents("Network Unavailable", e.Message);
      OnUpdateNetworkStatusMessage(this, eventArgs);
    } else { throw; } 
}
Mark
Ewww. I wouldn't do that.
Greg D
There really is no other way to avoid doing exactly the same action on several different explicit exception types without duplicating the code to identical exception handlers.
Mark
what's wrong with that Greg?
newbie
It is, under normal circumstances, nasty. I would never do it unless I had many duplicate event handlers. It would potentially lead someone astray by following that pattern when the handlers aren't identical.
Mark
@newbie: As a general rule, I try to avoid run-time type checking/downcasts unless there's a compelling reason not to. The fact that the language natively supports the equivalent behavior suggests to me that there isn't a compelling reason. Esp. given that I suspect most of the OP's exceptions should be posting different messages, not just "Network Unavailable."
Greg D
on these errors ther is nothing the user can do so i display the same error message but i also pass e.Message so that if someone else was using the code could then display my error message or the e.Message.
iEisenhower
@Greg D: he may just want to catch all exceptions wrap them in a inner exception and then throw as a new exception as his own choosing or possible pass them to some error handler log for user intervention. We don’t all write the same program. And different programs have different requirements. (Reporting a network error to an end user is a great way to get tons of support calls and “bug” complaints. Especially if all they were trying to do was save a file to a network drive.)
Matthew Whited
@Matthew Whited: That's possible, but unlikely given the context: "im a newbie in programming and C# and sockets programming." I've seen the "report everything as one error" paradigm used elsewhere, one app reports everything as "Network Error" no matter what the cause, that is bad form. I also maintain that masking `ArgumentException`s as "Network Unavailable" is poor form in all but the most advanced scenarios (e.g., some form of dynamic codegen). Either way, I'm not sure how your comment related to my earlier comment. :)
Greg D
@Greg D: I would agree that throwing ArgumentExceptions as NetworkExceptions would be confusing. But I think it would be better to handle sanitize the inputs that are cuasing the input instead of letting invaild inputs make it the socket anyway (but that's a difference issue then avoid "catch (Exception)")
Matthew Whited
+2  A: 

As all previous comments suggest, you should catch all the "known" exceptions with exception specific catch block and other unknown, or more appropriately, all "unmatched" exceptions with catch (Exception ex) { ... } or just catch { ... }

However, as specified in your code, you are handling all kind of exceptions in same way. So in this particular case, the output (what you are calling Fault Tolerance) will be the same, but the performance will improve (since the runtime need not compare exception type with given list of types).

Moral of the story : Use single common try-catch in this particular case. But in general, avoid such habit.

Mihir Gokani