Skip to content

Commit 11926b5

Browse files
jasmith-hsjasmith-hs
authored
Merge pull request #1196 from HubSpot/ignore-parse-errors
Add feature to ignore parsing errors when doing nested interpretation
2 parents 50f8d2f + 447fcfd commit 11926b5

7 files changed

Lines changed: 238 additions & 68 deletions

File tree

‎src/main/java/com/hubspot/jinjava/interpret/Context.java‎

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.common.collect.ImmutableSet;
2222
import com.google.common.collect.SetMultimap;
2323
import com.google.common.collect.Sets;
24+
import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF.TemplateErrorTypeHandlingStrategy;
2425
import com.hubspot.jinjava.lib.Importable;
2526
import com.hubspot.jinjava.lib.expression.ExpressionStrategy;
2627
import com.hubspot.jinjava.lib.exptest.ExpTest;
@@ -769,13 +770,64 @@ public TemporaryValueClosable<Boolean> withDeferLargeObjects(
769770
return temporaryValueClosable;
770771
}
771772

773+
@Deprecated
772774
public boolean getThrowInterpreterErrors() {
773-
return contextConfiguration.isThrowInterpreterErrors();
775+
ErrorHandlingStrategy errorHandlingStrategy = getErrorHandlingStrategy();
776+
return (
777+
errorHandlingStrategy.getFatalErrorStrategy() ==
778+
TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION
779+
);
774780
}
775781

782+
@Deprecated
776783
public void setThrowInterpreterErrors(boolean throwInterpreterErrors) {
777784
contextConfiguration =
778-
contextConfiguration.withThrowInterpreterErrors(throwInterpreterErrors);
785+
contextConfiguration.withErrorHandlingStrategy(
786+
ErrorHandlingStrategy
787+
.builder()
788+
.setFatalErrorStrategy(
789+
throwInterpreterErrors
790+
? TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION
791+
: TemplateErrorTypeHandlingStrategy.ADD_ERROR
792+
)
793+
.setNonFatalErrorStrategy(
794+
throwInterpreterErrors
795+
? TemplateErrorTypeHandlingStrategy.IGNORE // Deprecated, warnings are ignored when doing eager expression resolving
796+
: TemplateErrorTypeHandlingStrategy.ADD_ERROR
797+
)
798+
.build()
799+
);
800+
}
801+
802+
@Deprecated
803+
public TemporaryValueClosable<Boolean> withThrowInterpreterErrors() {
804+
TemporaryValueClosable<Boolean> temporaryValueClosable = new TemporaryValueClosable<>(
805+
getThrowInterpreterErrors(),
806+
this::setThrowInterpreterErrors
807+
);
808+
setThrowInterpreterErrors(true);
809+
return temporaryValueClosable;
810+
}
811+
812+
public ErrorHandlingStrategy getErrorHandlingStrategy() {
813+
return contextConfiguration.getErrorHandlingStrategy();
814+
}
815+
816+
public void setErrorHandlingStrategy(ErrorHandlingStrategy errorHandlingStrategy) {
817+
contextConfiguration =
818+
contextConfiguration.withErrorHandlingStrategy(errorHandlingStrategy);
819+
}
820+
821+
public TemporaryValueClosable<ErrorHandlingStrategy> withErrorHandlingStrategy(
822+
ErrorHandlingStrategy errorHandlingStrategy
823+
) {
824+
TemporaryValueClosable<ErrorHandlingStrategy> temporaryValueClosable =
825+
new TemporaryValueClosable<>(
826+
getErrorHandlingStrategy(),
827+
this::setErrorHandlingStrategy
828+
);
829+
setErrorHandlingStrategy(errorHandlingStrategy);
830+
return temporaryValueClosable;
779831
}
780832

781833
public boolean isPartialMacroEvaluation() {
@@ -829,10 +881,26 @@ private TemporaryValueClosable(T previousValue, Consumer<T> resetValueConsumer)
829881
this.resetValueConsumer = resetValueConsumer;
830882
}
831883

884+
public static <T> TemporaryValueClosable<T> noOp() {
885+
return new NoOpTemporaryValueClosable<>();
886+
}
887+
832888
@Override
833889
public void close() {
834890
resetValueConsumer.accept(previousValue);
835891
}
892+
893+
private static class NoOpTemporaryValueClosable<T> extends TemporaryValueClosable<T> {
894+
895+
private NoOpTemporaryValueClosable() {
896+
super(null, null);
897+
}
898+
899+
@Override
900+
public void close() {
901+
// No-op
902+
}
903+
}
836904
}
837905

