Re: Fwd: svn commit: r919953 - in /click/trunk/click/framework/src/org/apache/click/service: TemplateService.java VelocityTemplateService.java

[Bob Schellink]

In short look at line 119 in ErrorReport:

  if (te.getTemplateName() != null)
This code path is not followed and the sourceReader is not instantiated, meaning less diagnostics 
are provided.

kind regards

bob


On 7/03/2010 10:51 PM, Malcolm Edgar wrote:
I will take another look at this, its should be exposing the
underlying cause. Obviously its not. Thanks for the review.

regards Malcolm Edgar

On Sun, Mar 7, 2010 at 10:45 PM, Bob Schellink<sab...@gmail.com>  wrote:
Hi Malcolm,

If the template threw a normal Exception the error was wrapped in a
TemplateException and passed to ErrorReport. Inside ErrorReport a check is
made if the error is a TemplateException. If it is, a check is made if the
TemplateName is set nor not. If the TemplateName is *not* set, as will be
the case of a NPE, very little diagnostics is given. So I reverted
TemplateService to throw a normal exception instead of wrapping it in a TE.
This way the other code path is followed when diagnostics is provided for
the Exception.

Another way we could resolve it is to provide the templatePath when a normal
Exception is thrown?

kind regards

bob

On 7/03/2010 10:32 PM, Malcolm Edgar wrote:

Hi Bob,

What was the exception issue caused by this refactoring?

regards Malcolm Edgar

---------- Forwarded message ----------
From:<sa...@apache.org>
Date: Sun, Mar 7, 2010 at 8:33 PM
Subject: svn commit: r919953 - in
/click/trunk/click/framework/src/org/apache/click/service:
TemplateService.java VelocityTemplateService.java
To: comm...@click.apache.org


Author: sabob
Date: Sun Mar  7 09:33:51 2010
New Revision: 919953

URL: http://svn.apache.org/viewvc?rev=919953&view=rev
Log:
fixed exception handling caused by CLK-606 refactoring

Modified:

  click/trunk/click/framework/src/org/apache/click/service/TemplateService.java

  
click/trunk/click/framework/src/org/apache/click/service/VelocityTemplateService.java

Modified:
click/trunk/click/framework/src/org/apache/click/service/TemplateService.java
URL:
http://svn.apache.org/viewvc/click/trunk/click/framework/src/org/apache/click/service/TemplateService.java?rev=919953&r1=919952&r2=919953&view=diff

==============================================================================
---
click/trunk/click/framework/src/org/apache/click/service/TemplateService.java
(original)
+++
click/trunk/click/framework/src/org/apache/click/service/TemplateService.java
Sun Mar  7 09:33:51 2010
@@ -76,7 +76,7 @@
      * @throws TemplateException if template error occurs
      */
     public void renderTemplate(Page page, Map<String, Object>    model,
Writer writer)
-        throws IOException, TemplateException;
+        throws TemplateException, Exception;

     /**
      * Render the given template and model to the writer.
@@ -88,6 +88,6 @@
      * @throws TemplateException if template error occurs
      */
     public void renderTemplate(String templatePath, Map<String,
Object>    model, Writer writer)
-        throws IOException, TemplateException;
+        throws TemplateException, Exception;

  }

Modified:
click/trunk/click/framework/src/org/apache/click/service/VelocityTemplateService.java
URL:
http://svn.apache.org/viewvc/click/trunk/click/framework/src/org/apache/click/service/VelocityTemplateService.java?rev=919953&r1=919952&r2=919953&view=diff

==============================================================================
---
click/trunk/click/framework/src/org/apache/click/service/VelocityTemplateService.java
(original)
+++
click/trunk/click/framework/src/org/apache/click/service/VelocityTemplateService.java
Sun Mar  7 09:33:51 2010
@@ -31,6 +31,7 @@
  import javax.servlet.ServletContext;

  import org.apache.click.Page;
+import org.apache.click.Context;
  import org.apache.click.util.ClickUtils;
  import org.apache.click.util.ErrorReport;
  import org.apache.commons.lang.Validate;
@@ -315,7 +316,7 @@
      * @throws TemplateException if template error occurs
      */
     public void renderTemplate(Page page, Map<String, Object>    model,
Writer writer)
-        throws IOException, TemplateException {
+        throws TemplateException, Exception {

         String templatePath = page.getTemplate();

@@ -338,7 +339,7 @@
      * @throws Exception if an error occurs
      */
     public void renderTemplate(String templatePath, Map<String,
Object>    model, Writer writer)
-        throws IOException, TemplateException {
+        throws TemplateException, Exception {

         internalRenderTemplate(templatePath, null, model, writer);
     }
@@ -506,7 +507,7 @@
                                           Page page,
                                           Map<String, Object>    model,
                                           Writer writer)
-        throws IOException, TemplateException {
+        throws TemplateException, Exception {

         final VelocityContext velocityContext = new
VelocityContext(model);

@@ -534,9 +535,6 @@

             template.merge(velocityContext, velocityWriter);

-        } catch (IOException ioe) {
-            throw ioe;
-
         } catch (ParseErrorException pee) {
             TemplateException te = new TemplateException(pee,

  pee.getTemplateName(),
@@ -615,16 +613,23 @@
             throw te;

         } catch (Exception error) {
-            TemplateException te = new TemplateException(error);

             // Exception occurred merging template and model. It is
possible
             // that some output has already been written, so we will
append the
             // error report to the previous output.
+
+            Context context = null;
+            if (page == null) {
+                context = Context.getThreadLocalContext();
+            } else {
+                context = page.getContext();
+            }
+
             ErrorReport errorReport =
-                new ErrorReport(te,
+                new ErrorReport(error,
                         ((page != null) ? page.getClass() : null),
                         configService.isProductionMode(),
-                        page.getContext().getRequest(),
+                        context.getRequest(),
                         configService.getServletContext());

             if (velocityWriter == null) {
@@ -634,7 +639,7 @@

             velocityWriter.write(errorReport.toString());

-            throw te;
+            throw error;

         } finally {
             if (velocityWriter != null) {