Re: Exception handling
Hi Eric, I think logging an exception with the context information (i.e. with a message describing the possible cause for this) would be of very much useful when it comes to the stack trace analysis so that by just looking at the log we can exactly find what went wrong. We are not just wrapping the exception in a SynapseException but also this SynapseException provides more information about the cause, again this is about the context of the error. Thanks, Ruwan On Thu, Apr 2, 2009 at 3:13 PM, Hubert, Eric wrote: > Hi Synapse devs, > > No worries, I don't want to start a discussion about exception handling in > general, I just would like to understand the reasoning of your design > choice. I stumbled over some areas in the code where I noticed that an > original exception is caught, logged AND rethrown wrapped inside a > SynapseException or SynapseUtilException (specific RuntimeExceptions). The > original exception is then mostly logged as an error, no matter whether the > caller may consider the exception to be critical or easily recoverable. If > he is able to recover, the original problem is already logged as an error > and he may only add a warning about the way he could recover. > > The good thing is that the almost the whole code looks pretty consistent in > this regard and I bet this is due to the following comment > "We always log each exception the first time it is caught, and throw a > SynapseException with an appropriate error message that tries to give out > additional information on why the exception occurred." > in the developer guidelines at > http://cwiki.apache.org/synapse/developer-guidelines.html (Would it make > sense to make this guideline easier available? Integrate or link it from the > main page?). > > What is the reasoning of this exception handling/logging approach? > > So far I was used to the following rule(s): > Either throw (chaining/wrapping of original exception) or log, except on > component boundaries where original exceptions have to be logged AND new > exceptions created and thrown (no chaining/wrapping of original exception > for this case, as implementation details and concrete stack/dependencies > shall be hidden from the caller). So far I never had problems with this > approach. What are other's experiences? > > Of course I will follow the chosen approach - this is more out of personal > interest. > > Regards, > Eric > -- Ruwan Linton Senior Software Engineer & Product Manager; WSO2 ESB; http://wso2.org/esb WSO2 Inc.; http://wso2.org email: ru...@wso2.com; cell: +94 77 341 3097 blog: http://ruwansblog.blogspot.com
Re: Exception handling
Hi Eric thanks for your reply. Humans life would be to boring, if we would always share the same opinion. ;-) Of course I may be wrong too and in some cases we maybe able to improve the code we already have, and I am fully open to change which is good for the project :-) Item 43: Throw exceptions appropriate to the abstraction This is very, very important. And at this point I would rather also like to add, that one sometimes should also take care to not violate this indirectly via chaining the cause which brings along all the details which should be abstracted. That's via a favour to create new exceptions at that point a properly the details at the abstraction boarder. For Synapse, I am not sure I fully agree.. since the main dependency with Axis2 sorts of causes this.. see we get messages from transports, into Axis2, then to Synapse for mediation, then again through Axis2 to transports and out etc.. I do not think keeping the original cause should be abstracted, but left as-is for easier problem resolution via logs etc. Ok, if you decide that you can handle this exception accordingly, I totally agree. But in this case why should you rethrow it? If you can't handle it and decide the caller might be able to handle it (not arguing about checked or unchecked at this point at all to concentrate on one topic), you can't log the error as it might be not a real problem for the caller which may be able to compensate. Logging some general context info for later correlation is another topic. This way you also avoid logging the same exception a couple of times, as the caller can't know whether something has already been logged and from his perspective it is the first time he is catching this exception. I don't think you use the SynapseException only for wrapping, but also for "self created" exceptions, or? Another one from Joshua "..To avoid this problem, higher layers should catch lower-level exceptions and, in their place, throw exceptions that are explainable in terms of the higher-level abstraction." - note that no where in the code do we catch a SynapseException and log or re-throw it.. thats bound by developer discipline so that each exception is only uniquely logged once with most of the information about its cause. Within the Synapse code I do not think there is any outside layer that is able to handle something the Synapse layer beneath could have handled.. I think we need to dig into the code and take some concrete examples, and see if we can really improve things with an alternative, and if we can definitely improve it Anyway it looks like there has already been some discussion about it and I do not want to change something about, just make sure I'm not missing a very special reason. ;-) No discussion is always good, and questioning to improve what we have .. I think best is to keep an eye for something related to exception handling we could improve when we next look at some piece of code, and then take it up for discussion/dissection and more analysis with everyone.. :-) cheers asankha -- Asankha C. Perera AdroitLogic, http://adroitlogic.org http://esbmagic.blogspot.com
RE: Exception handling
Hi Asankha, thanks for your reply. Humans life would be to boring, if we would always share the same opinion. ;-) > This basically follows some ideas from Joshua Bloch (Effective Java: > Programming Language Guide), and some discussions on the use of checked > exceptions vs runtime exceptions. Ah, cool. I also read Josh's books and followed a lot of those discussions. > Item 40:Use checked exceptions for recoverable conditions and run-time > exceptions for programming errors I agree with this. > Item 41:Avoid unnecessary use of checked exceptions Also agreed, although it is sometimes not that easy to correctly foresee whether the caller might be able to recover or not. In case of uncertainty I tend to assume the first. > Item 43: Throw exceptions appropriate to the abstraction This is very, very important. And at this point I would rather also like to add, that one sometimes should also take care to not violate this indirectly via chaining the cause which brings along all the details which should be abstracted. That's via a favour to create new exceptions at that point a properly the details at the abstraction boarder. > I personally follow logging of an Exception the first time you find out > about it (i.e. thrown from an external system to you), or you throwing > an exception - so that most of the context information about the > exception could be logged for analysis. Ok, if you decide that you can handle this exception accordingly, I totally agree. But in this case why should you rethrow it? If you can't handle it and decide the caller might be able to handle it (not arguing about checked or unchecked at this point at all to concentrate on one topic), you can't log the error as it might be not a real problem for the caller which may be able to compensate. Logging some general context info for later correlation is another topic. This way you also avoid logging the same exception a couple of times, as the caller can't know whether something has already been logged and from his perspective it is the first time he is catching this exception. I don't think you use the SynapseException only for wrapping, but also for "self created" exceptions, or? > Most of the cases where a > SynapseException is thrown are unrecoverable situations - since many are > dependent on a myriad of causes stemming from AxisFaults.. There are Hmm, you know the code for sure better, but I know for sure that this is at least sometimes not the case. Anyway it looks like there has already been some discussion about it and I do not want to change something about, just make sure I'm not missing a very special reason. ;-) Regards, Eric
Re: Exception handling
Hi Eric I stumbled over some areas in the code where I noticed that an original exception is caught, logged AND rethrown wrapped inside a SynapseException or SynapseUtilException (specific RuntimeExceptions). The original exception is then mostly logged as an error, no matter whether the caller may consider the exception to be critical or easily recoverable. If he is able to recover, the original problem is already logged as an error and he may only add a warning about the way he could recover. The good thing is that the almost the whole code looks pretty consistent in this regard and I bet this is due to the following comment "We always log each exception the first time it is caught, and throw a SynapseException with an appropriate error message that tries to give out additional information on why the exception occurred." in the developer guidelines at http://cwiki.apache.org/synapse/developer-guidelines.html (Would it make sense to make this guideline easier available? Integrate or link it from the main page?). What is the reasoning of this exception handling/logging approach? So far I was used to the following rule(s): Either throw (chaining/wrapping of original exception) or log, except on component boundaries where original exceptions have to be logged AND new exceptions created and thrown (no chaining/wrapping of original exception for this case, as implementation details and concrete stack/dependencies shall be hidden from the caller). So far I never had problems with this approach. What are other's experiences? Of course I will follow the chosen approach - this is more out of personal interest. This basically follows some ideas from Joshua Bloch (Effective Java: Programming Language Guide), and some discussions on the use of checked exceptions vs runtime exceptions. Item 40:Use checked exceptions for recoverable conditions and run-time exceptions for programming errors Item 41:Avoid unnecessary use of checked exceptions Item 43: Throw exceptions appropriate to the abstraction I personally follow logging of an Exception the first time you find out about it (i.e. thrown from an external system to you), or you throwing an exception - so that most of the context information about the exception could be logged for analysis. Most of the cases where a SynapseException is thrown are unrecoverable situations - since many are dependent on a myriad of causes stemming from AxisFaults.. There are places in the code where we catch these, and handle error recovery etc, where appropriate. If we know we can recover from some some types of errors (e.g. transport reconnects etc) we do that too.. cheers asankha [1] http://www.ibm.com/developerworks/java/library/j-jtp05254.html [2] http://java.sun.com/docs/books/tutorial/essential/exceptions/runtime.html -- Asankha C. Perera AdroitLogic, http://adroitlogic.org http://esbmagic.blogspot.com - To unsubscribe, e-mail: dev-unsubscr...@synapse.apache.org For additional commands, e-mail: dev-h...@synapse.apache.org
Re: Exception handling
Interesting question. Personally, I've always considered the log and throw approach as an anti-pattern. Andreas On Thu, Apr 2, 2009 at 11:43, Hubert, Eric wrote: > Hi Synapse devs, > > No worries, I don't want to start a discussion about exception handling in > general, I just would like to understand the reasoning of your design choice. > I stumbled over some areas in the code where I noticed that an original > exception is caught, logged AND rethrown wrapped inside a SynapseException or > SynapseUtilException (specific RuntimeExceptions). The original exception is > then mostly logged as an error, no matter whether the caller may consider the > exception to be critical or easily recoverable. If he is able to recover, the > original problem is already logged as an error and he may only add a warning > about the way he could recover. > > The good thing is that the almost the whole code looks pretty consistent in > this regard and I bet this is due to the following comment > "We always log each exception the first time it is caught, and throw a > SynapseException with an appropriate error message that tries to give out > additional information on why the exception occurred." > in the developer guidelines at > http://cwiki.apache.org/synapse/developer-guidelines.html (Would it make > sense to make this guideline easier available? Integrate or link it from the > main page?). > > What is the reasoning of this exception handling/logging approach? > > So far I was used to the following rule(s): > Either throw (chaining/wrapping of original exception) or log, except on > component boundaries where original exceptions have to be logged AND new > exceptions created and thrown (no chaining/wrapping of original exception for > this case, as implementation details and concrete stack/dependencies shall be > hidden from the caller). So far I never had problems with this approach. What > are other's experiences? > > Of course I will follow the chosen approach - this is more out of personal > interest. > > Regards, > Eric > - To unsubscribe, e-mail: dev-unsubscr...@synapse.apache.org For additional commands, e-mail: dev-h...@synapse.apache.org
