Blog Stats
  • Posts - 44
  • Articles - 0
  • Comments - 67
  • Trackbacks - 0

 

Why unhandled exceptions are useful

It's the bane of most programmers' lives - an unhandled exception causes your application or webapp to crash, an ugly dialog gets displayed to the user, and they come complaining to you. Then, somehow, you need to figure out what went wrong. Hopefully, you've got a log file, or some other way of reporting unhandled exceptions (obligatory employer plug: SmartAssembly reports an application's unhandled exceptions straight to you, along with the entire state of the stack and variables at that point). If not, you have to try and replicate it yourself, or do some psychic debugging to try and figure out what's wrong.

However, it's good that the program crashed. Or, more precisely, it is correct behaviour. An unhandled exception in your application means that, somewhere in your code, there is an assumption that you made that is actually invalid.

Coding assumptions

Let me explain a bit more. Every method, every line of code you write, depends on implicit assumptions that you have made. Take this following simple method, that copies a collection to an array and includes an item if it isn't in the collection already, using a supplied IEqualityComparer:

public static T[] ToArrayWithItem(
    ICollection<T> coll, T obj, IEqualityComparer<T> comparer)
{
    // check if the object is in collection already
    // using the supplied comparer
    foreach (var item in coll)
    {
        if (comparer.Equals(item, obj))
        {
            // it's in the collection already
            // simply copy the collection to an array
            // and return it
            T[] array = new T[coll.Count];
            coll.CopyTo(array, 0);
            return array;
        }
    }
    
    // not in the collection
    // copy coll to an array, and add obj to it
    // then return it
    T[] array = new T[coll.Count+1];
    coll.CopyTo(array, 0);
    array[array.Length-1] = obj;
    return array;
}

What's all the assumptions made by this fairly simple bit of code?

  1. coll is never null
  2. comparer is never null
  3. coll.CopyTo(array, 0) will copy all the items in the collection into the array, in the order defined for the collection, starting at the first item in the array.
  4. The enumerator for coll returns all the items in the collection, in the order defined for the collection
  5. comparer.Equals returns true if the items are equal (for whatever definition of 'equal' the comparer uses), false otherwise
  6. comparer.Equals, coll.CopyTo, and the coll enumerator will never throw an exception or hang for any possible input and any possible values of T
  7. coll will have less than 4 billion items in it (this is a built-in limit of the CLR)
  8. array won't be more than 2GB, both on 32 and 64-bit systems, for any possible values of T (again, a limit of the CLR)
  9. There are no threads that will modify coll while this method is running
and, more esoterically:
  1. The C# compiler will compile this code to IL according to the C# specification
  2. The CLR and JIT compiler will produce machine code to execute the IL on the user's computer
  3. The computer will execute the machine code correctly
That's a lot of assumptions. Now, it could be that all these assumptions are valid for the situations this method is called. But if this does crash out with an exception, or crash later on, then that shows one of the assumptions has been invalidated somehow.

An unhandled exception shows that your code is running in a situation which you did not anticipate, and there is something about how your code runs that you do not understand. Debugging the problem is the process of learning more about the new situation and how your code interacts with it. When you understand the problem, the solution is (usually) obvious. The solution may be a one-line fix, the rewrite of a method or class, or a large-scale refactoring of the codebase, but whatever it is, the fix for the crash will incorporate the new information you've gained about your own code, along with the modified assumptions.

When code is running with an assumption or invariant it depended on broken, then the result is 'undefined behaviour'. Anything can happen, up to and including formatting the entire disk or making the user's computer sentient and start doing a good impression of Skynet. You might think that those can't happen, but at Halting problem levels of generality, as soon as an assumption the code depended on is broken, the program can do anything. That is why it's important to fail-fast and stop the program as soon as an invariant is broken, to minimise the damage that is done.

What does this mean in practice?

