SpEL: Make EvaluationException more informative with Expression [SPR-10938]

[spring-projects-issues]

Artem Bilan opened SPR-10938 and commented

As far as SpEL is some runtime tool, which evaluation result fully depends of provided context (rootObject, variables etc..), it's not enough to see in theStackTrace just a problem, e.g.

Caused by: java.lang.IllegalArgumentException: Cannot convert value of type [java.util.Date] to required type [int]:

It would be better to see which Expression (at least) causes the problem.

In addition, SpEL is some central tool in the Spring Integration, where one Handler may evaluate not one expression for one call, but different Messages may cause different EvaluationException, but with the same expression.

Something similar there is already in the Spring Integration: https://github.com/spring-projects/spring-integration/blob/master/spring-integration-core/src/main/java/org/springframework/integration/util/AbstractExpressionEvaluator.java#L118
But, it would be better, if core EvaluationException will provide the same.

---

Issue Links:

• Include expression string in SpelParseException [SPR-14942] #19509 Include expression string in SpelParseException

Referenced from: commits 014d156

[spring-projects-issues]

Phil Webb commented

ExpressionException provides getExpressionString() and toDetailedString() methods that can be used to report details about the expression. Will they work for your needs?

[spring-projects-issues]

Artem Bilan commented

Hi, Phil!

Thank you for attention to this.

What is bad here, that ExpressionException extends RuntimeException. So, it assumes that we don't catch it and see the StackTrace as it is from orginal RuntimeException behavior.
You suggest to catch all expression.getValue and build our onw exception, and already with getExpressionString() or toDetailedString().
In this case the contract of ExpressionException should not be RuntimeException.

What would be better to override ExpressionException.getMessage():

@Override
public String getMessage() {
     return this.toDetailedString();
}

And use super.getMessage() inside toDetailedString()
In this case we save backward compatibility and get more info with new getMessage() variant.

How about this opinion?

Cheers

[spring-projects-issues]

Phil Webb commented

Pulling in Andy Clement to this discussion as he was the original author.

Andy, was/is there any specific reason not to expose the detailed string as part of the standard message? Are there security concerns for example?

[spring-projects-issues]

Andy Clement commented

No special reason that I can recall. I'd be ok with including that extra information in the exception message I think.