Exception handling

[jitendra vishnoi]

Hi All,

I was going through Stack Exchange - Why should 'boneheaded' exceptions not be caught, especially in server code?(in my case it will not cause server crash)
and it reminded me few scenario which I faced some time back and looking for what should be correct solution/feedback.
I will try to explain scenario and my understanding.

1. GET /uri/<storeId>/orders, This api will return list of store orders(array of JSON object). Internal logic of API is like that it will collect stores order ids from different source(step 1) and start collecting details(step 2) for each order and while collecting order detail for any order it may get exception(may be order got corrupted by some process) it will cause whole API to fail and API will return respective error so that store(client) can inform back to us and we should investigate the issue and fix it. But investigation can take time so we created a flag which will change API behavior and force it log the exception for corrupted order and return rest of orders.
If flag is true then PROS of this approach is that store will able to keep processing order(consider 10 order a day) but we have rely on someone who monitor logs and if missed we will never notice that something wrong happening with some orders.
If flag is OFF, API will fail and return error and we have to rely on stores exception handling(whether they inform us back or not) + someone from team to monitor logs. If none notice the issue those order will never get processed.
Query - Should we go with flag value as true/false ?

2. User object is injected by framework in our code. We can get user attributes by its getProperty method, eg. user.getProperty("firstName"). I have seen people put a NPE check before getting any value form user object with explanation that it will not cause any issue(which is correct). But as my understanding if user is null then something is already wrong with application and we should not put NPE as we will never know that it happened(Obviously customer will never get "firstName" if user is null).
Query - Shouldn't we avoid these checks ?

[Les Morgan]

jitendra vishnoi,

IMO: EVERY possible exception should be caught.  Once you catch that exception, then what to do has to be based on your business rules, nobody else can really tell you what they are.

If this is happening on the server, by all means, log them!  Somebody may actually find time to check, but more appropriately you should have a reporting module that checks the logs for you.  One that gleans through the mess that is always in a log and search for error levels, set my your business rules, and write them to a report, then email that report to the appropriate person to deal with it.  Maybe email it to the appropriate persons or team if you cannot relay on 1 person to always take action, but make it as difficult to ignore as possible (get it right into the appropriate person(s) face(s))--make a person decide: yes, we need to deal with this or no, this is not important.

So there is my opinion--I've banged around in this industry on all sides of development and support.  Never just let something go unchecked, always catch ALL exceptions, always account and avoid ALL error possibilities.

Les

[Tim Holloway]

Les Morgan wrote:
IMO: EVERY possible exception should be caught.  Once you catch that exception, then what to do has to be based on your business rules, nobody else can really tell you what they are.

Close, but no cigar. There are a few exceptions that are worse to catch than to let through. For example, handling an OutOfMemoryException is likely to involve code that attempts to acquire memory and thus effectively recurse itself. A few other exceptions where the integrity of the JVM has become questionable also qualify.

You should do one of 3 things with an Exception (or for that matter, any Throwable).

1. Catch it and handle it. That could include re-shaping and re-throwing it.

2. Catch it and report it.

3. Don't catch it and let the JVM control program deal with it.

Consider the fourth option at your peril: if you catch an Exception and silently eat it and I have to deal with the code, expect me to express (ahem) EXTREME "displeasure". Mysteries owing to silently-discarded Exceptions may be met with violence.

At a minimum, if you catch an Exception and it's safe to ignore it, write a handler that can log at a debug level. That is, option 2, above.

[Campbell Ritchie]

As a general rule of thumb, I always think,

What am I going to do aboiut that exception?

I wouldn't anticipate an out of memory problem in the first place, so I wouldn't even think of catching one. If I suffer a un‑declared null pointer exception or arithmetic exception, I shall probably have to go through the code to work out how to correct the problem. In the following case however, it might be possible to correct the problem “in‑flight”.

/**
 * This method doesn't have undeclared null pointer exceptions
 * ...
 *@param x some parameter or other. Mustn't be null
 *@throws NullPointerException if x is null
 */
public void foo(XYZ x) ...

So most unchecked exceptions are best allowed to propagate and (good grief!) I have re‑invented “declare or handle” as in the Java™ Tutorials.

[jitendra vishnoi]

Happy new year to all and thanks for looking into my query.

Les - I am assuming(correct me if you meant for all) that you are talking about checked exception(not RunTimeException or the one @Tim suggessed - OutOfMemoryException) and yes we always log checked exception and based on logic either re-throw or continue with rest of logic.

1. GET /uri/<storeId>/orders, This api will return list of store orders(array of JSON object). Internal logic of API is like that it will collect stores order ids from different source(step 1) and start collecting details(step 2) for each order and while collecting order detail for any order it may get exception(may be order got corrupted by some process)
...

Here exception is RunTimeException as we noticed it we put a try-catch block and logged this exception, The flag which we created just to control whether whole API should fail(error in api response + log the culprit order) or just log the culprit order and return rest of orders detail . I feel kind of agree with Les that its business who can decide whats expected behavior.

2. User object is injected by framework in our code. We can get user attributes by its getProperty method...

Campbell - As I understood we can have doc updated when particular method can throw NPE or any RunTimeException. What about some filed which is promised by framework to be injected. Framework would most probably able to inject those object unless there is some issue with application startup(or may be issue within in framework/some config issue) and framework was not proper initialized. What is the point of having a NPE check when application is not started properly.

[Tim Holloway]

You bring up an interesting conversation.

In the case of something such as a webapp, there are actually three layers of concern. The top layer, is, of course, the JVM itself, the bottom layer is the webapp, and the middle layer is the webapp container.

The webapp container (server) will catch most of the Exceptions thrown out of a webapp. There are even standard mechanisms on how to deal with such exceptions, such as the errorpage directives in web.xml.

However, on the whole, a "real" exception (HTTP 503) should be caught and dealt with within the webapp. It looks unprofessional otherwise.

Note that a "404 - Not Found" condition is not a webapp exception. It's "thrown" internally to the container when the container cannot resolve a resource request and thus no application logic is involved.

[jitendra vishnoi]

Thank you Tim.