To start with, document and check your assumptions. As with most things, there is a level of judgement required. How you check and document your assumptions depends on how the code is used (that's some more assumptions you've made), how likely it is a method will be passed invalid arguments or called in an invalid state, how likely it is the assumptions will be broken, how expensive it is to check the assumptions, and how bad things are likely to get if the assumptions are broken.

Now, some assumptions you can assume unless proven otherwise. You can safely assume the C# compiler, CLR, and computer all run the method correctly, unless you have evidence of a compiler, CLR or processor bug. You can also assume that interface implementations work the way you expect them to; implementing an interface is more than simply declaring methods with certain signatures in your type. The behaviour of those methods, and how they work, is part of the interface contract as well.

For example, for members of a public API, it is very important to document your assumptions and check your state before running the bulk of the method, throwing ArgumentException, ArgumentNullException, InvalidOperationException, or another exception type as appropriate if the input or state is wrong. For internal and private methods, it is less important. If a private method expects collection items in a certain order, then you don't necessarily need to explicitly check it in code, but you can add comments or documentation specifying what state you expect the collection to be in at a certain point. That way, anyone debugging your code can immediately see what's wrong if this does ever become an issue. You can also use DEBUG preprocessor blocks and Debug.Assert to document and check your assumptions without incurring a performance hit in release builds.

On my coding soapbox...

A few pet peeves of mine around assumptions. Firstly, catch-all try blocks:

try
{
    ...
}
catch { }
A catch-all hides exceptions generated by broken assumptions, and lets the program carry on in an unknown state. Later, an exception is likely to be generated due to further broken assumptions due to the unknown state, causing difficulties when debugging as the catch-all has hidden the original problem. It's much better to let the program crash straight away, so you know where the problem is. You should only use a catch-all if you are sure that any exception generated in the try block is safe to ignore. That's a pretty big ask!

Secondly, using as when you should be casting. Doing this:

(obj as IFoo).Method();
or this:
IFoo foo = obj as IFoo;
...
foo.Method();
when you should be doing this:
((IFoo)obj).Method();
or this:
IFoo foo = (IFoo)obj;
...
foo.Method();
There's an assumption here that obj will always implement IFoo. If it doesn't, then by using as instead of a cast you've turned an obvious InvalidCastException at the point of the cast that will probably tell you what type obj actually is, into a non-obvious NullReferenceException at some later point that gives you no information at all. If you believe obj is always an IFoo, then say so in code! Let it fail-fast if not, then it's far easier to figure out what's wrong.

Thirdly, document your assumptions. If an algorithm depends on a non-trivial relationship between several objects or variables, then say so. A single-line comment will do. Don't leave it up to whoever's debugging your code after you to figure it out.

Conclusion

It's better to crash out and fail-fast when an assumption is broken. If it doesn't, then there's likely to be further crashes along the way that hide the original problem. Or, even worse, your program will be running in an undefined state, where anything can happen. Unhandled exceptions aren't good per-se, but they give you some very useful information about your code that you didn't know before. And that can only be a good thing.


Feedback

# re: Why unhandled exceptions are useful

Gravatar Even better is to have your type system enforce that all errors are handled at summer point so there is never anything unhandled. 6/3/2013 6:06 PM | orbitz

# re: Why unhandled exceptions are useful

Gravatar You have some very good points, and i would like to add some of my own, if I may. Instead of following a strict code of ethic regarding fail hard and fast, i would encourage you to understand what you are making and how it should respond. If the application or algorithm is not a business critical phase, then I would catch the exception and log it into a file and return either null or empty result. I would also take this approach if i am programming a change on an existing system, which is critical not to break, because then it could fail safely and i can look at the log files.
I would recommend: fail hard and fast early on in a project to get away all the rookie assumptions, then fail safely afterwards :) 6/5/2013 7:40 AM | Egil andre

# re: Why unhandled exceptions are useful

Gravatar Design by Contract would allow/enforce making these assumptions part of the code contract, with failures producing more meaningful exceptions. 6/5/2013 10:00 AM | robg

