Re: svn commit: r982273 - /ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java

[Adam Heath-2]

On 08/04/2010 09:53 AM, [hidden email] wrote:

> Author: jacopoc
> Date: Wed Aug  4 14:53:54 2010
> New Revision: 982273
>
> URL: http://svn.apache.org/viewvc?rev=982273&view=rev
> Log:
> Improved error handling: the java event was hiding the error returned by the service.
>
>
> Modified:
>      ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java
>
> Modified: ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java?rev=982273&r1=982272&r2=982273&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java (original)
> +++ ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java Wed Aug  4 14:53:54 2010
> @@ -37,6 +37,7 @@ import org.ofbiz.entity.GenericPK;
>   import org.ofbiz.entity.GenericValue;
>   import org.ofbiz.service.GenericServiceException;
>   import org.ofbiz.service.LocalDispatcher;
> +import org.ofbiz.service.ServiceUtil;
>
>   public class ProductionRunEvents {
>
> @@ -88,6 +89,10 @@ public class ProductionRunEvents {
>               inputMap.put("lotId", parameters.get("lotId"));
>               inputMap.put("userLogin", userLogin);
>               Map result = dispatcher.runSync("productionRunDeclareAndProduce", inputMap);
> +            if (ServiceUtil.isError(result)) {
> +                request.setAttribute("_ERROR_MESSAGE_", ServiceUtil.getErrorMessage(result));
> +                return "error";
> +            }
>           } catch (GenericServiceException e) {
>               String errMsg = "Error issuing materials: " + e.toString();
>               Debug.logError(e, errMsg, module);

Not directly related to this change, but I see that this controller
event catches an exception, then sets an attribute on the request with
an error message.  Would it not be a nice feature to be able to set
the exception into the request, so that the controller can do
something nice for the client browser?


>
>

[Adam Heath-2]

On 08/04/2010 09:53 AM, [hidden email] wrote:

> Author: jacopoc
> Date: Wed Aug  4 14:53:54 2010
> New Revision: 982273
>
> URL: http://svn.apache.org/viewvc?rev=982273&view=rev
> Log:
> Improved error handling: the java event was hiding the error returned by the service.
>
>
> Modified:
>      ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java
>
> Modified: ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java?rev=982273&r1=982272&r2=982273&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java (original)
> +++ ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunEvents.java Wed Aug  4 14:53:54 2010
> @@ -37,6 +37,7 @@ import org.ofbiz.entity.GenericPK;
>   import org.ofbiz.entity.GenericValue;
>   import org.ofbiz.service.GenericServiceException;
>   import org.ofbiz.service.LocalDispatcher;
> +import org.ofbiz.service.ServiceUtil;
>
>   public class ProductionRunEvents {
>
> @@ -88,6 +89,10 @@ public class ProductionRunEvents {
>               inputMap.put("lotId", parameters.get("lotId"));
>               inputMap.put("userLogin", userLogin);
>               Map result = dispatcher.runSync("productionRunDeclareAndProduce", inputMap);
> +            if (ServiceUtil.isError(result)) {
> +                request.setAttribute("_ERROR_MESSAGE_", ServiceUtil.getErrorMessage(result));
> +                return "error";
> +            }

There's another anti-pattern in this file(unrelated to this commit).

==
Map fooMap = UtilMisc.toMap(key1, value1, key2, value2);
fooMap.put(key3, value3);
fooMap.put(key4, value4);
// do something with fooMap.
==

The anti-pattern here is the unconditional addition of extra keys.
Either don't use toMap at all, or put them all into the toMap method call.


>           } catch (GenericServiceException e) {
>               String errMsg = "Error issuing materials: " + e.toString();
>               Debug.logError(e, errMsg, module);
>
>