838906
public Node getCurrentNode() {

‎src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,51 @@ default boolean isDeferLargeObjects() {
3434
}
3535

3636
@Default
37-
default boolean isThrowInterpreterErrors() {
37+
default boolean isPartialMacroEvaluation() {
3838
return false;
3939
}
4040

4141
@Default
42-
default boolean isPartialMacroEvaluation() {
42+
default boolean isUnwrapRawOverride() {
4343
return false;
4444
}
4545

4646
@Default
47-
default boolean isUnwrapRawOverride() {
48-
return false;
47+
default ErrorHandlingStrategy getErrorHandlingStrategy() {
48+
return ErrorHandlingStrategy.of();
49+
}
50+
51+
@Immutable(singleton = true)
52+
@HubSpotImmutableStyle
53+
interface ErrorHandlingStrategyIF {
54+
@Default
55+
default TemplateErrorTypeHandlingStrategy getFatalErrorStrategy() {
56+
return TemplateErrorTypeHandlingStrategy.ADD_ERROR;
57+
}
58+
59+
@Default
60+
default TemplateErrorTypeHandlingStrategy getNonFatalErrorStrategy() {
61+
return TemplateErrorTypeHandlingStrategy.ADD_ERROR;
62+
}
63+
64+
enum TemplateErrorTypeHandlingStrategy {
65+
IGNORE,
66+
ADD_ERROR,
67+
THROW_EXCEPTION,
68+
}
69+
70+
static ErrorHandlingStrategy throwAll() {
71+
return ErrorHandlingStrategy
72+
.of()
73+
.withFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION)
74+
.withNonFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION);
75+
}
76+
77+
static ErrorHandlingStrategy ignoreAll() {
78+
return ErrorHandlingStrategy
79+
.of()
80+
.withFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.IGNORE)
81+
.withNonFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.IGNORE);
82+
}
4983
}
5084
}

‎src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java‎

Lines changed: 62 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
import com.hubspot.jinjava.el.ExpressionResolver;
3030
import com.hubspot.jinjava.el.ext.DeferredParsingException;
3131
import com.hubspot.jinjava.el.ext.ExtendedParser;
32+
import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable;
33+
import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF;
34+
import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF.TemplateErrorTypeHandlingStrategy;
3235
import com.hubspot.jinjava.interpret.TemplateError.ErrorItem;
3336
import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
3437
import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
@@ -83,6 +86,8 @@ public class JinjavaInterpreter implements PyishSerializable {
8386

8487
public static final String OUTPUT_UNDEFINED_VARIABLES_ERROR =
8588
"OUTPUT_UNDEFINED_VARIABLES_ERROR";
89+
public static final String IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS =
90+
"IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS";
8691
private final Multimap<String, BlockInfo> blocks = ArrayListMultimap.create();
8792
private final LinkedList<Node> extendParentRoots = new LinkedList<>();
8893
private final Map<String, RevertibleObject> revertibleObjects = new HashMap<>();
@@ -265,13 +270,30 @@ public String renderFlat(String template, long renderLimit) {
265270
return template;
266271
} else {
267272
context.setRenderDepth(depth + 1);
268-
return render(parse(template), false, renderLimit);
273+
Node parsedNode;
274+
try (
275+
TemporaryValueClosable<ErrorHandlingStrategy> c = ignoreParseErrorsIfActivated()
276+
) {
277+
parsedNode = parse(template);
278+
}
279+
return render(parsedNode, false, renderLimit);
269280
}
270281
} finally {
271282
context.setRenderDepth(depth);
272283
}
273284
}
274285

