[
https://issues.apache.org/jira/browse/EL-17?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13039949#comment-13039949
]
Sebb commented on EL-17:
------------------------
The original code in EL 1.0 is
{code}
if (pLogger.isLoggingError ()) {
pLogger.logError
(Constants.NO_PROPERTY_EDITOR,
str,
pClass.getName ());
}
return null;
{code}
At first sight, this appears to conditionally log a message and then return
null.
However, the logError() method actually throws the exception ELException - it
does not log an error at all.
Furthermore, the method isLoggingError() always returns true, so the return
statement is never reached.
This means that the new code is equivalent, provided that isErrorEnabled()
behaves the same as isLoggingError().
However, that is not the case, as the latter is the logging level, whereas the
former determined whether or not to throw an Exception.
> Exceptions thrown in conditional logging blocks - looks totally wrong
> ---------------------------------------------------------------------
>
> Key: EL-17
> URL: https://issues.apache.org/jira/browse/EL-17
> Project: Commons EL
> Issue Type: Bug
> Reporter: Sebb
>
> The commit:
> http://svn.apache.org/viewvc?view=revision&revision=133240
> says
> Minor cleanup. Patch provided by Ryan Lubke
> Sounds innocuous. However, some of the changes actually added throw
> statements within conditional logging statements, for example:
> {code}
> ---
> jakarta/commons/proper/el/trunk/src/java/org/apache/commons/el/Coercions.java
> 2003/07/31 02:01:07 133239
> +++
> jakarta/commons/proper/el/trunk/src/java/org/apache/commons/el/Coercions.java
> 2003/08/01 21:12:32 133240
> @@ -394,11 +394,13 @@
> try {
> return pValue.toString ();
> }
> - catch (Exception exc) {
> + catch (Exception exc) {
> if (log.isErrorEnabled()) {
> - log.error(
> -
> MessageUtil.getMessageWithArgs(Constants.TOSTRING_EXCEPTION,
> -
> pValue.getClass().getName()), exc);
> + String message = MessageUtil.getMessageWithArgs(
> + Constants.TOSTRING_EXCEPTION,
> + pValue.getClass().getName());
> + log.error(message, exc);
> + throw new ELException(exc);
> }
> return "";
> }
> {code}
> The first part of the change does nothing except expose the message as a
> string, but why is the throw statement added?
> That seems completely wrong.
> There are a few other places where throw statements have been added to the
> logging conditional blocks.
> Either these should be removed, or they should be outside the block.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