# re: Why unhandled exceptions are useful

Gravatar Egil - indeed, as long as returning null or empty is a valid return value for your algorithm, and the calling code knows to expect it. Otherwise you're just pushing the problem further down the line. 6/5/2013 11:02 AM | Simon Cooper

# re: Why unhandled exceptions are useful

Gravatar Egil andre is correct - you've gone from one extreme to another. You need to decide on whether the result of the exception is critical or not, and handle it appropriately. I always use the as keyword instead of casting, so that if the resulting object is null, I can handle it in the best way. I never do (x as Something).method(), without first checking the result of x and handling it in an appropriate way.



6/5/2013 1:44 PM | Andrew Johns

# re: Why unhandled exceptions are useful

Gravatar Actually I guess that means it's no longer an unexpected exception if I am prepared to handle it accordingly. :) As you were. :D 6/5/2013 1:45 PM | Andrew Johns

# re: Why unhandled exceptions are useful

Gravatar Egil - Who is going to look for the log? The operational support staff won't have time to examine all logs for an exception that only happens once in a blue moon. If they did, they'd never get the approval to go in and fix it.

Don't worry about breaking an existing application - if it's thrown the exception it is already broken. 6/5/2013 2:05 PM | Tom Falconer

# re: Why unhandled exceptions are useful

Gravatar Tom - in my opinion, we cannot rely on LOB to report the crash details accurately if it happens once in a blue moon. At least logging the stack trace would have all the needed information at one place.

Again, like others mentioned, the level of logging should depend on where the exception occurred and how important it is to be reproducible. 6/5/2013 10:22 PM | Hari K

# re: Why unhandled exceptions are useful

Gravatar Missed an assumption, one I regularly ignore so I can see my code blow up.:)
When you copy the array, you have enough memory available to make the copy.

One assumption that might not cause an exception but could cause grief is that your code is efficient enough to run the process in a reasonable time. Somebody wrote an article about bubble sort. (A routine that is probably older than the author.) Then some ignorant soul said it was efficient. This is an N^2 routine. (double the number quadruples the time.) 1000 items sorted in 3 millisecond. At 100K it was over 30 seconds. Double that and it took 2:10. I cut off testing that sort at 200K, kept on testing my binary sort and the built-in array sort to 150 million. (Couldn't store 200 million.) Mine did it in just over 45 seconds built-in just under 30 seconds. (It consistently paced at 2/3 my routine's speed.) I estimated the bubble sort at just under 2 and a third years.
750^2=562500, 562500*130/60/1440/365.25=2.31719...
150M/200K=750 times bigger, 2:10 is 130 seconds, /60/1440 divided by seconds in the day to get the day count... I wouldn't hold my breath waiting for a server to remain up for over 2 years.

Once I assumed int would blow up because it's max value was exceeded. Wrong assumption, if you want to make sure arithmetic overflow errors do blow up use strict..

The author made a wrong assumption. LongCount can easily exceed 4 billion. Assuming he is using Microsoft's C# code once you exceed int.MaxValue (Just over 2 B.), the numbers go negative for int.

For C# array types COUNT is int IEnumerable. You'd never exceed uint (Just over 4 B.) but you could blow up trying to create an array with a negative count.

I was surprised how easy it was to blow up memory with recursive routines. One in Debug mode would blow in under 500 iterations, compiled lasted over 4K iterations. 6/6/2013 8:00 AM | Ken

# re: Why unhandled exceptions are useful

Gravatar (x as Something).method() is a very dirty way to call a method. In fact, one can never really be sure that x is Something, so it's allways better to declare x as Something first and then to proof if it's null or not.
var x = q as Something;
if(x != null) ...

Assuming that x will allways be Something is a very naive way to code... 6/14/2013 12:26 PM | DaveK

Post A Comment
Title:
Name:
Email:
Comment:
Verification:
 
 

 

 

Copyright © simonc