[ 
https://issues.apache.org/jira/browse/JEXL-257?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16659070#comment-16659070
 ] 

Henri Biestro edited comment on JEXL-257 at 10/22/18 3:16 PM:
--------------------------------------------------------------

Notes and implications of the proposed tryInvoke behaviour change. 
The proposed modifications breaks the API contract by suddenly potentially 
throwing an exception, tryInvoke working on the premise that a previously 
resolved method can be called again successfully with the same type of 
arguments. Most if not all executors would be impacted by the change (not only 
MethodExecutor but all *Executor).
Reviewing the code, there does not seem to be a case where the code caches a 
method that has (hard) failed. 
The issue described occurs when the 1st invocation is successful but the 2nd 
(tryInvoke) and 3rd (invoke) fail... If we want to avoid the 3rd (failing) 
invocation, we need the 2nd to keep around its cause implying the TRY_FAIL can 
not be a singleton anymore.
Being only used for caching purpose for *internal, the probability that the API 
contract actually breaks user code seems very (very) unlikely...


was (Author: henrib):
Notes and implications of the proposed tryInvoke behaviour change. 
The proposed modifications breaks the API contract by suddenly potentially 
throwing an exception, tryInvoke working on the premise that a previously 
resolved method can be called again successfully with the same type of 
arguments. Most if not all executors would be impacted by the change (not only 
MethodExecutor but all *Executor).
Reviewing the code, there does not seem to be a case where the code caches a 
method that has (hard) failed. 
The issue described appears when the 1st invocation is successful but the 2nd 
(tryInvoke) and 3rd (invoke) fail... If we want to avoid the 3rd (failing) 
invocation, we need the 2nd to keep around its cause implying the TRY_FAIL can 
not be a singleton anymore.
Being only used for caching purpose for *internal, the probability that the API 
contract actually breaks user code seems very (very) unlikely...

