svn commit: r1734270 - in /ofbiz/branches/release15.12: ./ framework/service/src/org/ofbiz/service/engine/GroovyEngine.java

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1734270 - in /ofbiz/branches/release15.12: ./ framework/service/src/org/ofbiz/service/engine/GroovyEngine.java

jleroux@apache.org
Author: jleroux
Date: Wed Mar  9 15:37:26 2016
New Revision: 1734270

URL: http://svn.apache.org/viewvc?rev=1734270&view=rev
Log:
"Applied fix from trunk for revision: 1734269"
------------------------------------------------------------------------
r1734269 | jleroux | 2016-03-09 16:36:56 +0100 (mer. 09 mars 2016) | 7 lignes

A patch from Forrest Rae for "GroovyEngine.serviceInvoker masks Groovy script exceptions in some cases" https://issues.apache.org/jira/browse/OFBIZ-6888

If GroovyEngine.serviceInvoker catches an exception in a Groovy script, it would mask the exception in some cases.  An exception's detailMessage can be null.  If it is null, the exception won't be properly returned and logged, and that will make spotting problems very difficult.  This was the case for me while trying to track down a problem with a java.util.ConcurrentModificationException error in a Groovy script.  I suspect that this choice to mask exceptions and only return the message has cause bugs to not be spotted.

Disabling this for now in favor of returning a proper exception.  GroovyEngine.serviceInvoker() should throw GenericServiceException if error, rather than returning ServiceUtil.returnError(e.getMessage())


------------------------------------------------------------------------


Modified:
    ofbiz/branches/release15.12/   (props changed)
    ofbiz/branches/release15.12/framework/service/src/org/ofbiz/service/engine/GroovyEngine.java

Propchange: ofbiz/branches/release15.12/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Mar  9 15:37:26 2016
@@ -9,4 +9,4 @@
 /ofbiz/branches/json-integration-refactoring:1634077-1635900
 /ofbiz/branches/multitenant20100310:921280-927264
 /ofbiz/branches/release13.07:1547657
-/ofbiz/trunk:1722712,1723007,1723248,1724402,1724411,1724566,1724689,1724763,1724916,1724918,1724925,1724930,1724940,1724943,1724946,1724951,1724957,1724975,1724978,1725006,1725217,1725257,1725561,1725574,1726388,1726486,1726493,1726828,1728398,1728411,1729005,1729078,1729609,1729809,1730035,1730456,1730735-1730736,1730747,1730758,1730882,1730889,1731382,1731396,1732454,1732570,1732721,1733951,1733956,1734246
+/ofbiz/trunk:1722712,1723007,1723248,1724402,1724411,1724566,1724689,1724763,1724916,1724918,1724925,1724930,1724940,1724943,1724946,1724951,1724957,1724975,1724978,1725006,1725217,1725257,1725561,1725574,1726388,1726486,1726493,1726828,1728398,1728411,1729005,1729078,1729609,1729809,1730035,1730456,1730735-1730736,1730747,1730758,1730882,1730889,1731382,1731396,1732454,1732570,1732721,1733951,1733956,1734246,1734269

Modified: ofbiz/branches/release15.12/framework/service/src/org/ofbiz/service/engine/GroovyEngine.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release15.12/framework/service/src/org/ofbiz/service/engine/GroovyEngine.java?rev=1734270&r1=1734269&r2=1734270&view=diff
==============================================================================
--- ofbiz/branches/release15.12/framework/service/src/org/ofbiz/service/engine/GroovyEngine.java (original)
+++ ofbiz/branches/release15.12/framework/service/src/org/ofbiz/service/engine/GroovyEngine.java Wed Mar  9 15:37:26 2016
@@ -121,7 +121,10 @@ public final class GroovyEngine extends
         } catch (GeneralException ge) {
             throw new GenericServiceException(ge);
         } catch (Exception e) {
-            return ServiceUtil.returnError(e.getMessage());
+            // detailMessage can be null.  If it is null, the exception won't be properly returned and logged, and that will
+            // make spotting problems very difficult.  Disabling this for now in favor of returning a proper exception.
+            // return ServiceUtil.returnError(e.getMessage());
+            throw new GenericServiceException("Error running Groovy method [" + modelService.invoke + "] in Groovy file [" + modelService.location + "]: ", e);
         }
     }
 }