struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rene Gielen <gie...@it-neering.net>
Subject Re: Exception thrown in OGNL evaluation.
Date Fri, 17 Aug 2007 07:42:12 GMT
Ruimo,

see below

Ruimo Uno schrieb:
> Hi, thanks for your comment.
> 
> 2007/8/17, Rene Gielen <gielen@it-neering.net>:
>> It's no bug, it's a feature...
>>
>> The policy for model access (e.g. property calls) via expression
>> evaluation is fail silent. It would cause tons of exceptions if ognl
>> expression evaluation / property access would not swallow them.
> 
> I feel it is overkill. The property getters in action classes may
> throw RuntimeException because of programming bugs. Even in this case,
> the current implementation just ignore the exception and the shown
> page will be just corrupted. We cannot show user friendly 'sorry page'
> because the Struts exception handler cannot get exception.
>

You would not be able to use the most common use patterns without having 
to do exepction handling on the JSP page then! Think of a model object 
foo with a getter getFoo() in your a FooCrudAction. You have an 
editFoo.jsp. If you want to create a new foo entry, the sequence would be
- invoke edit/create action, triggering execute() in FooCrudAction
- render editFoo.jsp
- submit form
- invoke save action, triggering save() in FooCrudAction
- on success, render editFoo.jsp

if ognl would not swallow the exceptions, then placing simple tag like
<s:textfield label="'Name'" name="foo.name" />
would fail with NPE in first rendering if you did not provide a 
(unnecessary) this.foo=new Foo() in your execute() method, or a lazy 
getter. Just a simple example. But if you would change this behaviour, 
the showcase app code would need approximately an additional 10-15% more 
code just for doing unneeded initializations and exception handling.

What if the execption is thrown? It will be thrown within JSP page 
handling, and swallowed there! Your OpenSessionInView Interceptor or any 
higher level layer would take no notice of the exception. Instead you 
would have to use the anti-pattern of exception handliing with in JSP 
code to present useful information.

>> Most likely, you are doing business logic calls in your model access
>> domain and you should think of moving any call with the possibility to get
>> service relevant exceptions to the described business logic domain access
>> methods.
> 
> No, it is not 'business logc'. In 'open session in view' pattern, the
> db transaction is kept open until the view rendering finishies. If you
> call the property access method of the acction class, it calls the
> getter method of tne entity class. It triggers O/R mapping layer
> invocation and lazily accesses the database. As a result, some kind of
> system exception (such as SQLException) may be thrown while view
> rendering.
> 

This pattern is widely used, I'm using it myself in many applications. 
But where do you expect the exception? If you make sure that you have a 
foo object loaded from persistence layer, the the foo.getBars() call to 
a a lazy initialized collection of referenced objects should never fail 
due to _runtime_ problems you have to deal with. If, for example, for 
some reason your session is closed, you have to look for a design time 
problem - most commonly people forget about holding the session open 
(luckily we have OpenSessionInView).
The best practice pattern for a displaying a foo object looks like this:

- invoke showFoo.action?id=1
- prepare() method of your action: this.foo = fooService.getById(this.id)
- handle any exception you like, or let it pop to the upper layers
- on success, render showFoo.jsp
- iterate over foo.bars in view - will be lazy initialized, but should 
not throw any exception is your design was proper

but if there was an exception, and OGNL would pop it, you still would 
_never_ get it in your Interceptor - your container will swallow it when 
rendering the JSP.

it's the same for saving etc as well.

Did you check out these?:
http://cwiki.apache.org/confluence/display/WW/CRUD+Demo+I
http://appfuse.org/display/APF/Demos+and+Videos

Regards,
Rene

>> Am Do, 16.08.2007, 17:01, schrieb Ruimo Uno:
>>> Hi,
>>>
>>> I have submitted this to user list a few hours ago but finally, I
>>> found the root cause of this issue. So, here I report it to dev list.
>>> If an exception is thrown while OGNL expression evaluation such as in
>>> the following scenario:
>>>
>>> XXXAction
>>>     public String getMessage() {
>>>         throw new NullPointerException();
>>>     }
>>>
>>> XXX.jsp
>>>   <s:property value="message"/>
>>>
>>> The exception is wrapped in an OgnlException and re-thrown.  It is
>>> caught in com.opensymphony.xwork2.util.CompoundRootAccessor.getProperty()
>>> and wrapped in a XWorkException() and re-thrown.
>>>
>>>     } catch (OgnlException e) {
>>>         if (e.getReason() != null) {
>>>             final String msg = "Caught an Ognl exception while getting
>>> property " + name;
>>>             throw new XWorkException(msg, e);
>>>         }
>>>     } catch (IntrospectionException e) {
>>>
>>> Finally, it is caught in
>>> com.opensymphony.xwork2.util.OgnlValueStack.findValue(String, Class)
>>> and swallowed.
>>>
>>>         } catch (Exception e) {
>>>             logLookupFailure(expr, e);
>>>
>>>             return findInContext(expr);
>>>         } finally {
>>>
>>> As a result, the upper layer does not receive any exception. So, no
>>> exception handler is invoked even if it is registered. Moreover, in
>>> 'open session in view' pattern, since the upper layer are not notified
>>> any error, the transaction is wrongly committed and it causes serious
>>> problem.
>>>
>>> I tried to add a catch clause for RuntimeException:
>>>
>>>         } catch (RuntimeException e) {
>>>             logLookupFailure(expr, e);
>>>             throw e;
>>>         } catch (Exception e) {
>>>             logLookupFailure(expr, e);
>>>
>>>             return findInContext(expr);
>>>         } finally {
>>>
>>> Yes, the exception is successfully thrown to the upper layer with this
>>> code change. However, Struts shows 'java.io.IOException: Stream
>>> closed:' on the browser. This message seems a bit confusing. and
>>> difficult to realize what is actually happening (NPE is thrown). Does
>>> anybody have better solution on this?
>>>
>>> Thanks,
>>>
>>> --
>>> Ruimo Uno
>>> (Shisei Hanai)
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
>>> For additional commands, e-mail: dev-help@struts.apache.org
>>>
>>>
>>
>> --
>> Rene Gielen  | http://it-neering.net/
>> Aachen       | PGP-ID: BECB785A
>> Germany      | gielen at it-neering.net
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
>> For additional commands, e-mail: dev-help@struts.apache.org
>>
>>
> 
> 


-- 
Rene Gielen  | http://it-neering.net/
Aachen       | PGP-ID: BECB785A
Germany      | gielen at it-neering.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Mime
View raw message