search

jakartaee / expression-language

Jakarta Expression Language
https://eclipse.org/ee4j/el
Other
59 stars 49 forks source link

Should all exceptions in ExpressionFactory#coerceToType be wrapped in an ELException? #217

Open mkomko opened 1 month ago

mkomko commented 1 month ago

I was trying to implement better logging for our Faces application and ran into a problem. When the problematic expression goes through ExpressionFactory#coerceToType, the resulting exception is not wrapped in an ELException and thus some information is lost along the way.

Here is our example stack trace:

org.hibernate.LazyInitializationException: Unable to perform requested lazy initialization [redacted.object] - session is closed and settings disallow loading outside the Session
    at org.hibernate@6.4.4.Final//org.hibernate.bytecode.enhance.spi.interceptor.EnhancementHelper.throwLazyInitializationException(EnhancementHelper.java:275)
    at org.hibernate@6.4.4.Final//org.hibernate.bytecode.enhance.spi.interceptor.EnhancementHelper.performWork(EnhancementHelper.java:174)
    at org.hibernate@6.4.4.Final//org.hibernate.bytecode.enhance.spi.interceptor.EnhancementAsProxyLazinessInterceptor.handleRead(EnhancementAsProxyLazinessInterceptor.java:106)
    at org.hibernate@6.4.4.Final//org.hibernate.bytecode.enhance.spi.interceptor.AbstractInterceptor.readObject(AbstractInterceptor.java:152)
    at deployment.redacted.ear.redacted-ejb.jar//redacted.Object.$$_hibernate_read_field(RedactedEntity.java)
    at deployment.redacted.ear.redacted-ejb.jar//redacted.Object.toString(RedactedEntity.java:1173)
    at org.glassfish.expressly@5.0.0//org.glassfish.expressly.lang.ELSupport.coerceToString(ELSupport.java:401)
    at org.glassfish.expressly@5.0.0//org.glassfish.expressly.lang.ELSupport.coerceToType(ELSupport.java:438)
    at org.glassfish.expressly@5.0.0//org.glassfish.expressly.ExpressionFactoryImpl.coerceToType(ExpressionFactoryImpl.java:70)
    at jakarta.el.api@4.0.1.Final//jakarta.el.ELContext.convertToType(ELContext.java:440)
    at org.glassfish.expressly@5.0.0//org.glassfish.expressly.lang.EvaluationContext.convertToType(EvaluationContext.java:141)
    at org.glassfish.expressly@5.0.0//org.glassfish.expressly.ValueExpressionImpl.getValue(ValueExpressionImpl.java:142)
    at org.jboss.weld.core@5.1.2.Final//org.jboss.weld.module.web.el.WeldValueExpression.getValue(WeldValueExpression.java:50)
    at jakarta.faces.impl:myfaces@4.0.2//org.apache.myfaces.view.facelets.el.ContextAwareTagValueExpression.getValue(ContextAwareTagValueExpression.java:100)

If the exception would be wrapped in an ELException, MyFaces would wrap it in an exception that contains information about the location and expression that resulted in the exception. As it is, the exception is being passed through.

I checked different implementations, and both seem to behave the same: Tomcat Expressly

The question is: should the exception actually be wrapped in an ELException? The spec says: An ELException is thrown if an error results from applying the conversion rules.

Thank you very much in advance!

mkomko commented 1 week ago

@markt-asf Do you have any thoughts?

markt-asf commented 1 week ago

That looks like a reasonable interpretation of the current Javadoc. Fixing that would be an implementation issue rather than an EL API issue.

I think this is something we could write a TCK test for therefore I am leaving this issue open until we have a TCK test.