286+
private TemporaryValueClosable<ErrorHandlingStrategy> ignoreParseErrorsIfActivated() {
287+
return config
288+
.getFeatures()
289+
.getActivationStrategy(
290+
JinjavaInterpreter.IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS
291+
)
292+
.isActive(context)
293+
? context.withErrorHandlingStrategy(ErrorHandlingStrategyIF.ignoreAll())
294+
: TemporaryValueClosable.noOp();
295+
}
296+
275297
/**
276298
* Parse the given string into a root Node, and then renders it processing extend parents.
277299
*
@@ -812,46 +834,51 @@ public void addError(TemplateError templateError) {
812834
if (templateError == null) {
813835
return;
814836
}
815-
816-
if (context.getThrowInterpreterErrors()) {
817-
if (templateError.getSeverity() == ErrorType.FATAL) {
818-
// Throw fatal errors when locating deferred words.
837+
ErrorHandlingStrategy errorHandlingStrategy = context.getErrorHandlingStrategy();
838+
TemplateErrorTypeHandlingStrategy errorTypeHandlingStrategy =
839+
templateError.getSeverity() == ErrorType.FATAL
840+
? errorHandlingStrategy.getFatalErrorStrategy()
841+
: errorHandlingStrategy.getNonFatalErrorStrategy();
842+
switch (errorTypeHandlingStrategy) {
843+
case IGNORE:
844+
return;
845+
case THROW_EXCEPTION:
819846
throw new TemplateSyntaxException(
820847
this,
821848
templateError.getFieldName(),
822849
templateError.getMessage()
823850
);
824-
} else {
825-
// Hide warning errors when locating deferred words.
826-
return;
827-
}
828-
}
829-
// fix line numbers not matching up with source template
830-
if (!context.getCurrentPathStack().isEmpty()) {
831-
if (
832-
!templateError.getSourceTemplate().isPresent() &&
833-
context.getCurrentPathStack().peek().isPresent()
834-
) {
835-
templateError.setMessage(
836-
getWrappedErrorMessage(
837-
context.getCurrentPathStack().peek().get(),
838-
templateError
839-
)
840-
);
841-
templateError.setSourceTemplate(context.getCurrentPathStack().peek().get());
842-
}
843-
templateError.setStartPosition(context.getCurrentPathStack().getTopStartPosition());
844-
templateError.setLineno(context.getCurrentPathStack().getTopLineNumber());
845-
}
851+
case ADD_ERROR:
852+
default: // Checkstyle
853+
// fix line numbers not matching up with source template
854+
if (!context.getCurrentPathStack().isEmpty()) {
855+
if (
856+
!templateError.getSourceTemplate().isPresent() &&
857+
context.getCurrentPathStack().peek().isPresent()
858+
) {
859+
templateError.setMessage(
860+
getWrappedErrorMessage(
861+
context.getCurrentPathStack().peek().get(),
862+
templateError
863+
)
864+
);
865+
templateError.setSourceTemplate(context.getCurrentPathStack().peek().get());
866+
}
867+
templateError.setStartPosition(
868+
context.getCurrentPathStack().getTopStartPosition()
869+
);
870+
templateError.setLineno(context.getCurrentPathStack().getTopLineNumber());
871+
}
846872

847-
// Limit the number of errors and filter duplicates
848-
if (errors.size() < MAX_ERROR_SIZE) {
849-
templateError = templateError.withScopeDepth(scopeDepth);
850-
int errorCode = templateError.hashCode();
851-
if (!errorSet.contains(errorCode)) {
852-
this.errors.add(templateError);
853-
this.errorSet.add(errorCode);
854-
}
873+
// Limit the number of errors and filter duplicates
874+
if (errors.size() < MAX_ERROR_SIZE) {
875+
templateError = templateError.withScopeDepth(scopeDepth);
876+
int errorCode = templateError.hashCode();
877+
if (!errorSet.contains(errorCode)) {
878+
this.errors.add(templateError);
879+
this.errorSet.add(errorCode);
880+
}
881+
}
855882
}
856883
}
857884

‎src/main/java/com/hubspot/jinjava/lib/expression/EagerExpressionStrategy.java‎

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
import com.google.common.annotations.Beta;
44
import com.hubspot.jinjava.JinjavaConfig;
5+
import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable;
6+
import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF;
7+
import com.hubspot.jinjava.interpret.ErrorHandlingStrategy;
58
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
6-
import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
9+
import com.hubspot.jinjava.interpret.TemplateSyntaxException;
710
import com.hubspot.jinjava.lib.filter.EscapeFilter;
811
import com.hubspot.jinjava.lib.tag.eager.DeferredToken;
912
import com.hubspot.jinjava.lib.tag.eager.EagerExecutionResult;
@@ -14,7 +17,6 @@
1417
import com.hubspot.jinjava.util.EagerReconstructionUtils;
1518
import com.hubspot.jinjava.util.Logging;
1619
import com.hubspot.jinjava.util.PrefixToPreserveState;
17-
import java.util.Objects;
1820
import org.apache.commons.lang3.StringUtils;
1921

2022
@Beta
@@ -103,17 +105,20 @@ public static String postProcessResult(
103105
StringUtils.contains(result, master.getSymbols().getExpressionStartWithTag()))
104106
) {
105107
if (interpreter.getConfig().isNestedInterpretationEnabled()) {
106-
long errorSizeStart = getParsingErrorsCount(interpreter);
107-
108-
interpreter.parse(result);
109-
110-
if (getParsingErrorsCount(interpreter) == errorSizeStart) {
108+
try {
109+
try (
110+
TemporaryValueClosable<ErrorHandlingStrategy> c = interpreter
111+
.getContext()
112+
.withErrorHandlingStrategy(ErrorHandlingStrategyIF.throwAll())
113+
) {
114+
interpreter.parse(result);
115+
}
111116
try {
112117
result = interpreter.renderFlat(result);
113118
} catch (Exception e) {
114119
Logging.ENGINE_LOG.warn("Error rendering variable node result", e);
115120
}
116-
}
121+
} catch (TemplateSyntaxException ignored) {}
117122
} else {
118123
result = EagerReconstructionUtils.wrapInRawIfNeeded(result, interpreter);
119124
}
@@ -125,18 +130,6 @@ public static String postProcessResult(
125130
return result;
126131
}
127132

128-
private static long getParsingErrorsCount(JinjavaInterpreter interpreter) {
129-
return interpreter
130-
.getErrors()
131-
.stream()
132-
.filter(Objects::nonNull)
133-
.filter(error ->
134-
"Unclosed comment".equals(error.getMessage()) ||
135-
error.getReason() == ErrorReason.DISABLED
136-
)
137-
.count();
138-
}
139-
140133
private static String wrapInExpression(String output, JinjavaInterpreter interpreter) {
141134
JinjavaConfig config = interpreter.getConfig();
142135
return String.format(

0 commit comments

Comments
 (0)