Re: svn commit: r1824294 - in /ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility: facility/CountFacilityInventoryByProduct.groovy facility/ViewFacilityInventoryByProduct.groovy shipment/ReceiveInventoryAgainstPurchaseOrder.groovy

[Jacques Le Roux]

Thanks for the review Michael,

Yes I was wondering about that, so put the second part in the commit log.
I did something alike but simpler at r1824157. There, only NumberFormatException was clearly involved. So even if the message is only in English it 
should be profitable and does not speak about the log.
Anyway, I agree all should be better translated. Actually I must say I did not think about it and simply followed the way it's mostly done currently 
for the 40 same cases (look for <<request.setAttribute("_ERROR_MESSAGE>>in Groovy files)
I have creates OFBIZ-10225 for that.

About the log, I agree it's not a good idea to ask an user to look into the 
log, I was lazy :)

Why did I do that? I could not let only the log. Because we speak of Groovy files which are used in the context of UI. For Groovy, Java or Minilang 
used in context of services, obviously the log and only the log fits. But it's not enough for UI. The user needs to know immediately that a problem 
occurred. I was lazy because, unlike of r1824157, fpor these case in CountFacilityInventoryByProduct and View FacilityInventoryByProduct it needs more 
care (is it only a NFE exception, which fields ar concerned, etc.)

My idea was that if ever a problem occurs in production the user would be able to look into the log (small config) or would ask for help. I agree it's 
not good :) I'll look at it again

BTW, in r1824434  I have used the same pattern than in r1824157 for 
NumberFormatException in ReceiveInventoryAgainstPurchaseOrder (to be l10n)

I need to check if r1824157 needs to be improved as well (return on 
NumberFormatException)...

Jacques


Le 15/02/2018 à 13:51, Michael Brohl a écrit :
Hi Jacques,

do you think that it is appropriate to present the user an error message to check the log? Normal users won't have access to the log in a productive 
environment I guess. The message should contain a qualified information what was wrong.

The message is also not translated by UILabels which should always be the case 
for frontend messages.

Regards,

Michael


Am 15.02.18 um 12:01 schrieb jler...@apache.org:
Author: jleroux
Date: Thu Feb 15 11:01:41 2018
New Revision: 1824294

URL: http://svn.apache.org/viewvc?rev=1824294&view=rev
Log:
Fixed: Fix Default or Empty Catch block in Java and Groovy files
(OFBIZ-8341)

Fixes few remaining swallowed Exception in Groovy files.

Not sure if always showing an error message in ui is a good idea.
But this should not happen too often and could help users to fix wrong passed
numbers...

Modified:
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/facility/CountFacilityInventoryByProduct.groovy
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/facility/ViewFacilityInventoryByProduct.groovy
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/shipment/ReceiveInventoryAgainstPurchaseOrder.groovy

Modified: 
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/facility/CountFacilityInventoryByProduct.groovy
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/facility/CountFacilityInventoryByProduct.groovy?rev=1824294&r1=1824293&r2=1824294&view=diff
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/facility/CountFacilityInventoryByProduct.groovy
 (original)
+++ 
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/facility/CountFacilityInventoryByProduct.groovy
 Thu Feb 15 11:01:41 2018
@@ -181,7 +181,8 @@ if (action) {
              checkTime = UtilDateTime.toTimestamp(cal.getTime())
              searchParameterString += "&monthsInPastLimit=" + 
monthsInPastLimitStr
          } catch (Exception e) {
-            // Ignore
+            Debug.logError(e, "Caught an exception : " + e.toString(), module)
+            request.setAttribute("_ERROR_MESSAGE", "An exception occured please 
check the log")
          }
      }
  @@ -278,7 +279,8 @@ if (action) {
                          try {
                              salesUsageQuantity += 
salesUsageItem.getDouble("quantity").doubleValue()
                          } catch (Exception e) {
-                            // Ignore
+                            Debug.logError(e, "Caught an exception : " + 
e.toString(), module)
+ request.setAttribute("_ERROR_MESSAGE", "An exception occured please check the 
log")
                          }
                      }
                  }
@@ -299,7 +301,8 @@ if (action) {
                          try {
                              productionUsageQuantity += 
productionUsageItem.getDouble("quantity").doubleValue()
                          } catch (Exception e) {
-                            // Ignore
+                            Debug.logError(e, "Caught an exception : " + 
e.toString(), module)
+ request.setAttribute("_ERROR_MESSAGE", "An exception occured please check the 
log")
                          }
                      }
                  }

Modified: 
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/facility/ViewFacilityInventoryByProduct.groovy
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/facility/ViewFacilityInventoryByProduct.groovy?rev=1824294&r1=1824293&r2=1824294&view=diff
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/facility/ViewFacilityInventoryByProduct.groovy
 (original)
+++ 
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/facility/ViewFacilityInventoryByProduct.groovy
 Thu Feb 15 11:01:41 2018
@@ -174,7 +174,8 @@ if (action) {
              checkTime = UtilDateTime.toTimestamp(cal.getTime())
              searchParameterString += "&monthsInPastLimit=" + 
monthsInPastLimitStr
          } catch (Exception e) {
-            // Ignore
+            Debug.logError(e, "Caught an exception : " + e.toString(), module)
+            request.setAttribute("_ERROR_MESSAGE", "An exception occured please 
check the log")
          }
      }

Modified: 
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/shipment/ReceiveInventoryAgainstPurchaseOrder.groovy
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/shipment/ReceiveInventoryAgainstPurchaseOrder.groovy?rev=1824294&r1=1824293&r2=1824294&view=diff
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/shipment/ReceiveInventoryAgainstPurchaseOrder.groovy
 (original)
+++ ofbiz/ofbiz-framework/trunk/applications/product/groovyScripts/facility/shipment/ReceiveInventoryAgainstPurchaseOrder.groovy Thu Feb 15 
11:01:41 2018
@@ -216,8 +216,10 @@ if (productIdToReceive) {
          if (productQtyToReceive) {
              try {
                  quantity = Double.parseDouble(productQtyToReceive)
-            } catch (Exception e) {
+            } catch (NumberFormatException nfe) {
                  // Ignore the quantity update if there's a problem parsing it
+                Debug.logError(nfe, "Caught an exception : " + nfe.toString(), 
module)
+                request.setAttribute("_ERROR_MESSAGE", "An exception occured please 
check the log")
              }
          }