You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adrian Crum <ad...@sandglass-software.com> on 2012/05/05 22:55:26 UTC
Re: svn commit: r1334497 - in /ofbiz/trunk: applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/
applications/product/src/org/ofbiz/shipment/thirdparty/fedex/ applications/product/src/org/ofbiz/shipment/thirdparty/usps/
specialpurpose...
From my perspective, a bunch of catch blocks that all do the exact same
thing can be consolidated into a single catch block.
-Adrian
On 5/5/2012 9:46 PM, jleroux@apache.org wrote:
> Author: jleroux
> Date: Sat May 5 20:46:39 2012
> New Revision: 1334497
>
> URL: http://svn.apache.org/viewvc?rev=1334497&view=rev
> Log:
> Non functional changes.
> Removes a bunch of useless exceptions catches. There are still few cases to discuss, I believe there are useless (or at least without much more information) as well. we will decide on dev ML later...
>
> Modified:
> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java
> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java
> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java
> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java
> ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java
>
> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java (original)
> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java Sat May 5 20:46:39 2012
> @@ -19,7 +19,6 @@
> package org.ofbiz.accounting.thirdparty.clearcommerce;
>
> import java.io.ByteArrayOutputStream;
> -import java.io.IOException;
> import java.io.OutputStream;
> import java.math.BigDecimal;
> import java.util.ArrayList;
> @@ -28,7 +27,6 @@ import java.util.List;
> import java.util.Locale;
> import java.util.Map;
>
> -import javax.xml.parsers.ParserConfigurationException;
> import javax.xml.transform.TransformerException;
>
> import org.ofbiz.accounting.payment.PaymentGatewayServices;
> @@ -47,7 +45,6 @@ import org.ofbiz.service.DispatchContext
> import org.ofbiz.service.ServiceUtil;
> import org.w3c.dom.Document;
> import org.w3c.dom.Element;
> -import org.xml.sax.SAXException;
>
>
> /**
> @@ -921,12 +918,8 @@ public class CCPaymentServices {
> Document responseDocument = null;
> try {
> responseDocument = UtilXml.readXmlDocument(response, false);
> - } catch (SAXException se) {
> - throw new ClearCommerceException("Error reading response Document from a String: " + se.getMessage());
> - } catch (ParserConfigurationException pce) {
> - throw new ClearCommerceException("Error reading response Document from a String: " + pce.getMessage());
> - } catch (IOException ioe) {
> - throw new ClearCommerceException("Error reading response Document from a String: " + ioe.getMessage());
> + } catch (Exception e) {
> + throw new ClearCommerceException("Error reading response Document from a String: " + e.getMessage());
> }
> if (Debug.verboseOn()) Debug.logVerbose("Result severity from clearCommerce:" + getMessageListMaxSev(responseDocument), module);
> if (Debug.verboseOn()&& getMessageListMaxSev(responseDocument)> 4)
>
> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java (original)
> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java Sat May 5 20:46:39 2012
> @@ -19,7 +19,6 @@
>
> package org.ofbiz.shipment.thirdparty.fedex;
>
> -import java.io.IOException;
> import java.io.StringWriter;
> import java.math.BigDecimal;
> import java.sql.Timestamp;
> @@ -27,8 +26,6 @@ import java.util.List;
> import java.util.Locale;
> import java.util.Map;
>
> -import javax.xml.parsers.ParserConfigurationException;
> -
> import javolution.util.FastList;
> import javolution.util.FastMap;
>
> @@ -58,7 +55,6 @@ import org.ofbiz.service.ServiceUtil;
> import org.ofbiz.shipment.shipment.ShipmentServices;
> import org.w3c.dom.Document;
> import org.w3c.dom.Element;
> -import org.xml.sax.SAXException;
>
> /**
> * Fedex Shipment Services
> @@ -364,24 +360,12 @@ public class FedexServices {
> try {
> fDXSubscriptionReplyDocument = UtilXml.readXmlDocument(fDXSubscriptionReplyString, false);
> Debug.logInfo("Fedex response for FDXSubscriptionRequest:" + fDXSubscriptionReplyString, module);
> - } catch (SAXException se) {
> - String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + se.toString();
> - Debug.logError(se, errorMessage, module);
> - return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
> - "FacilityShipmentFedexSubscriptionTemplateParsingError",
> - UtilMisc.toMap("errorString", se.toString()), locale));
> - } catch (ParserConfigurationException pce) {
> - String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + pce.toString();
> - Debug.logError(pce, errorMessage, module);
> - return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
> - "FacilityShipmentFedexSubscriptionTemplateParsingError",
> - UtilMisc.toMap("errorString", pce.toString()), locale));
> - } catch (IOException ioe) {
> - String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + ioe.toString();
> - Debug.logError(ioe, errorMessage, module);
> + } catch (Exception e) {
> + String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + e.toString();
> + Debug.logError(e, errorMessage, module);
> return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
> "FacilityShipmentFedexSubscriptionTemplateParsingError",
> - UtilMisc.toMap("errorString", ioe.toString()), locale));
> + UtilMisc.toMap("errorString", e.toString()), locale));
> }
>
> Element fedexSubscriptionReplyElement = fDXSubscriptionReplyDocument.getDocumentElement();
> @@ -996,17 +980,9 @@ public class FedexServices {
> Document fdxShipReplyDocument = null;
> try {
> fdxShipReplyDocument = UtilXml.readXmlDocument(fDXShipReplyString, false);
> - } catch (SAXException se) {
> - String errorMessage = "Error parsing the FDXShipReply: " + se.toString();
> - Debug.logError(se, errorMessage, module);
> - // TODO: Cancel the package
> - } catch (ParserConfigurationException pe) {
> - String errorMessage = "Error parsing the FDXShipReply: " + pe.toString();
> - Debug.logError(pe, errorMessage, module);
> - // TODO Cancel the package
> - } catch (IOException ioe) {
> - String errorMessage = "Error parsing the FDXShipReply: " + ioe.toString();
> - Debug.logError(ioe, errorMessage, module);
> + } catch (Exception e) {
> + String errorMessage = "Error parsing the FDXShipReply: " + e.toString();
> + Debug.logError(e, errorMessage, module);
> // TODO Cancel the package
> }
>
>
> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java?rev=1334497&r1=1334496&r2=1334497&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java (original)
> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java Sat May 5 20:46:39 2012
> @@ -30,16 +30,13 @@ import javax.servlet.ServletOutputStream
> import javax.servlet.http.HttpServlet;
> import javax.servlet.http.HttpServletRequest;
> import javax.servlet.http.HttpServletResponse;
> -import javax.xml.parsers.ParserConfigurationException;
> import javax.xml.transform.TransformerException;
>
> +import org.ofbiz.base.util.Debug;
> import org.ofbiz.base.util.UtilValidate;
> import org.ofbiz.base.util.UtilXml;
> -import org.ofbiz.base.util.Debug;
> -
> import org.w3c.dom.Document;
> import org.w3c.dom.Element;
> -import org.xml.sax.SAXException;
>
> /**
> * USPS Webtools API Mock API Servlet
> @@ -77,14 +74,8 @@ public class UspsMockApiServlet extends
> Document requestDocument = null;
> try {
> requestDocument = UtilXml.readXmlDocument(xmlValue, false);
> - } catch (SAXException se) {
> - Debug.logError(se, module);
> - return;
> - } catch (ParserConfigurationException pce) {
> - Debug.logError(pce, module);
> - return;
> - } catch (IOException xmlReadException) {
> - Debug.logError(xmlReadException, module);
> + } catch (Exception e) {
> + Debug.logError(e, module);
> return;
> }
>
>
> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java (original)
> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java Sat May 5 20:46:39 2012
> @@ -32,7 +32,6 @@ import java.util.ListIterator;
> import java.util.Locale;
> import java.util.Map;
>
> -import javax.xml.parsers.ParserConfigurationException;
> import javax.xml.transform.TransformerException;
>
> import javolution.util.FastList;
> @@ -67,7 +66,6 @@ import org.ofbiz.shipment.shipment.Shipm
> import org.ofbiz.shipment.shipment.ShipmentWorker;
> import org.w3c.dom.Document;
> import org.w3c.dom.Element;
> -import org.xml.sax.SAXException;
>
> /**
> * USPS Webtools API Services
> @@ -1757,18 +1755,8 @@ public class UspsServices {
> Document responseDocument = null;
> try {
> responseDocument = UtilXml.readXmlDocument(responseString, false);
> - } catch (SAXException se) {
> - throw new UspsRequestException(UtilProperties.getMessage(resourceError,
> - "FacilityShipmentUspsResponseError",
> - UtilMisc.toMap("errorString", se.getMessage()), locale));
> - } catch (ParserConfigurationException pce) {
> - throw new UspsRequestException(UtilProperties.getMessage(resourceError,
> - "FacilityShipmentUspsResponseError",
> - UtilMisc.toMap("errorString", pce.getMessage()), locale));
> - } catch (IOException xmlReadException) {
> - throw new UspsRequestException(UtilProperties.getMessage(resourceError,
> - "FacilityShipmentUspsResponseError",
> - UtilMisc.toMap("errorString", xmlReadException.getMessage()), locale));
> + } catch (Exception e) {
> + throw new UspsRequestException(UtilProperties.getMessage(resourceError, "FacilityShipmentUspsResponseError", UtilMisc.toMap("errorString", e.getMessage()), locale));
> }
>
> // If a top-level error document is returned, throw exception
>
> Modified: ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
> ==============================================================================
> --- ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java (original)
> +++ ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java Sat May 5 20:46:39 2012
> @@ -25,8 +25,6 @@ import java.util.HashMap;
> import java.util.List;
> import java.util.Map;
>
> -import javax.xml.parsers.ParserConfigurationException;
> -
> import org.ofbiz.base.util.Debug;
> import org.ofbiz.base.util.GeneralException;
> import org.ofbiz.base.util.ObjectType;
> @@ -41,7 +39,6 @@ import org.ofbiz.entity.serialize.XmlSer
> import org.ofbiz.service.DispatchContext;
> import org.ofbiz.service.GenericServiceException;
> import org.ofbiz.service.ModelService;
> -import org.xml.sax.SAXException;
>
> /**
> * Workflow Application Services - 'Services' and 'Workers' for interaction with Workflow Application API
> @@ -219,14 +216,8 @@ public class WfApplicationServices {
> throws GenericServiceException {
> try {
> return UtilGenerics.checkMap(XmlSerializer.deserialize((String) runTimeData.get("runtimeInfo"), delegator));
> - } catch (SerializeException se) {
> - throw new GenericServiceException(se.getMessage(), se);
> - } catch (ParserConfigurationException pe) {
> - throw new GenericServiceException(pe.getMessage(), pe);
> - } catch (SAXException se) {
> - throw new GenericServiceException(se.getMessage(), se);
> - } catch (IOException ioe) {
> - throw new GenericServiceException(ioe.getMessage(), ioe);
> + } catch (Exception e) {
> + throw new GenericServiceException(e.getMessage(), e);
> }
> }
>
>
>
Re: Try/Catch Patterns (was: svn commit: r1334497 ...)
Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Scott Gray" <sc...@hotwaxmedia.com>
> On 6/05/2012, at 10:43 PM, Jacques Le Roux wrote:
>
>> From: "Scott Gray" <sc...@hotwaxmedia.com>
>>> On 6/05/2012, at 9:28 PM, Jacques Le Roux wrote:
>>>
>>>> From: "Adrian Crum" <ad...@sandglass-software.com>
>>>>> On 5/6/2012 10:07 AM, Jacques Le Roux wrote:
>>>>>> Nobody is speaking about swallowing, I hope we have not such things in OFBiz! (though Scott's wait to read the end before
>>>>>> thinking I did not get you and being upset ;o)
>>>>>>
>>>>>> No, it's more about stuff like
>>>>>>
>>>>>> } catch (BirtException e) {
>>>>>> throw new ViewHandlerException("Birt Error create engine: " + e.toString(), e);
>>>>>> } catch (IOException e) {
>>>>>> throw new ViewHandlerException("Error in the response writer/output stream: " + e.toString(), e);
>>>>>> } catch (SQLException e) {
>>>>>> throw new ViewHandlerException("get connection error: " + e.toString(), e);
>>>>>> } catch (GenericEntityException e) {
>>>>>> throw new ViewHandlerException("generic entity error: " + e.toString(), e);
>>>>>> } catch (GeneralException e) {
>>>>>> throw new ViewHandlerException("general error: " + e.toString(), e);
>>>>>> } catch (SAXException se) {
>>>>>> String errMsg = "Error SAX rendering " + page + " view handler: " + se.toString();
>>>>>> Debug.logError(se, errMsg, module);
>>>>>> throw new ViewHandlerException(errMsg, se);
>>>>>> } catch (ParserConfigurationException pe) {
>>>>>> String errMsg = "Error parser rendering " + page + " view handler: " + pe.toString();
>>>>>> Debug.logError(pe, errMsg, module);
>>>>>> throw new ViewHandlerException(errMsg, pe);
>>>>>> }
>>>>>>
>>>>>>
>>>>>> } catch (GeneralException ge) {
>>>>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + ge.toString();
>>>>>> Debug.logError(ge, errMsg, module);
>>>>>> return ServiceUtil.returnError(errMsg);
>>>>>> } catch (IOException ie) {
>>>>>> String errMsg = "Error I/O rendering " + birtContentType + " attachment for email: " + ie.toString();
>>>>>> Debug.logError(ie, errMsg, module);
>>>>>> return ServiceUtil.returnError(errMsg);
>>>>>> } catch (FOPException fe) {
>>>>>> String errMsg = "Error FOP rendering " + birtContentType + " attachment for email: " + fe.toString();
>>>>>> Debug.logError(fe, errMsg, module);
>>>>>> return ServiceUtil.returnError(errMsg);
>>>>>> } catch (SAXException se) {
>>>>>> String errMsg = "Error SAX rendering " + birtContentType + " attachment for email: " + se.toString();
>>>>>> Debug.logError(se, errMsg, module);
>>>>>> return ServiceUtil.returnError(errMsg);
>>>>>> } catch (ParserConfigurationException pe) {
>>>>>> String errMsg = "Error parser rendering " + birtContentType + " attachment for email: " + pe.toString();
>>>>>> Debug.logError(pe, errMsg, module);
>>>>>> return ServiceUtil.returnError(errMsg);
>>>>>> } catch (EngineException ee) {
>>>>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + ee.toString();
>>>>>> Debug.logError(ee, errMsg, module);
>>>>>> return ServiceUtil.returnError(errMsg);
>>>>>> } catch (SQLException se) {
>>>>>> String errMsg = "Error SQL rendering " + birtContentType + " attachment for email: " + se.toString();
>>>>>> Debug.logError(se, errMsg, module);
>>>>>> return ServiceUtil.returnError(errMsg);
>>>>>> }
>>>>>>
>>>>>> I think the
>>>>>> exception.toString()
>>>>>> Debug.logError(exception, errMsg, module);
>>>>>> information is enough. This for 4 reasons:
>>>>>>
>>>>>> 1. exception.toString(), and exception in Debug.logError give enough information. No needs to specify things like
>>>>>> "Error I/O rendering "
>>>>>> "Error FOP rendering"
>>>>>> etc.
>>>>>> this will anyway be given by the thrown exception
>>>>>> 2. It's more legible
>>>>>> 3. It's an antipattern people C/P (like I did) so it spreads everywhere in code
>>>>>> 4. Those kinds of exception are exceptionally thrown. BTW I personaly believe checked exceptions in Java is not really the
>>>>>> best part
>>>>>> of the language. As soons as it has spread in library and at large code, you have to suffer all those try/catch blocks
>>>>>>
>>>>>> So this like below, this can be rewritten
>>>>>>
>>>>>> } catch (Exception e) {
>>>>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + e.toString();
>>>>>> Debug.logError(e, errMsg, module);
>>>>>> return ServiceUtil.returnError(errMsg);
>>>>>> }
>>>>>>
>>>>> The "+ e.toString()" is not needed because Debug will print the Exception's message anyway. I usually delete those because
>>>>> they make the log harder to read.
>>>>> -Adrian
>>>>
>>>> Yes indeed, when you use the ternary version of Debug.log like in "Debug.logError(e, errMsg, module);"
>>>> Another thing we can do to clean the error stack in log...
>>>>
>>>> Jacques
>>>
>>>
>>> 1. There's nothing wrong with swallowing exceptions if the situation calls for it
>>> 2. I was merely pointing out that you can't simply take Adrian's comment and apply it everywhere as some golden rule. Not only
>>> that, most resources I've seen consider it to be an anti-pattern anyway.
>>>
>>> Regards
>>> Scott
>>
>> About 2. : I consider it an anti-pattern because, in cases like above, it clutters the code uselessly to give redundant
>> information (also given by the exception itself).
>> I did not mean an anti-pattern in all situations of course, especially when you want to give more information than only what the
>> exception can tell.
>>
>> So if nobody disagree, in case like above, I will removed "useless" exceptions catches in current code.
>>
>> Jacques
>
> I disagree, I was referring to Adrian's suggestion as an anti-pattern. The whole reason I replied is because I knew someone
> reading his comment would take it upon themselves to begin mass converting the entire codebase, as if some gospel had just been
> handed down that must be spread throughout the land. One reason not to do it is because you will now be catching all exceptions
> even the ones you don't know about that may be added to the API in the future and may require different handling. Just because
> everything is handled in the same manner now doesn't mean that it will be in the future. If we follow this path we won't even
> know that a new type exception has been added.
>
> The way things are now does absolutely no harm and is safer than what you are proposing, at the mere cost of a few extra lines of
> code I don't see a need to change them.
>
> Regards
> Scott
OK, no worries
Jacques
Re: Try/Catch Patterns (was: svn commit: r1334497 ...)
Posted by Scott Gray <sc...@hotwaxmedia.com>.
On 6/05/2012, at 10:43 PM, Jacques Le Roux wrote:
> From: "Scott Gray" <sc...@hotwaxmedia.com>
>> On 6/05/2012, at 9:28 PM, Jacques Le Roux wrote:
>>
>>> From: "Adrian Crum" <ad...@sandglass-software.com>
>>>> On 5/6/2012 10:07 AM, Jacques Le Roux wrote:
>>>>> Nobody is speaking about swallowing, I hope we have not such things in OFBiz! (though Scott's wait to read the end before thinking I did not get you and being upset ;o)
>>>>>
>>>>> No, it's more about stuff like
>>>>>
>>>>> } catch (BirtException e) {
>>>>> throw new ViewHandlerException("Birt Error create engine: " + e.toString(), e);
>>>>> } catch (IOException e) {
>>>>> throw new ViewHandlerException("Error in the response writer/output stream: " + e.toString(), e);
>>>>> } catch (SQLException e) {
>>>>> throw new ViewHandlerException("get connection error: " + e.toString(), e);
>>>>> } catch (GenericEntityException e) {
>>>>> throw new ViewHandlerException("generic entity error: " + e.toString(), e);
>>>>> } catch (GeneralException e) {
>>>>> throw new ViewHandlerException("general error: " + e.toString(), e);
>>>>> } catch (SAXException se) {
>>>>> String errMsg = "Error SAX rendering " + page + " view handler: " + se.toString();
>>>>> Debug.logError(se, errMsg, module);
>>>>> throw new ViewHandlerException(errMsg, se);
>>>>> } catch (ParserConfigurationException pe) {
>>>>> String errMsg = "Error parser rendering " + page + " view handler: " + pe.toString();
>>>>> Debug.logError(pe, errMsg, module);
>>>>> throw new ViewHandlerException(errMsg, pe);
>>>>> }
>>>>>
>>>>>
>>>>> } catch (GeneralException ge) {
>>>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + ge.toString();
>>>>> Debug.logError(ge, errMsg, module);
>>>>> return ServiceUtil.returnError(errMsg);
>>>>> } catch (IOException ie) {
>>>>> String errMsg = "Error I/O rendering " + birtContentType + " attachment for email: " + ie.toString();
>>>>> Debug.logError(ie, errMsg, module);
>>>>> return ServiceUtil.returnError(errMsg);
>>>>> } catch (FOPException fe) {
>>>>> String errMsg = "Error FOP rendering " + birtContentType + " attachment for email: " + fe.toString();
>>>>> Debug.logError(fe, errMsg, module);
>>>>> return ServiceUtil.returnError(errMsg);
>>>>> } catch (SAXException se) {
>>>>> String errMsg = "Error SAX rendering " + birtContentType + " attachment for email: " + se.toString();
>>>>> Debug.logError(se, errMsg, module);
>>>>> return ServiceUtil.returnError(errMsg);
>>>>> } catch (ParserConfigurationException pe) {
>>>>> String errMsg = "Error parser rendering " + birtContentType + " attachment for email: " + pe.toString();
>>>>> Debug.logError(pe, errMsg, module);
>>>>> return ServiceUtil.returnError(errMsg);
>>>>> } catch (EngineException ee) {
>>>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + ee.toString();
>>>>> Debug.logError(ee, errMsg, module);
>>>>> return ServiceUtil.returnError(errMsg);
>>>>> } catch (SQLException se) {
>>>>> String errMsg = "Error SQL rendering " + birtContentType + " attachment for email: " + se.toString();
>>>>> Debug.logError(se, errMsg, module);
>>>>> return ServiceUtil.returnError(errMsg);
>>>>> }
>>>>>
>>>>> I think the
>>>>> exception.toString()
>>>>> Debug.logError(exception, errMsg, module);
>>>>> information is enough. This for 4 reasons:
>>>>>
>>>>> 1. exception.toString(), and exception in Debug.logError give enough information. No needs to specify things like
>>>>> "Error I/O rendering "
>>>>> "Error FOP rendering"
>>>>> etc.
>>>>> this will anyway be given by the thrown exception
>>>>> 2. It's more legible
>>>>> 3. It's an antipattern people C/P (like I did) so it spreads everywhere in code
>>>>> 4. Those kinds of exception are exceptionally thrown. BTW I personaly believe checked exceptions in Java is not really the best part
>>>>> of the language. As soons as it has spread in library and at large code, you have to suffer all those try/catch blocks
>>>>>
>>>>> So this like below, this can be rewritten
>>>>>
>>>>> } catch (Exception e) {
>>>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + e.toString();
>>>>> Debug.logError(e, errMsg, module);
>>>>> return ServiceUtil.returnError(errMsg);
>>>>> }
>>>>>
>>>> The "+ e.toString()" is not needed because Debug will print the Exception's message anyway. I usually delete those because they make the log harder to read.
>>>> -Adrian
>>>
>>> Yes indeed, when you use the ternary version of Debug.log like in "Debug.logError(e, errMsg, module);"
>>> Another thing we can do to clean the error stack in log...
>>>
>>> Jacques
>>
>>
>> 1. There's nothing wrong with swallowing exceptions if the situation calls for it
>> 2. I was merely pointing out that you can't simply take Adrian's comment and apply it everywhere as some golden rule. Not only that, most resources I've seen consider it to be an anti-pattern anyway.
>>
>> Regards
>> Scott
>
> About 2. : I consider it an anti-pattern because, in cases like above, it clutters the code uselessly to give redundant information (also given by the exception itself).
> I did not mean an anti-pattern in all situations of course, especially when you want to give more information than only what the exception can tell.
>
> So if nobody disagree, in case like above, I will removed "useless" exceptions catches in current code.
>
> Jacques
I disagree, I was referring to Adrian's suggestion as an anti-pattern. The whole reason I replied is because I knew someone reading his comment would take it upon themselves to begin mass converting the entire codebase, as if some gospel had just been handed down that must be spread throughout the land. One reason not to do it is because you will now be catching all exceptions even the ones you don't know about that may be added to the API in the future and may require different handling. Just because everything is handled in the same manner now doesn't mean that it will be in the future. If we follow this path we won't even know that a new type exception has been added.
The way things are now does absolutely no harm and is safer than what you are proposing, at the mere cost of a few extra lines of code I don't see a need to change them.
Regards
Scott
Re: Try/Catch Patterns (was: svn commit: r1334497 ...)
Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Scott Gray" <sc...@hotwaxmedia.com>
> On 6/05/2012, at 9:28 PM, Jacques Le Roux wrote:
>
>> From: "Adrian Crum" <ad...@sandglass-software.com>
>>> On 5/6/2012 10:07 AM, Jacques Le Roux wrote:
>>>> Nobody is speaking about swallowing, I hope we have not such things in OFBiz! (though Scott's wait to read the end before
>>>> thinking I did not get you and being upset ;o)
>>>>
>>>> No, it's more about stuff like
>>>>
>>>> } catch (BirtException e) {
>>>> throw new ViewHandlerException("Birt Error create engine: " + e.toString(), e);
>>>> } catch (IOException e) {
>>>> throw new ViewHandlerException("Error in the response writer/output stream: " + e.toString(), e);
>>>> } catch (SQLException e) {
>>>> throw new ViewHandlerException("get connection error: " + e.toString(), e);
>>>> } catch (GenericEntityException e) {
>>>> throw new ViewHandlerException("generic entity error: " + e.toString(), e);
>>>> } catch (GeneralException e) {
>>>> throw new ViewHandlerException("general error: " + e.toString(), e);
>>>> } catch (SAXException se) {
>>>> String errMsg = "Error SAX rendering " + page + " view handler: " + se.toString();
>>>> Debug.logError(se, errMsg, module);
>>>> throw new ViewHandlerException(errMsg, se);
>>>> } catch (ParserConfigurationException pe) {
>>>> String errMsg = "Error parser rendering " + page + " view handler: " + pe.toString();
>>>> Debug.logError(pe, errMsg, module);
>>>> throw new ViewHandlerException(errMsg, pe);
>>>> }
>>>>
>>>>
>>>> } catch (GeneralException ge) {
>>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + ge.toString();
>>>> Debug.logError(ge, errMsg, module);
>>>> return ServiceUtil.returnError(errMsg);
>>>> } catch (IOException ie) {
>>>> String errMsg = "Error I/O rendering " + birtContentType + " attachment for email: " + ie.toString();
>>>> Debug.logError(ie, errMsg, module);
>>>> return ServiceUtil.returnError(errMsg);
>>>> } catch (FOPException fe) {
>>>> String errMsg = "Error FOP rendering " + birtContentType + " attachment for email: " + fe.toString();
>>>> Debug.logError(fe, errMsg, module);
>>>> return ServiceUtil.returnError(errMsg);
>>>> } catch (SAXException se) {
>>>> String errMsg = "Error SAX rendering " + birtContentType + " attachment for email: " + se.toString();
>>>> Debug.logError(se, errMsg, module);
>>>> return ServiceUtil.returnError(errMsg);
>>>> } catch (ParserConfigurationException pe) {
>>>> String errMsg = "Error parser rendering " + birtContentType + " attachment for email: " + pe.toString();
>>>> Debug.logError(pe, errMsg, module);
>>>> return ServiceUtil.returnError(errMsg);
>>>> } catch (EngineException ee) {
>>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + ee.toString();
>>>> Debug.logError(ee, errMsg, module);
>>>> return ServiceUtil.returnError(errMsg);
>>>> } catch (SQLException se) {
>>>> String errMsg = "Error SQL rendering " + birtContentType + " attachment for email: " + se.toString();
>>>> Debug.logError(se, errMsg, module);
>>>> return ServiceUtil.returnError(errMsg);
>>>> }
>>>>
>>>> I think the
>>>> exception.toString()
>>>> Debug.logError(exception, errMsg, module);
>>>> information is enough. This for 4 reasons:
>>>>
>>>> 1. exception.toString(), and exception in Debug.logError give enough information. No needs to specify things like
>>>> "Error I/O rendering "
>>>> "Error FOP rendering"
>>>> etc.
>>>> this will anyway be given by the thrown exception
>>>> 2. It's more legible
>>>> 3. It's an antipattern people C/P (like I did) so it spreads everywhere in code
>>>> 4. Those kinds of exception are exceptionally thrown. BTW I personaly believe checked exceptions in Java is not really the best
>>>> part
>>>> of the language. As soons as it has spread in library and at large code, you have to suffer all those try/catch blocks
>>>>
>>>> So this like below, this can be rewritten
>>>>
>>>> } catch (Exception e) {
>>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + e.toString();
>>>> Debug.logError(e, errMsg, module);
>>>> return ServiceUtil.returnError(errMsg);
>>>> }
>>>>
>>> The "+ e.toString()" is not needed because Debug will print the Exception's message anyway. I usually delete those because they
>>> make the log harder to read.
>>> -Adrian
>>
>> Yes indeed, when you use the ternary version of Debug.log like in "Debug.logError(e, errMsg, module);"
>> Another thing we can do to clean the error stack in log...
>>
>> Jacques
>
>
> 1. There's nothing wrong with swallowing exceptions if the situation calls for it
> 2. I was merely pointing out that you can't simply take Adrian's comment and apply it everywhere as some golden rule. Not only
> that, most resources I've seen consider it to be an anti-pattern anyway.
>
> Regards
> Scott
About 2. : I consider it an anti-pattern because, in cases like above, it clutters the code uselessly to give redundant information
(also given by the exception itself).
I did not mean an anti-pattern in all situations of course, especially when you want to give more information than only what the
exception can tell.
So if nobody disagree, in case like above, I will removed "useless" exceptions catches in current code.
Jacques
Re: Try/Catch Patterns (was: svn commit: r1334497 ...)
Posted by Scott Gray <sc...@hotwaxmedia.com>.
On 6/05/2012, at 9:28 PM, Jacques Le Roux wrote:
> From: "Adrian Crum" <ad...@sandglass-software.com>
>> On 5/6/2012 10:07 AM, Jacques Le Roux wrote:
>>> Nobody is speaking about swallowing, I hope we have not such things in OFBiz! (though Scott's wait to read the end before thinking I did not get you and being upset ;o)
>>>
>>> No, it's more about stuff like
>>>
>>> } catch (BirtException e) {
>>> throw new ViewHandlerException("Birt Error create engine: " + e.toString(), e);
>>> } catch (IOException e) {
>>> throw new ViewHandlerException("Error in the response writer/output stream: " + e.toString(), e);
>>> } catch (SQLException e) {
>>> throw new ViewHandlerException("get connection error: " + e.toString(), e);
>>> } catch (GenericEntityException e) {
>>> throw new ViewHandlerException("generic entity error: " + e.toString(), e);
>>> } catch (GeneralException e) {
>>> throw new ViewHandlerException("general error: " + e.toString(), e);
>>> } catch (SAXException se) {
>>> String errMsg = "Error SAX rendering " + page + " view handler: " + se.toString();
>>> Debug.logError(se, errMsg, module);
>>> throw new ViewHandlerException(errMsg, se);
>>> } catch (ParserConfigurationException pe) {
>>> String errMsg = "Error parser rendering " + page + " view handler: " + pe.toString();
>>> Debug.logError(pe, errMsg, module);
>>> throw new ViewHandlerException(errMsg, pe);
>>> }
>>>
>>>
>>> } catch (GeneralException ge) {
>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + ge.toString();
>>> Debug.logError(ge, errMsg, module);
>>> return ServiceUtil.returnError(errMsg);
>>> } catch (IOException ie) {
>>> String errMsg = "Error I/O rendering " + birtContentType + " attachment for email: " + ie.toString();
>>> Debug.logError(ie, errMsg, module);
>>> return ServiceUtil.returnError(errMsg);
>>> } catch (FOPException fe) {
>>> String errMsg = "Error FOP rendering " + birtContentType + " attachment for email: " + fe.toString();
>>> Debug.logError(fe, errMsg, module);
>>> return ServiceUtil.returnError(errMsg);
>>> } catch (SAXException se) {
>>> String errMsg = "Error SAX rendering " + birtContentType + " attachment for email: " + se.toString();
>>> Debug.logError(se, errMsg, module);
>>> return ServiceUtil.returnError(errMsg);
>>> } catch (ParserConfigurationException pe) {
>>> String errMsg = "Error parser rendering " + birtContentType + " attachment for email: " + pe.toString();
>>> Debug.logError(pe, errMsg, module);
>>> return ServiceUtil.returnError(errMsg);
>>> } catch (EngineException ee) {
>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + ee.toString();
>>> Debug.logError(ee, errMsg, module);
>>> return ServiceUtil.returnError(errMsg);
>>> } catch (SQLException se) {
>>> String errMsg = "Error SQL rendering " + birtContentType + " attachment for email: " + se.toString();
>>> Debug.logError(se, errMsg, module);
>>> return ServiceUtil.returnError(errMsg);
>>> }
>>>
>>> I think the
>>> exception.toString()
>>> Debug.logError(exception, errMsg, module);
>>> information is enough. This for 4 reasons:
>>>
>>> 1. exception.toString(), and exception in Debug.logError give enough information. No needs to specify things like
>>> "Error I/O rendering "
>>> "Error FOP rendering"
>>> etc.
>>> this will anyway be given by the thrown exception
>>> 2. It's more legible
>>> 3. It's an antipattern people C/P (like I did) so it spreads everywhere in code
>>> 4. Those kinds of exception are exceptionally thrown. BTW I personaly believe checked exceptions in Java is not really the best part
>>> of the language. As soons as it has spread in library and at large code, you have to suffer all those try/catch blocks
>>>
>>> So this like below, this can be rewritten
>>>
>>> } catch (Exception e) {
>>> String errMsg = "Error rendering " + birtContentType + " attachment for email: " + e.toString();
>>> Debug.logError(e, errMsg, module);
>>> return ServiceUtil.returnError(errMsg);
>>> }
>>>
>> The "+ e.toString()" is not needed because Debug will print the Exception's message anyway. I usually delete those because they make the log harder to read.
>> -Adrian
>
> Yes indeed, when you use the ternary version of Debug.log like in "Debug.logError(e, errMsg, module);"
> Another thing we can do to clean the error stack in log...
>
> Jacques
1. There's nothing wrong with swallowing exceptions if the situation calls for it
2. I was merely pointing out that you can't simply take Adrian's comment and apply it everywhere as some golden rule. Not only that, most resources I've seen consider it to be an anti-pattern anyway.
Regards
Scott
Re: Try/Catch Patterns (was: svn commit: r1334497 ...)
Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Adrian Crum" <ad...@sandglass-software.com>
> On 5/6/2012 10:07 AM, Jacques Le Roux wrote:
>> Nobody is speaking about swallowing, I hope we have not such things in
>> OFBiz! (though Scott's wait to read the end before thinking I did not
>> get you and being upset ;o)
>>
>> No, it's more about stuff like
>>
>> } catch (BirtException e) {
>> throw new ViewHandlerException("Birt Error create engine: " +
>> e.toString(), e);
>> } catch (IOException e) {
>> throw new ViewHandlerException("Error in the response writer/output
>> stream: " + e.toString(), e);
>> } catch (SQLException e) {
>> throw new ViewHandlerException("get connection error: " +
>> e.toString(), e);
>> } catch (GenericEntityException e) {
>> throw new ViewHandlerException("generic entity error: " +
>> e.toString(), e);
>> } catch (GeneralException e) {
>> throw new ViewHandlerException("general error: " + e.toString(), e);
>> } catch (SAXException se) {
>> String errMsg = "Error SAX rendering " + page + " view handler: " +
>> se.toString();
>> Debug.logError(se, errMsg, module);
>> throw new ViewHandlerException(errMsg, se);
>> } catch (ParserConfigurationException pe) {
>> String errMsg = "Error parser rendering " + page + " view handler:
>> " + pe.toString();
>> Debug.logError(pe, errMsg, module);
>> throw new ViewHandlerException(errMsg, pe);
>> }
>>
>>
>> } catch (GeneralException ge) {
>> String errMsg = "Error rendering " + birtContentType + " attachment
>> for email: " + ge.toString();
>> Debug.logError(ge, errMsg, module);
>> return ServiceUtil.returnError(errMsg);
>> } catch (IOException ie) {
>> String errMsg = "Error I/O rendering " + birtContentType + "
>> attachment for email: " + ie.toString();
>> Debug.logError(ie, errMsg, module);
>> return ServiceUtil.returnError(errMsg);
>> } catch (FOPException fe) {
>> String errMsg = "Error FOP rendering " + birtContentType + "
>> attachment for email: " + fe.toString();
>> Debug.logError(fe, errMsg, module);
>> return ServiceUtil.returnError(errMsg);
>> } catch (SAXException se) {
>> String errMsg = "Error SAX rendering " + birtContentType + "
>> attachment for email: " + se.toString();
>> Debug.logError(se, errMsg, module);
>> return ServiceUtil.returnError(errMsg);
>> } catch (ParserConfigurationException pe) {
>> String errMsg = "Error parser rendering " + birtContentType + "
>> attachment for email: " + pe.toString();
>> Debug.logError(pe, errMsg, module);
>> return ServiceUtil.returnError(errMsg);
>> } catch (EngineException ee) {
>> String errMsg = "Error rendering " + birtContentType + " attachment
>> for email: " + ee.toString();
>> Debug.logError(ee, errMsg, module);
>> return ServiceUtil.returnError(errMsg);
>> } catch (SQLException se) {
>> String errMsg = "Error SQL rendering " + birtContentType + "
>> attachment for email: " + se.toString();
>> Debug.logError(se, errMsg, module);
>> return ServiceUtil.returnError(errMsg);
>> }
>>
>> I think the
>> exception.toString()
>> Debug.logError(exception, errMsg, module);
>> information is enough. This for 4 reasons:
>>
>> 1. exception.toString(), and exception in Debug.logError give enough
>> information. No needs to specify things like
>> "Error I/O rendering "
>> "Error FOP rendering"
>> etc.
>> this will anyway be given by the thrown exception
>> 2. It's more legible
>> 3. It's an antipattern people C/P (like I did) so it spreads
>> everywhere in code
>> 4. Those kinds of exception are exceptionally thrown. BTW I personaly
>> believe checked exceptions in Java is not really the best part
>> of the language. As soons as it has spread in library and at large
>> code, you have to suffer all those try/catch blocks
>>
>> So this like below, this can be rewritten
>>
>> } catch (Exception e) {
>> String errMsg = "Error rendering " + birtContentType + " attachment
>> for email: " + e.toString();
>> Debug.logError(e, errMsg, module);
>> return ServiceUtil.returnError(errMsg);
>> }
>>
>
> The "+ e.toString()" is not needed because Debug will print the
> Exception's message anyway. I usually delete those because they make the
> log harder to read.
>
> -Adrian
Yes indeed, when you use the ternary version of Debug.log like in "Debug.logError(e, errMsg, module);"
Another thing we can do to clean the error stack in log...
Jacques
Try/Catch Patterns (was: svn commit: r1334497 ...)
Posted by Adrian Crum <ad...@sandglass-software.com>.
On 5/6/2012 10:07 AM, Jacques Le Roux wrote:
> Nobody is speaking about swallowing, I hope we have not such things in
> OFBiz! (though Scott's wait to read the end before thinking I did not
> get you and being upset ;o)
>
> No, it's more about stuff like
>
> } catch (BirtException e) {
> throw new ViewHandlerException("Birt Error create engine: " +
> e.toString(), e);
> } catch (IOException e) {
> throw new ViewHandlerException("Error in the response writer/output
> stream: " + e.toString(), e);
> } catch (SQLException e) {
> throw new ViewHandlerException("get connection error: " +
> e.toString(), e);
> } catch (GenericEntityException e) {
> throw new ViewHandlerException("generic entity error: " +
> e.toString(), e);
> } catch (GeneralException e) {
> throw new ViewHandlerException("general error: " + e.toString(), e);
> } catch (SAXException se) {
> String errMsg = "Error SAX rendering " + page + " view handler: " +
> se.toString();
> Debug.logError(se, errMsg, module);
> throw new ViewHandlerException(errMsg, se);
> } catch (ParserConfigurationException pe) {
> String errMsg = "Error parser rendering " + page + " view handler:
> " + pe.toString();
> Debug.logError(pe, errMsg, module);
> throw new ViewHandlerException(errMsg, pe);
> }
>
>
> } catch (GeneralException ge) {
> String errMsg = "Error rendering " + birtContentType + " attachment
> for email: " + ge.toString();
> Debug.logError(ge, errMsg, module);
> return ServiceUtil.returnError(errMsg);
> } catch (IOException ie) {
> String errMsg = "Error I/O rendering " + birtContentType + "
> attachment for email: " + ie.toString();
> Debug.logError(ie, errMsg, module);
> return ServiceUtil.returnError(errMsg);
> } catch (FOPException fe) {
> String errMsg = "Error FOP rendering " + birtContentType + "
> attachment for email: " + fe.toString();
> Debug.logError(fe, errMsg, module);
> return ServiceUtil.returnError(errMsg);
> } catch (SAXException se) {
> String errMsg = "Error SAX rendering " + birtContentType + "
> attachment for email: " + se.toString();
> Debug.logError(se, errMsg, module);
> return ServiceUtil.returnError(errMsg);
> } catch (ParserConfigurationException pe) {
> String errMsg = "Error parser rendering " + birtContentType + "
> attachment for email: " + pe.toString();
> Debug.logError(pe, errMsg, module);
> return ServiceUtil.returnError(errMsg);
> } catch (EngineException ee) {
> String errMsg = "Error rendering " + birtContentType + " attachment
> for email: " + ee.toString();
> Debug.logError(ee, errMsg, module);
> return ServiceUtil.returnError(errMsg);
> } catch (SQLException se) {
> String errMsg = "Error SQL rendering " + birtContentType + "
> attachment for email: " + se.toString();
> Debug.logError(se, errMsg, module);
> return ServiceUtil.returnError(errMsg);
> }
>
> I think the
> exception.toString()
> Debug.logError(exception, errMsg, module);
> information is enough. This for 4 reasons:
>
> 1. exception.toString(), and exception in Debug.logError give enough
> information. No needs to specify things like
> "Error I/O rendering "
> "Error FOP rendering"
> etc.
> this will anyway be given by the thrown exception
> 2. It's more legible
> 3. It's an antipattern people C/P (like I did) so it spreads
> everywhere in code
> 4. Those kinds of exception are exceptionally thrown. BTW I personaly
> believe checked exceptions in Java is not really the best part
> of the language. As soons as it has spread in library and at large
> code, you have to suffer all those try/catch blocks
>
> So this like below, this can be rewritten
>
> } catch (Exception e) {
> String errMsg = "Error rendering " + birtContentType + " attachment
> for email: " + e.toString();
> Debug.logError(e, errMsg, module);
> return ServiceUtil.returnError(errMsg);
> }
>
The "+ e.toString()" is not needed because Debug will print the
Exception's message anyway. I usually delete those because they make the
log harder to read.
-Adrian
Re: svn commit: r1334497 - in /ofbiz/trunk: applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/ applications/product/src/org/ofbiz/shipment/thirdparty/fedex/ applications/product/src/org/ofbiz/shipment/thirdparty/usps/ specialpurpose
Posted by Jacques Le Roux <ja...@les7arts.com>.
Nobody is speaking about swallowing, I hope we have not such things in OFBiz! (though Scott's wait to read the end before thinking I
did not get you and being upset ;o)
No, it's more about stuff like
} catch (BirtException e) {
throw new ViewHandlerException("Birt Error create engine: " + e.toString(), e);
} catch (IOException e) {
throw new ViewHandlerException("Error in the response writer/output stream: " + e.toString(), e);
} catch (SQLException e) {
throw new ViewHandlerException("get connection error: " + e.toString(), e);
} catch (GenericEntityException e) {
throw new ViewHandlerException("generic entity error: " + e.toString(), e);
} catch (GeneralException e) {
throw new ViewHandlerException("general error: " + e.toString(), e);
} catch (SAXException se) {
String errMsg = "Error SAX rendering " + page + " view handler: " + se.toString();
Debug.logError(se, errMsg, module);
throw new ViewHandlerException(errMsg, se);
} catch (ParserConfigurationException pe) {
String errMsg = "Error parser rendering " + page + " view handler: " + pe.toString();
Debug.logError(pe, errMsg, module);
throw new ViewHandlerException(errMsg, pe);
}
} catch (GeneralException ge) {
String errMsg = "Error rendering " + birtContentType + " attachment for email: " + ge.toString();
Debug.logError(ge, errMsg, module);
return ServiceUtil.returnError(errMsg);
} catch (IOException ie) {
String errMsg = "Error I/O rendering " + birtContentType + " attachment for email: " + ie.toString();
Debug.logError(ie, errMsg, module);
return ServiceUtil.returnError(errMsg);
} catch (FOPException fe) {
String errMsg = "Error FOP rendering " + birtContentType + " attachment for email: " + fe.toString();
Debug.logError(fe, errMsg, module);
return ServiceUtil.returnError(errMsg);
} catch (SAXException se) {
String errMsg = "Error SAX rendering " + birtContentType + " attachment for email: " + se.toString();
Debug.logError(se, errMsg, module);
return ServiceUtil.returnError(errMsg);
} catch (ParserConfigurationException pe) {
String errMsg = "Error parser rendering " + birtContentType + " attachment for email: " + pe.toString();
Debug.logError(pe, errMsg, module);
return ServiceUtil.returnError(errMsg);
} catch (EngineException ee) {
String errMsg = "Error rendering " + birtContentType + " attachment for email: " + ee.toString();
Debug.logError(ee, errMsg, module);
return ServiceUtil.returnError(errMsg);
} catch (SQLException se) {
String errMsg = "Error SQL rendering " + birtContentType + " attachment for email: " + se.toString();
Debug.logError(se, errMsg, module);
return ServiceUtil.returnError(errMsg);
}
I think the
exception.toString()
Debug.logError(exception, errMsg, module);
information is enough. This for 4 reasons:
1. exception.toString(), and exception in Debug.logError give enough information. No needs to specify things like
"Error I/O rendering "
"Error FOP rendering"
etc.
this will anyway be given by the thrown exception
2. It's more legible
3. It's an antipattern people C/P (like I did) so it spreads everywhere in code
4. Those kinds of exception are exceptionally thrown. BTW I personaly believe checked exceptions in Java is not really the best part
of the language. As soons as it has spread in library and at large code, you have to suffer all those try/catch blocks
So this like below, this can be rewritten
} catch (Exception e) {
String errMsg = "Error rendering " + birtContentType + " attachment for email: " + e.toString();
Debug.logError(e, errMsg, module);
return ServiceUtil.returnError(errMsg);
}
Of course I don't mean this as a general common pattern. It only apply to cases where the "same" thing (of course with specific
information proper to the exception thrown) is finally rendered when the exception is thrown (see more arguments at bottom)
Quoting http://docs.oracle.com/javase/tutorial/essential/exceptions/advantages.html (***are mine to underline***)
<<You could even set up an exception handler that handles any Exception with the handler here.
// A (too) general exception handler
catch (Exception e) {
...
}
The Exception class is close to the top of the Throwable class hierarchy. Therefore, this handler will catch many other exceptions
in addition to those that the handler is intended to catch. ***You may want to handle exceptions this way if all you want your
program to do, for example, is print out an error message for the user and then exit.***
In most situations, however, you want exception handlers to be as specific as possible. The reason is that the first thing a handler
must do is determine what type of exception occurred before it can decide on the best recovery strategy. In effect, by not catching
specific errors, the handler must accommodate any possibility. Exception handlers that are too general can make code more
error-prone by catching and handling exceptions that weren't anticipated by the programmer and for which the handler was not
intended.>>
To be back to Scott's objection (I believe he referred to same Objection than expressed by Oracle, which makes sense) I believe
anyway that in cases above (from OFBiz) we will give enough information if even an unanticipated exception is thrown. So we could
adopt this pattern where the anti-pattern above is used (DRY principle). In our case, in <<print out an error message for the user
and then exit>> the user is actually the developer reading the stack, we are not specifically handling the type of exception by the
calling method.
If we agree, we could not only recommend this pattern but also clean current code, as I already begun.
Opinions?
Jacques
From: "Scott Gray" <sc...@hotwaxmedia.com>
>I think it depends on what is being done with the exceptions, if you're swallowing them then I don't think it would be such a good
>idea.
>
> Regards
> Scott
>
> On 6/05/2012, at 8:55 AM, Adrian Crum <ad...@sandglass-software.com> wrote:
>
>> From my perspective, a bunch of catch blocks that all do the exact same thing can be consolidated into a single catch block.
>>
>> -Adrian
>>
>> On 5/5/2012 9:46 PM, jleroux@apache.org wrote:
>>> Author: jleroux
>>> Date: Sat May 5 20:46:39 2012
>>> New Revision: 1334497
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1334497&view=rev
>>> Log:
>>> Non functional changes.
>>> Removes a bunch of useless exceptions catches. There are still few cases to discuss, I believe there are useless (or at least
>>> without much more information) as well. we will decide on dev ML later...
>>>
>>> Modified:
>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java
>>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java
>>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java
>>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java
>>> ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java
>>>
>>> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java (original)
>>> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java Sat May 5
>>> 20:46:39 2012
>>> @@ -19,7 +19,6 @@
>>> package org.ofbiz.accounting.thirdparty.clearcommerce;
>>>
>>> import java.io.ByteArrayOutputStream;
>>> -import java.io.IOException;
>>> import java.io.OutputStream;
>>> import java.math.BigDecimal;
>>> import java.util.ArrayList;
>>> @@ -28,7 +27,6 @@ import java.util.List;
>>> import java.util.Locale;
>>> import java.util.Map;
>>>
>>> -import javax.xml.parsers.ParserConfigurationException;
>>> import javax.xml.transform.TransformerException;
>>>
>>> import org.ofbiz.accounting.payment.PaymentGatewayServices;
>>> @@ -47,7 +45,6 @@ import org.ofbiz.service.DispatchContext
>>> import org.ofbiz.service.ServiceUtil;
>>> import org.w3c.dom.Document;
>>> import org.w3c.dom.Element;
>>> -import org.xml.sax.SAXException;
>>>
>>>
>>> /**
>>> @@ -921,12 +918,8 @@ public class CCPaymentServices {
>>> Document responseDocument = null;
>>> try {
>>> responseDocument = UtilXml.readXmlDocument(response, false);
>>> - } catch (SAXException se) {
>>> - throw new ClearCommerceException("Error reading response Document from a String: " + se.getMessage());
>>> - } catch (ParserConfigurationException pce) {
>>> - throw new ClearCommerceException("Error reading response Document from a String: " + pce.getMessage());
>>> - } catch (IOException ioe) {
>>> - throw new ClearCommerceException("Error reading response Document from a String: " + ioe.getMessage());
>>> + } catch (Exception e) {
>>> + throw new ClearCommerceException("Error reading response Document from a String: " + e.getMessage());
>>> }
>>> if (Debug.verboseOn()) Debug.logVerbose("Result severity from clearCommerce:" + getMessageListMaxSev(responseDocument),
>>> module);
>>> if (Debug.verboseOn()&& getMessageListMaxSev(responseDocument)> 4)
>>>
>>> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java (original)
>>> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java Sat May 5 20:46:39 2012
>>> @@ -19,7 +19,6 @@
>>>
>>> package org.ofbiz.shipment.thirdparty.fedex;
>>>
>>> -import java.io.IOException;
>>> import java.io.StringWriter;
>>> import java.math.BigDecimal;
>>> import java.sql.Timestamp;
>>> @@ -27,8 +26,6 @@ import java.util.List;
>>> import java.util.Locale;
>>> import java.util.Map;
>>>
>>> -import javax.xml.parsers.ParserConfigurationException;
>>> -
>>> import javolution.util.FastList;
>>> import javolution.util.FastMap;
>>>
>>> @@ -58,7 +55,6 @@ import org.ofbiz.service.ServiceUtil;
>>> import org.ofbiz.shipment.shipment.ShipmentServices;
>>> import org.w3c.dom.Document;
>>> import org.w3c.dom.Element;
>>> -import org.xml.sax.SAXException;
>>>
>>> /**
>>> * Fedex Shipment Services
>>> @@ -364,24 +360,12 @@ public class FedexServices {
>>> try {
>>> fDXSubscriptionReplyDocument = UtilXml.readXmlDocument(fDXSubscriptionReplyString, false);
>>> Debug.logInfo("Fedex response for FDXSubscriptionRequest:" + fDXSubscriptionReplyString, module);
>>> - } catch (SAXException se) {
>>> - String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + se.toString();
>>> - Debug.logError(se, errorMessage, module);
>>> - return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>> - "FacilityShipmentFedexSubscriptionTemplateParsingError",
>>> - UtilMisc.toMap("errorString", se.toString()), locale));
>>> - } catch (ParserConfigurationException pce) {
>>> - String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + pce.toString();
>>> - Debug.logError(pce, errorMessage, module);
>>> - return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>> - "FacilityShipmentFedexSubscriptionTemplateParsingError",
>>> - UtilMisc.toMap("errorString", pce.toString()), locale));
>>> - } catch (IOException ioe) {
>>> - String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + ioe.toString();
>>> - Debug.logError(ioe, errorMessage, module);
>>> + } catch (Exception e) {
>>> + String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + e.toString();
>>> + Debug.logError(e, errorMessage, module);
>>> return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>> "FacilityShipmentFedexSubscriptionTemplateParsingError",
>>> - UtilMisc.toMap("errorString", ioe.toString()), locale));
>>> + UtilMisc.toMap("errorString", e.toString()), locale));
>>> }
>>>
>>> Element fedexSubscriptionReplyElement = fDXSubscriptionReplyDocument.getDocumentElement();
>>> @@ -996,17 +980,9 @@ public class FedexServices {
>>> Document fdxShipReplyDocument = null;
>>> try {
>>> fdxShipReplyDocument = UtilXml.readXmlDocument(fDXShipReplyString, false);
>>> - } catch (SAXException se) {
>>> - String errorMessage = "Error parsing the FDXShipReply: " + se.toString();
>>> - Debug.logError(se, errorMessage, module);
>>> - // TODO: Cancel the package
>>> - } catch (ParserConfigurationException pe) {
>>> - String errorMessage = "Error parsing the FDXShipReply: " + pe.toString();
>>> - Debug.logError(pe, errorMessage, module);
>>> - // TODO Cancel the package
>>> - } catch (IOException ioe) {
>>> - String errorMessage = "Error parsing the FDXShipReply: " + ioe.toString();
>>> - Debug.logError(ioe, errorMessage, module);
>>> + } catch (Exception e) {
>>> + String errorMessage = "Error parsing the FDXShipReply: " + e.toString();
>>> + Debug.logError(e, errorMessage, module);
>>> // TODO Cancel the package
>>> }
>>>
>>>
>>> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java?rev=1334497&r1=1334496&r2=1334497&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java (original)
>>> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java Sat May 5 20:46:39 2012
>>> @@ -30,16 +30,13 @@ import javax.servlet.ServletOutputStream
>>> import javax.servlet.http.HttpServlet;
>>> import javax.servlet.http.HttpServletRequest;
>>> import javax.servlet.http.HttpServletResponse;
>>> -import javax.xml.parsers.ParserConfigurationException;
>>> import javax.xml.transform.TransformerException;
>>>
>>> +import org.ofbiz.base.util.Debug;
>>> import org.ofbiz.base.util.UtilValidate;
>>> import org.ofbiz.base.util.UtilXml;
>>> -import org.ofbiz.base.util.Debug;
>>> -
>>> import org.w3c.dom.Document;
>>> import org.w3c.dom.Element;
>>> -import org.xml.sax.SAXException;
>>>
>>> /**
>>> * USPS Webtools API Mock API Servlet
>>> @@ -77,14 +74,8 @@ public class UspsMockApiServlet extends
>>> Document requestDocument = null;
>>> try {
>>> requestDocument = UtilXml.readXmlDocument(xmlValue, false);
>>> - } catch (SAXException se) {
>>> - Debug.logError(se, module);
>>> - return;
>>> - } catch (ParserConfigurationException pce) {
>>> - Debug.logError(pce, module);
>>> - return;
>>> - } catch (IOException xmlReadException) {
>>> - Debug.logError(xmlReadException, module);
>>> + } catch (Exception e) {
>>> + Debug.logError(e, module);
>>> return;
>>> }
>>>
>>>
>>> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java (original)
>>> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java Sat May 5 20:46:39 2012
>>> @@ -32,7 +32,6 @@ import java.util.ListIterator;
>>> import java.util.Locale;
>>> import java.util.Map;
>>>
>>> -import javax.xml.parsers.ParserConfigurationException;
>>> import javax.xml.transform.TransformerException;
>>>
>>> import javolution.util.FastList;
>>> @@ -67,7 +66,6 @@ import org.ofbiz.shipment.shipment.Shipm
>>> import org.ofbiz.shipment.shipment.ShipmentWorker;
>>> import org.w3c.dom.Document;
>>> import org.w3c.dom.Element;
>>> -import org.xml.sax.SAXException;
>>>
>>> /**
>>> * USPS Webtools API Services
>>> @@ -1757,18 +1755,8 @@ public class UspsServices {
>>> Document responseDocument = null;
>>> try {
>>> responseDocument = UtilXml.readXmlDocument(responseString, false);
>>> - } catch (SAXException se) {
>>> - throw new UspsRequestException(UtilProperties.getMessage(resourceError,
>>> - "FacilityShipmentUspsResponseError",
>>> - UtilMisc.toMap("errorString", se.getMessage()), locale));
>>> - } catch (ParserConfigurationException pce) {
>>> - throw new UspsRequestException(UtilProperties.getMessage(resourceError,
>>> - "FacilityShipmentUspsResponseError",
>>> - UtilMisc.toMap("errorString", pce.getMessage()), locale));
>>> - } catch (IOException xmlReadException) {
>>> - throw new UspsRequestException(UtilProperties.getMessage(resourceError,
>>> - "FacilityShipmentUspsResponseError",
>>> - UtilMisc.toMap("errorString", xmlReadException.getMessage()), locale));
>>> + } catch (Exception e) {
>>> + throw new UspsRequestException(UtilProperties.getMessage(resourceError, "FacilityShipmentUspsResponseError",
>>> UtilMisc.toMap("errorString", e.getMessage()), locale));
>>> }
>>>
>>> // If a top-level error document is returned, throw exception
>>>
>>> Modified: ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java (original)
>>> +++ ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java Sat May 5 20:46:39 2012
>>> @@ -25,8 +25,6 @@ import java.util.HashMap;
>>> import java.util.List;
>>> import java.util.Map;
>>>
>>> -import javax.xml.parsers.ParserConfigurationException;
>>> -
>>> import org.ofbiz.base.util.Debug;
>>> import org.ofbiz.base.util.GeneralException;
>>> import org.ofbiz.base.util.ObjectType;
>>> @@ -41,7 +39,6 @@ import org.ofbiz.entity.serialize.XmlSer
>>> import org.ofbiz.service.DispatchContext;
>>> import org.ofbiz.service.GenericServiceException;
>>> import org.ofbiz.service.ModelService;
>>> -import org.xml.sax.SAXException;
>>>
>>> /**
>>> * Workflow Application Services - 'Services' and 'Workers' for interaction with Workflow Application API
>>> @@ -219,14 +216,8 @@ public class WfApplicationServices {
>>> throws GenericServiceException {
>>> try {
>>> return UtilGenerics.checkMap(XmlSerializer.deserialize((String) runTimeData.get("runtimeInfo"), delegator));
>>> - } catch (SerializeException se) {
>>> - throw new GenericServiceException(se.getMessage(), se);
>>> - } catch (ParserConfigurationException pe) {
>>> - throw new GenericServiceException(pe.getMessage(), pe);
>>> - } catch (SAXException se) {
>>> - throw new GenericServiceException(se.getMessage(), se);
>>> - } catch (IOException ioe) {
>>> - throw new GenericServiceException(ioe.getMessage(), ioe);
>>> + } catch (Exception e) {
>>> + throw new GenericServiceException(e.getMessage(), e);
>>> }
>>> }
>>>
>>>
>>>
>
Re: svn commit: r1334497 - in /ofbiz/trunk: applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/ applications/product/src/org/ofbiz/shipment/thirdparty/fedex/ applications/product/src/org/ofbiz/shipment/thirdparty/usps/ specialpurpose...
Posted by Scott Gray <sc...@hotwaxmedia.com>.
I think it depends on what is being done with the exceptions, if you're swallowing them then I don't think it would be such a good idea.
Regards
Scott
On 6/05/2012, at 8:55 AM, Adrian Crum <ad...@sandglass-software.com> wrote:
> From my perspective, a bunch of catch blocks that all do the exact same thing can be consolidated into a single catch block.
>
> -Adrian
>
> On 5/5/2012 9:46 PM, jleroux@apache.org wrote:
>> Author: jleroux
>> Date: Sat May 5 20:46:39 2012
>> New Revision: 1334497
>>
>> URL: http://svn.apache.org/viewvc?rev=1334497&view=rev
>> Log:
>> Non functional changes.
>> Removes a bunch of useless exceptions catches. There are still few cases to discuss, I believe there are useless (or at least without much more information) as well. we will decide on dev ML later...
>>
>> Modified:
>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java
>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java
>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java
>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java
>> ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java
>>
>> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java (original)
>> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java Sat May 5 20:46:39 2012
>> @@ -19,7 +19,6 @@
>> package org.ofbiz.accounting.thirdparty.clearcommerce;
>>
>> import java.io.ByteArrayOutputStream;
>> -import java.io.IOException;
>> import java.io.OutputStream;
>> import java.math.BigDecimal;
>> import java.util.ArrayList;
>> @@ -28,7 +27,6 @@ import java.util.List;
>> import java.util.Locale;
>> import java.util.Map;
>>
>> -import javax.xml.parsers.ParserConfigurationException;
>> import javax.xml.transform.TransformerException;
>>
>> import org.ofbiz.accounting.payment.PaymentGatewayServices;
>> @@ -47,7 +45,6 @@ import org.ofbiz.service.DispatchContext
>> import org.ofbiz.service.ServiceUtil;
>> import org.w3c.dom.Document;
>> import org.w3c.dom.Element;
>> -import org.xml.sax.SAXException;
>>
>>
>> /**
>> @@ -921,12 +918,8 @@ public class CCPaymentServices {
>> Document responseDocument = null;
>> try {
>> responseDocument = UtilXml.readXmlDocument(response, false);
>> - } catch (SAXException se) {
>> - throw new ClearCommerceException("Error reading response Document from a String: " + se.getMessage());
>> - } catch (ParserConfigurationException pce) {
>> - throw new ClearCommerceException("Error reading response Document from a String: " + pce.getMessage());
>> - } catch (IOException ioe) {
>> - throw new ClearCommerceException("Error reading response Document from a String: " + ioe.getMessage());
>> + } catch (Exception e) {
>> + throw new ClearCommerceException("Error reading response Document from a String: " + e.getMessage());
>> }
>> if (Debug.verboseOn()) Debug.logVerbose("Result severity from clearCommerce:" + getMessageListMaxSev(responseDocument), module);
>> if (Debug.verboseOn()&& getMessageListMaxSev(responseDocument)> 4)
>>
>> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java (original)
>> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java Sat May 5 20:46:39 2012
>> @@ -19,7 +19,6 @@
>>
>> package org.ofbiz.shipment.thirdparty.fedex;
>>
>> -import java.io.IOException;
>> import java.io.StringWriter;
>> import java.math.BigDecimal;
>> import java.sql.Timestamp;
>> @@ -27,8 +26,6 @@ import java.util.List;
>> import java.util.Locale;
>> import java.util.Map;
>>
>> -import javax.xml.parsers.ParserConfigurationException;
>> -
>> import javolution.util.FastList;
>> import javolution.util.FastMap;
>>
>> @@ -58,7 +55,6 @@ import org.ofbiz.service.ServiceUtil;
>> import org.ofbiz.shipment.shipment.ShipmentServices;
>> import org.w3c.dom.Document;
>> import org.w3c.dom.Element;
>> -import org.xml.sax.SAXException;
>>
>> /**
>> * Fedex Shipment Services
>> @@ -364,24 +360,12 @@ public class FedexServices {
>> try {
>> fDXSubscriptionReplyDocument = UtilXml.readXmlDocument(fDXSubscriptionReplyString, false);
>> Debug.logInfo("Fedex response for FDXSubscriptionRequest:" + fDXSubscriptionReplyString, module);
>> - } catch (SAXException se) {
>> - String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + se.toString();
>> - Debug.logError(se, errorMessage, module);
>> - return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>> - "FacilityShipmentFedexSubscriptionTemplateParsingError",
>> - UtilMisc.toMap("errorString", se.toString()), locale));
>> - } catch (ParserConfigurationException pce) {
>> - String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + pce.toString();
>> - Debug.logError(pce, errorMessage, module);
>> - return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>> - "FacilityShipmentFedexSubscriptionTemplateParsingError",
>> - UtilMisc.toMap("errorString", pce.toString()), locale));
>> - } catch (IOException ioe) {
>> - String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + ioe.toString();
>> - Debug.logError(ioe, errorMessage, module);
>> + } catch (Exception e) {
>> + String errorMessage = "Error parsing the FDXSubscriptionRequest response: " + e.toString();
>> + Debug.logError(e, errorMessage, module);
>> return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>> "FacilityShipmentFedexSubscriptionTemplateParsingError",
>> - UtilMisc.toMap("errorString", ioe.toString()), locale));
>> + UtilMisc.toMap("errorString", e.toString()), locale));
>> }
>>
>> Element fedexSubscriptionReplyElement = fDXSubscriptionReplyDocument.getDocumentElement();
>> @@ -996,17 +980,9 @@ public class FedexServices {
>> Document fdxShipReplyDocument = null;
>> try {
>> fdxShipReplyDocument = UtilXml.readXmlDocument(fDXShipReplyString, false);
>> - } catch (SAXException se) {
>> - String errorMessage = "Error parsing the FDXShipReply: " + se.toString();
>> - Debug.logError(se, errorMessage, module);
>> - // TODO: Cancel the package
>> - } catch (ParserConfigurationException pe) {
>> - String errorMessage = "Error parsing the FDXShipReply: " + pe.toString();
>> - Debug.logError(pe, errorMessage, module);
>> - // TODO Cancel the package
>> - } catch (IOException ioe) {
>> - String errorMessage = "Error parsing the FDXShipReply: " + ioe.toString();
>> - Debug.logError(ioe, errorMessage, module);
>> + } catch (Exception e) {
>> + String errorMessage = "Error parsing the FDXShipReply: " + e.toString();
>> + Debug.logError(e, errorMessage, module);
>> // TODO Cancel the package
>> }
>>
>>
>> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java?rev=1334497&r1=1334496&r2=1334497&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java (original)
>> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java Sat May 5 20:46:39 2012
>> @@ -30,16 +30,13 @@ import javax.servlet.ServletOutputStream
>> import javax.servlet.http.HttpServlet;
>> import javax.servlet.http.HttpServletRequest;
>> import javax.servlet.http.HttpServletResponse;
>> -import javax.xml.parsers.ParserConfigurationException;
>> import javax.xml.transform.TransformerException;
>>
>> +import org.ofbiz.base.util.Debug;
>> import org.ofbiz.base.util.UtilValidate;
>> import org.ofbiz.base.util.UtilXml;
>> -import org.ofbiz.base.util.Debug;
>> -
>> import org.w3c.dom.Document;
>> import org.w3c.dom.Element;
>> -import org.xml.sax.SAXException;
>>
>> /**
>> * USPS Webtools API Mock API Servlet
>> @@ -77,14 +74,8 @@ public class UspsMockApiServlet extends
>> Document requestDocument = null;
>> try {
>> requestDocument = UtilXml.readXmlDocument(xmlValue, false);
>> - } catch (SAXException se) {
>> - Debug.logError(se, module);
>> - return;
>> - } catch (ParserConfigurationException pce) {
>> - Debug.logError(pce, module);
>> - return;
>> - } catch (IOException xmlReadException) {
>> - Debug.logError(xmlReadException, module);
>> + } catch (Exception e) {
>> + Debug.logError(e, module);
>> return;
>> }
>>
>>
>> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java (original)
>> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java Sat May 5 20:46:39 2012
>> @@ -32,7 +32,6 @@ import java.util.ListIterator;
>> import java.util.Locale;
>> import java.util.Map;
>>
>> -import javax.xml.parsers.ParserConfigurationException;
>> import javax.xml.transform.TransformerException;
>>
>> import javolution.util.FastList;
>> @@ -67,7 +66,6 @@ import org.ofbiz.shipment.shipment.Shipm
>> import org.ofbiz.shipment.shipment.ShipmentWorker;
>> import org.w3c.dom.Document;
>> import org.w3c.dom.Element;
>> -import org.xml.sax.SAXException;
>>
>> /**
>> * USPS Webtools API Services
>> @@ -1757,18 +1755,8 @@ public class UspsServices {
>> Document responseDocument = null;
>> try {
>> responseDocument = UtilXml.readXmlDocument(responseString, false);
>> - } catch (SAXException se) {
>> - throw new UspsRequestException(UtilProperties.getMessage(resourceError,
>> - "FacilityShipmentUspsResponseError",
>> - UtilMisc.toMap("errorString", se.getMessage()), locale));
>> - } catch (ParserConfigurationException pce) {
>> - throw new UspsRequestException(UtilProperties.getMessage(resourceError,
>> - "FacilityShipmentUspsResponseError",
>> - UtilMisc.toMap("errorString", pce.getMessage()), locale));
>> - } catch (IOException xmlReadException) {
>> - throw new UspsRequestException(UtilProperties.getMessage(resourceError,
>> - "FacilityShipmentUspsResponseError",
>> - UtilMisc.toMap("errorString", xmlReadException.getMessage()), locale));
>> + } catch (Exception e) {
>> + throw new UspsRequestException(UtilProperties.getMessage(resourceError, "FacilityShipmentUspsResponseError", UtilMisc.toMap("errorString", e.getMessage()), locale));
>> }
>>
>> // If a top-level error document is returned, throw exception
>>
>> Modified: ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java?rev=1334497&r1=1334496&r2=1334497&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java (original)
>> +++ ofbiz/trunk/specialpurpose/workflow/src/org/ofbiz/workflow/WfApplicationServices.java Sat May 5 20:46:39 2012
>> @@ -25,8 +25,6 @@ import java.util.HashMap;
>> import java.util.List;
>> import java.util.Map;
>>
>> -import javax.xml.parsers.ParserConfigurationException;
>> -
>> import org.ofbiz.base.util.Debug;
>> import org.ofbiz.base.util.GeneralException;
>> import org.ofbiz.base.util.ObjectType;
>> @@ -41,7 +39,6 @@ import org.ofbiz.entity.serialize.XmlSer
>> import org.ofbiz.service.DispatchContext;
>> import org.ofbiz.service.GenericServiceException;
>> import org.ofbiz.service.ModelService;
>> -import org.xml.sax.SAXException;
>>
>> /**
>> * Workflow Application Services - 'Services' and 'Workers' for interaction with Workflow Application API
>> @@ -219,14 +216,8 @@ public class WfApplicationServices {
>> throws GenericServiceException {
>> try {
>> return UtilGenerics.checkMap(XmlSerializer.deserialize((String) runTimeData.get("runtimeInfo"), delegator));
>> - } catch (SerializeException se) {
>> - throw new GenericServiceException(se.getMessage(), se);
>> - } catch (ParserConfigurationException pe) {
>> - throw new GenericServiceException(pe.getMessage(), pe);
>> - } catch (SAXException se) {
>> - throw new GenericServiceException(se.getMessage(), se);
>> - } catch (IOException ioe) {
>> - throw new GenericServiceException(ioe.getMessage(), ioe);
>> + } catch (Exception e) {
>> + throw new GenericServiceException(e.getMessage(), e);
>> }
>> }
>>
>>
>>