Repository: PebbleTemplates/pebble
Pull request #187 (2016-03-21 13:01:49)

Exceptions in parallel nodes should bubble up and not get swallowed silently
As I said in #182: Exception in parallel nodes get swallowed and don't bubble up to the root context. That makes it hard to see that something is failing in the parallel sections as the template.eval doesn't throw an exception. The only way to detect it is that the parallel part isn't rendered. 

This patch adds a test for detecting this problem as well as changing FutureWriter to let exceptions bubble up on flush/close. I think this should fix the issue and it doesn't look like it introduces problems anywhere else.


Patch:
diff --git a/src/main/java/com/mitchellbosecke/pebble/utils/FutureWriter.java b/src/main/java/com/mitchellbosecke/pebble/utils/FutureWriter.java
index ac32a52d..c8b0adec 100644
--- a/src/main/java/com/mitchellbosecke/pebble/utils/FutureWriter.java
+++ b/src/main/java/com/mitchellbosecke/pebble/utils/FutureWriter.java
@@ -1,8 +1,8 @@
 /*******************************************************************************
  * This file is part of Pebble.
- * 
+ *
  * Copyright (c) 2014 by Mitchell Bösecke
- * 
+ *
  * For the full copyright and license information, please view the LICENSE
  * file that was distributed with this source code.
  ******************************************************************************/
@@ -20,12 +20,12 @@ import java.util.concurrent.TimeoutException;
  * A Writer that will wrap around the user-provided writer if the user also
  * provided an ExecutorService to the main PebbleEngine. A FutureWriter is
  * capable of handling Futures that will return a string.
- * 
+ *
  * It is not thread safe but that is okay. Each thread will have it's own
  * writer, provided by the "parallel" node; i.e. they will never share writers.
- * 
+ *
  * @author Mitchell
- * 
+ *
  */
 public class FutureWriter extends Writer {
 
@@ -99,9 +99,10 @@ public class FutureWriter extends Writer {
                 String result = future.get();
                 internalWriter.write(result);
                 internalWriter.flush();
-            } catch (InterruptedException | ExecutionException e) {
-                // TODO Auto-generated catch block
-                e.printStackTrace();
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+            } catch (ExecutionException e) {
+                throw new IOException(e);
             }
         }
         orderedFutures.clear();


Test patch:
diff --git a/src/test/java/com/mitchellbosecke/pebble/ConcurrencyTest.java b/src/test/java/com/mitchellbosecke/pebble/ConcurrencyTest.java
index 50a337d5..c38b05d7 100644
--- a/src/test/java/com/mitchellbosecke/pebble/ConcurrencyTest.java
+++ b/src/test/java/com/mitchellbosecke/pebble/ConcurrencyTest.java
@@ -30,6 +30,7 @@ import java.util.concurrent.Semaphore;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 public class ConcurrencyTest extends AbstractTest {
 
@@ -370,7 +371,55 @@ public class ConcurrencyTest extends AbstractTest {
         template.evaluate(multipleThreadResult);
 
         executor.shutdown();
-        
+
         assertEquals(singleThreadResult.toString(), multipleThreadResult.toString());
     }
+
+    @Test
+    public void testConcurrentEvaluationWithException() throws PebbleException, IOException
+    {
+        PebbleEngine engine = new PebbleEngine.Builder().loader(new StringLoader()).strictVariables(false).build();
+
+        String templateSource =
+                  "{% for i in [1,2,3,4,5,6,7,8,9,10] %}"
+                +   "{% parallel %}"
+                +       "{{test(i)}}"
+                +   "{% endparallel %}"
+                + "{% endfor %}";
+
+        PebbleTemplate template = engine.getTemplate(templateSource);
+
+        StringWriter singleThreadResult = new StringWriter();
+
+        try
+        {
+            template.evaluate(singleThreadResult);
+            fail("Expecting the single thread evaluation to throw an exception");
+        }
+        catch (Exception ex)
+        {
+        }
+
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        engine = new PebbleEngine.Builder().loader(new StringLoader()).executorService(executor).strictVariables(false).build();
+
+        template = engine.getTemplate(templateSource);
+
+        StringWriter multipleThreadResult = new StringWriter();
+
+        try
+        {
+            template.evaluate(multipleThreadResult);
+            fail("Expection the multi thread evaluation to throw an exception");
+        }
+        catch (Exception ex)
+        {
+        }
+
+        executor.shutdown();
+
+        assertEquals("Expection the result of multiple threads and single thread execution to match.",
+            singleThreadResult.toString(), multipleThreadResult.toString());
+    }
 }