Merge pull request #1357 from HubSpot/jasmith_hubspot/fatal-error-deferred-exception

feat(eager): add FATAL error when a node is deferred due to an exception

Author: jasmith-hs

src/main/java/com/hubspot/jinjava/tree/ExpressionNode.java

@@ -17,6 +17,7 @@
 
 import com.hubspot.jinjava.interpret.DeferredValueException;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
+import com.hubspot.jinjava.interpret.TemplateError;
 import com.hubspot.jinjava.lib.expression.DefaultExpressionStrategy;
 import com.hubspot.jinjava.lib.expression.ExpressionStrategy;
 import com.hubspot.jinjava.tree.output.OutputNode;
@@ -49,6 +50,11 @@ public OutputNode render(JinjavaInterpreter interpreter) {
       return expressionStrategy.interpretOutput(master, interpreter);
     } catch (DeferredValueException e) {
       interpreter.getContext().handleDeferredNode(this);
+      if (interpreter.getConfig().getExecutionMode().useEagerParser()) {
+        interpreter.addError(
+          TemplateError.fromException(e, getLineNumber(), getStartPosition())
+        );
+      }
       return new RenderedOutputNode(master.getImage());
     } finally {
       postProcess(interpreter);

src/main/java/com/hubspot/jinjava/tree/TagNode.java

@@ -21,6 +21,7 @@
 import com.hubspot.jinjava.interpret.InvalidInputException;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
 import com.hubspot.jinjava.interpret.OutputTooBigException;
+import com.hubspot.jinjava.interpret.TemplateError;
 import com.hubspot.jinjava.lib.tag.FlexibleTag;
 import com.hubspot.jinjava.lib.tag.Tag;
 import com.hubspot.jinjava.tree.output.OutputNode;
@@ -58,6 +59,11 @@ public OutputNode render(JinjavaInterpreter interpreter) {
       return tag.interpretOutput(this, interpreter);
     } catch (DeferredValueException e) {
       interpreter.getContext().handleDeferredNode(this);
+      if (interpreter.getConfig().getExecutionMode().useEagerParser()) {
+        interpreter.addError(
+          TemplateError.fromException(e, getLineNumber(), getStartPosition())
+        );
+      }
       return new RenderedOutputNode(reconstructImage());
     } catch (
       InterpretException

src/test/java/com/hubspot/jinjava/EagerTest.java

@@ -9,6 +9,7 @@
 import com.hubspot.jinjava.interpret.Context;
 import com.hubspot.jinjava.interpret.DeferredValue;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
+import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
 import com.hubspot.jinjava.loader.LocationResolver;
 import com.hubspot.jinjava.loader.RelativePathResolver;
 import com.hubspot.jinjava.loader.ResourceLocator;
@@ -1591,6 +1592,14 @@ public void itFailsOnModificationInAliasedMacro() {
     interpreter.render(input);
     // We don't support this
     assertThat(interpreter.getContext().getDeferredNodes()).isNotEmpty();
+    assertThat(interpreter.getErrors())
+      .singleElement()
+      .satisfies(error -> {
+        assertThat(error.getSeverity()).isEqualTo(ErrorType.FATAL);
+        assertThat(error.getMessage())
+          .contains("Cannot modify temporary import alias outside of import tag");
+      });
+    interpreter.removeLastError();
   }
 
   @Test

src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstDotTest.java

@@ -10,6 +10,8 @@
 import com.hubspot.jinjava.interpret.Context;
 import com.hubspot.jinjava.interpret.DeferredValue;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
+import com.hubspot.jinjava.interpret.TemplateError;
+import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
 import com.hubspot.jinjava.mode.EagerExecutionMode;
 import com.hubspot.jinjava.random.RandomNumberGeneratorStrategy;
 import com.hubspot.jinjava.testobjects.EagerAstDotTestObjects;
@@ -74,5 +76,8 @@ public void itDefersNodeWhenNestedDeferredMapDotThrowsDeferredValueException() {
     assertThat(interpreter.render("{{ foo_map.bar.deferred }}"))
       .isEqualTo("{{ foo_map.bar.deferred }}");
     assertThat(interpreter.getContext().getDeferredNodes()).isNotEmpty();
+    assertThat(interpreter.getErrors())
+      .extracting(TemplateError::getSeverity)
+      .containsExactly(ErrorType.FATAL);
   }
 }

src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerDoTagTest.java

@@ -7,7 +7,9 @@
 import com.hubspot.jinjava.LegacyOverrides;
 import com.hubspot.jinjava.interpret.DeferredValue;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
+import com.hubspot.jinjava.interpret.TemplateError;
 import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
+import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
 import com.hubspot.jinjava.lib.tag.DoTagTest;
 import com.hubspot.jinjava.lib.tag.Tag;
 import com.hubspot.jinjava.mode.EagerExecutionMode;
@@ -65,8 +67,11 @@ public void itLimitsLength() {
     }
     context.setDeferredExecutionMode(true);
     interpreter.render(String.format("{%% do deferred.append('%s') %%}", tooLong));
-    assertThat(interpreter.getErrors()).hasSize(1);
-    assertThat(interpreter.getErrors().get(0).getReason())
-      .isEqualTo(ErrorReason.OUTPUT_TOO_BIG);
+    assertThat(interpreter.getErrors())
+      .extracting(TemplateError::getReason)
+      .containsExactly(ErrorReason.EXCEPTION, ErrorReason.OUTPUT_TOO_BIG);
+    assertThat(interpreter.getErrors().get(0).getSeverity()).isEqualTo(ErrorType.FATAL);
+    assertThat(interpreter.getErrors().get(0).getMessage())
+      .contains("Can not convert deferred result to string");
   }
 }

src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagTest.java

@@ -6,7 +6,9 @@
 import com.hubspot.jinjava.ExpectedNodeInterpreter;
 import com.hubspot.jinjava.interpret.DeferredValue;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
+import com.hubspot.jinjava.interpret.TemplateError;
 import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
+import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
 import com.hubspot.jinjava.lib.tag.SetTagTest;
 import com.hubspot.jinjava.mode.EagerExecutionMode;
 import com.hubspot.jinjava.objects.Namespace;
@@ -165,9 +167,12 @@ public void itLimitsLength() {
     }
     context.setDeferredExecutionMode(true);
     interpreter.render(String.format("{%% set deferred = '%s' %%}", tooLong));
-    assertThat(interpreter.getErrors()).hasSize(1);
-    assertThat(interpreter.getErrors().get(0).getReason())
-      .isEqualTo(ErrorReason.OUTPUT_TOO_BIG);
+    assertThat(interpreter.getErrors())
+      .extracting(TemplateError::getReason)
+      .containsExactly(ErrorReason.EXCEPTION, ErrorReason.OUTPUT_TOO_BIG);
+    assertThat(interpreter.getErrors().get(0).getSeverity()).isEqualTo(ErrorType.FATAL);
+    assertThat(interpreter.getErrors().get(0).getMessage())
+      .contains("Output too big for eager execution");
   }
 
   @Test