forrest-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ross Gardler <rgard...@apache.org>
Subject Re: svn commit: r357622 - /forrest/trunk/main/java/org/apache/forrest/locationmap/LocationMapModule.java
Date Mon, 19 Dec 2005 13:42:31 GMT
Tim Williams wrote:
> On 12/19/05, Ross Gardler <rgardler@apache.org> wrote:
> 
>>twilliams@apache.org wrote:
>>
>>>Author: twilliams
>>>Date: Sun Dec 18 19:11:15 2005
>>>New Revision: 357622
>>>
>>>URL: http://svn.apache.org/viewcvs?rev=357622&view=rev
>>>Log:
>>>log null return values, but they aren't errors.
>>>
>>>Modified:
>>>    forrest/trunk/main/java/org/apache/forrest/locationmap/LocationMapModule.java
>>>
>>>Modified: forrest/trunk/main/java/org/apache/forrest/locationmap/LocationMapModule.java
>>>URL: http://svn.apache.org/viewcvs/forrest/trunk/main/java/org/apache/forrest/locationmap/LocationMapModule.java?rev=357622&r1=357621&r2=357622&view=diff
>>>==============================================================================
>>>--- forrest/trunk/main/java/org/apache/forrest/locationmap/LocationMapModule.java
(original)
>>>+++ forrest/trunk/main/java/org/apache/forrest/locationmap/LocationMapModule.java
Sun Dec 18 19:11:15 2005
>>>@@ -189,9 +189,10 @@
>>>              }
>>>
>>>           if (result == null) {
>>>-            String msg = "Locationmap did not return a location for hint " +
name;
>>>-            getLogger().error(msg);
>>>-            throw new Exception(msg);
>>>+              if (getLogger().isDebugEnabled()) {
>>>+                String msg = "Locationmap did not return a location for hint
" + name;
>>>+                getLogger().debug(msg);
>>>+              }
>>>           }
>>
>>
>>I made them exceptions because it makes the logs much clearer and, in
>>fact they are (IMHO) errors. My reasoning is this:
> 
> 
> I kept the log entry just changed it to debug instead of error.

Which means it only appears, deeply embedded in a huge load of other 
messages. Making it very difficult to spot this very common error, but...

>  We
> disagree on whether it's an error or not and I'll try to change your
> opinion below...

OK, lets see...

>>If I have a request for {lm:foo} and there is no match anywhere in the
>>locationmaps for that value then I cannot possibly evaluate that
>>location. Therefore Cocoon will be unable to process the request.
> 
> 
> Sure you can evaluate it and Cocoon can process it.  Returning null
> (i.e. saying the location does not exist) *is* evaluating that
> location and the exists selector can process it.  Otherwise, we're
> throwing exceptions every time we use the locationmap in an exists
> selector in a sitemap.
>  I don't consider it an error in the example
> below when foo does not exist.
> <map:select type="exists">
>     <map:when test="{lm:foo}">
>     </map:when>
>     <map:when test="{lm:bar}">
>     </map:when>
> </map:select>

Yes, good point, I had indeed missed that.

I'm not happy with this being a debug as it gets lost and makes 
locationmaps difficult to debug when this is an error (i.e. it is not 
used in a selector in the sitemap).

How about we make it a warning and change the message?

Ross

Mime
View raw message