forrest-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Williams <william...@gmail.com>
Subject Re: svn commit: r357622 - /forrest/trunk/main/java/org/apache/forrest/locationmap/LocationMapModule.java
Date Mon, 19 Dec 2005 13:01:48 GMT
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.  We
disagree on whether it's an error or not and I'll try to change your
opinion below...

> 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>

Not to mention, throwing exceptions on this makes our error logs
terribly large even though it's a perfectly reasonably approach --
take a look at the error log after building site-author statically.

> Did you miss the fact that I moved the trap to the top level so an
> exception is only thrown if *all* mounted locationmaps have been
> checked? (previously this trap was done on a per locationmap basis in
> which case it would not be an error).

I didn't miss that;)

> Or is it that I am missing something?

I believe this to be the case, but we'll see once we get it worked out.

--tim

Mime
View raw message