Menu
▾
▴
Re: [ebxmlms-develop] Error reporting in Hermes source code
|
From: Patrick Y. <kc...@ce...> - 2004-04-08 04:24:57
|
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head> <meta content=3D"text/html;charset=3Dwindows-1252" http-equiv=3D"Content-Type"> <title></title> </head> <body bgcolor=3D"#ffffff" text=3D"#000000"> Yes, it does make sense. Thanks! Designing a good exception hierarchy is no easy task, let's review that as well when we review our code later.<br> Regards, -Patrick<br> <br> Mayne, Peter wrote:<br> <blockquote cite=3D"mid...@s-...= m.au" type=3D"cite"> <meta http-equiv=3D"Content-Type" content=3D"text/html; "> <meta name=3D"Generator" content=3D"MS Exchange Server version 5.5.2654.45"> <title>Error reporting in Hermes source code</title> <p><font size=3D"2">I'd like to make a comment or two about error catching and reporting. Please read this as constructive criticism aimed at improving Hermes: I'm definitely not trying to be negative.</fon= t></p> <p><font size=3D"2">While tracking down the problems I ran into yesterday, I noticed that the code that catches and reports exceptions leaves something to be desired.</font></p> <p><font size=3D"2">Example 1</font> </p> <p><font size=3D"2">While trying to track down a "Cannot parse SOAP message" error, I eventually got down to EbxmlMessage.getMessageFromDataSource(), and the following code:</font></= p> <p><font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 try {</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 soapMess= age =3D MessageFactory.newInstance().createMessage</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0 (headers, new ByteArrayInputStream(soapMessageBytes, 0,</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0 lastIndex + 1));</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 } catch (Exception e= ) {</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 throw ne= w SOAPException("Cannot parse SOAP message");</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 }</font> </p> <p><font size=3D"2">The actual exception from the createMessage() call has been discarded, and a new exception created. Unfortunately, any useful information in the original exception, which in this case was the extremely useful "no content-type found" exception, as well as the stack trace, is now lost. I had to insert a fair few debugging lines before I narrowed this down.</font></p> <p><font size=3D"2">This is a prime example of "if I don't know how to handle it, let someone else do it". The solution for cases like this is simple: don't catch exceptions when you don't need to catch them. In this case, since createMessage() throws SOAPException and IOException, and getMessageFromDataSource() already throws SOAPException, I would let getMessageFromDataSource() throw IOException as well, and let the exception be handled higher up.</font></p> <p><font size=3D"2">Example 2</font> </p> <p><font size=3D"2">While trying to track down a NullPointerException, I came across the following pattern, repeated quite a few times in MessageServiceHandler (for instance, but also other places) in various forms:</font></p> <p><font size=3D"2">=A0=A0=A0=A0=A0=A0=A0 try {</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ...</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0 }</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0 catch (Exception e) {</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 String err =3D Error= Messages.getMessage(</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ErrorMessages.ERR_HERMES_UNKNOWN_ERROR, e);</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 logger.error(err);</= font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 throw new MessageServiceHandlerException(err);</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0 }</font> </p> <p><font size=3D"2">Here we have an exception that should be fully reported. However, getMessage builds a String that tells us what the exception is, and what it tells us, but loses the associated stack trace, which is extremely useful for problem tracking. Note also that "logger.error(err)" only logs the text of the exception, not the stack trace.</font></p> <p><font size=3D"2">Again, I had to insert a few debugging lines just to figure out where the exception was being thrown, and what was throwing it, because this pattern creates a completely different stack trace beginning with the "throw new MessageServiceHandlerException()" line, which is pretty much useless. All I had to work with yesterday was the error text "NullPointerException" thrown from a catch clause that covers all of dispatchMessage(), which provides no help at all to track the problem down.</font></p> <p><font size=3D"2">There are one or two things to do here. Firstly and most importantly, use the logger to log more information. In particular, use the "logger.error(Object, Throwable)" form, instead of the "logger.error(Object)" form, so the exception and its stack trace get logged. (Likewise for the other logger methods.)</font></p> <p><font size=3D"2">Secondly (and optionally), I would bite the bullet and use Java1.4 to use the "Exception(String, Throwable)" form, so information about the original exception is maintained. However, given the logged stack trace from logger.error(), and the desire to still support Java1.3, you could probably avoid this one.</font></p> <p><font size=3D"2">This would result in the above pattern becoming:</f= ont> </p> <p><font size=3D"2">=A0=A0=A0=A0=A0=A0=A0 catch(Exception e) {</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 String errMsg =3D Er= rorMessages.getMessage(</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ErrorMessages.ERR_HERMES_UNKNOWN_ERROR);</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 logger.error(errMsg,= e);</font> </p> <p><font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 // either for Jav= a1.4</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 throw new MessageServicehandlerException(errMsg, e);</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 // or Java 1.3</font= > <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 throw new MessageServicehandlerException(errMsg);</font> <br> <font size=3D"2">=A0=A0=A0=A0=A0=A0=A0 }</font> </p> <p><font size=3D"2">These changes would make exceptions much easier to track down. They would also simplify ErrorsMessage.getMessage() a bit, since it would no longer need to know about the exception.</font></p> <p><font size=3D"2">I hope all this makes sense.</font> </p> <p><font size=3D"2">PJDM</font> <br> <font size=3D"2">--</font> <br> <font size=3D"2">Peter Mayne</font> <br> <font size=3D"2">Technology Consultant</font> <br> <font size=3D"2">Spherion Technology Solutions</font> <br> <font size=3D"2">Level 1, 243 Northbourne Avenue, Lyneham, ACT, 2602</f= ont> <br> <font size=3D"2">T: 61 2 62689727=A0 F: 61 2 62689777</font> </p> <!--[object_id=3D#ap.spherion.com#]--> <p align=3D"left">=A0</p> <p align=3D"left"><font face=3D"Tahoma" size=3D"2"><font color=3D"#0000= ff"><br> =A0</font></font></p> </blockquote> <br> </body> </html> |