> Function throwing IllegalArgumentException may called twice
> -----------------------------------------------------------
>
>                 Key: JEXL-257
>                 URL: https://issues.apache.org/jira/browse/JEXL-257
>             Project: Commons JEXL
>          Issue Type: Bug
>    Affects Versions: 3.1
>            Reporter: Dmitri Blinov
>            Priority: Major
>
> With regard to JEXL-256 I have noticed that a function which once had throw 
> IllegalArgumentException may be called immediately once again in script. The 
> problem is not always reproducible so I think it is somehow related to 
> caching and {{tryInvoke()}} being called first and {{invoke()}} called 
> afterwards.
> I can't produce test case at the moment but try to describe on the example. I 
> ommited some irrelevant details for the sake of simplicity...
> First I have a function that evaluates Jexl script within script.
> {code:java}
> eval("java = 1")
> {code}
> Then I have a {{Context.set()}} that works like this
> {code:java}
> public void set(String name) {
> if (name.equals("java"))
>    throw new IllegalArgumentException("java");
> ...
> {code}
> As I stated in JEXL-256, when {{Context.set()}} throws 
> IllegalArgumentException then script execution immediately terminates with 
> this exception, and not with JexlException. So, the function {{eval()}} im my 
> case terminates with IllegalArgumentException also.
> Then I have two stack traces in the log, one for the first invocation
> {quote}java.lang.IllegalArgumentException: java
>  at MyDefaultContext.set(MyDefaultContext.java:279) 
>  at 
> org.apache.commons.jexl3.internal.Interpreter.executeAssign(Interpreter.java:1189)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:1094) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTAssignment.jjtAccept(ASTAssignment.java:18)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:890) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTJexlScript.jjtAccept(ASTJexlScript.java:58)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.interpret(Interpreter.java:190) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at org.apache.commons.jexl3.internal.Script.execute(Script.java:185) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at EvaluationContext.evalScript(EvaluationContext.java:1078) 
>  at EvaluationContext.evaluateScript(EvaluationContext.java:1043) 
>  at MyDefaultContext.eval(MyDefaultContext.java:736)
>  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_162]
>  at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) 
> ~[?:1.8.0_162]
>  at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>  ~[?:1.8.0_162]
>  at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_162]
>  at 
> org.apache.commons.jexl3.internal.introspection.MethodExecutor.invoke(MethodExecutor.java:93)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.introspection.MethodExecutor.tryInvoke(MethodExecutor.java:104)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter$Funcall.tryInvoke(Interpreter.java:1436)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at org.apache.commons.jexl3.internal.Interpreter.call(Interpreter.java:1545) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:1357) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTFunctionNode.jjtAccept(ASTFunctionNode.java:18)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.executeAssign(Interpreter.java:1149)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:1094) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTAssignment.jjtAccept(ASTAssignment.java:18)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.processAnnotation(Interpreter.java:1907)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter$1.call(Interpreter.java:1917) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> com.msy.einie.f1.jexl.MsyDefaultContext.processAnnotation(MsyDefaultContext.java:531)
>  ~[msy-1.0.jar:1.0]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.processAnnotation(Interpreter.java:1963)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.processAnnotation(Interpreter.java:1936)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:1877) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTAnnotatedStatement.jjtAccept(ASTAnnotatedStatement.java:18)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:890) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTJexlScript.jjtAccept(ASTJexlScript.java:58)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.interpret(Interpreter.java:190) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at org.apache.commons.jexl3.internal.Script.execute(Script.java:185) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
> {quote}
> and the one for the second
> {quote}java.lang.IllegalArgumentException: java
>  at MyDefaultContext.set(MyDefaultContext.java:279)
>  at 
> org.apache.commons.jexl3.internal.Interpreter.executeAssign(Interpreter.java:1189)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:1094) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTAssignment.jjtAccept(ASTAssignment.java:18)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:890) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTJexlScript.jjtAccept(ASTJexlScript.java:58)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.interpret(Interpreter.java:190) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at org.apache.commons.jexl3.internal.Script.execute(Script.java:185) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at EvaluationContext.evalScript(EvaluationContext.java:1078)
>  at EvaluationContext.evaluateScript(EvaluationContext.java:1043)
>  at MyDefaultContext.eval(MyDefaultContext.java:736)
>  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_162]
>  at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) 
> ~[?:1.8.0_162]
>  at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>  ~[?:1.8.0_162]
>  at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_162]
>  at 
> org.apache.commons.jexl3.internal.introspection.MethodExecutor.invoke(MethodExecutor.java:93)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at org.apache.commons.jexl3.internal.Interpreter.call(Interpreter.java:1642) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:1357) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTFunctionNode.jjtAccept(ASTFunctionNode.java:18)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.executeAssign(Interpreter.java:1149)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:1094) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTAssignment.jjtAccept(ASTAssignment.java:18)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.processAnnotation(Interpreter.java:1907)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter$1.call(Interpreter.java:1917) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> com.msy.einie.f1.jexl.MsyDefaultContext.processAnnotation(MsyDefaultContext.java:531)
>  ~[msy-1.0.jar:1.0]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.processAnnotation(Interpreter.java:1963)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.processAnnotation(Interpreter.java:1936)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:1877) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTAnnotatedStatement.jjtAccept(ASTAnnotatedStatement.java:18)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at org.apache.commons.jexl3.internal.Interpreter.visit(Interpreter.java:890) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.parser.ASTJexlScript.jjtAccept(ASTJexlScript.java:58)
>  ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at 
> org.apache.commons.jexl3.internal.Interpreter.interpret(Interpreter.java:190) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
>  at org.apache.commons.jexl3.internal.Script.execute(Script.java:185) 
> ~[commons-jexl-3.2.jar:3.2-SNAPSHOT]
> {quote}
> I think this is a bug and we should not call a function twice if it returned 
> {{IllegalArgumentException}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to