JEXLMap evaluateExpression's un-overridable default behavior restricts user's options.

[SHuang-Broad]

Verify

Can you see anything in the logs?
Make sure your issue is not already in the htsjdk issue tracker

Answer: At the time of ticket opening (2016-09-06), there's another fix that tries to remedy a related problem. But that fix doesn't change any logic, so couldn't solve this problem.

Subject of the issue

Summary: JEXLMap decides for user that records with missing annotation, by which the expression works, is associated with a false value.
See below for code block causing the bug.
Currently, one of the use case is in GATK's VariantFiltration tool, which filters variants and/or genotypes/samples based on user provided cmd line filter expressions.
In particular, the expected default behavior is that variants or genotypes which has missing data for the specific annotation the filter works on should be marked as PASSing unless user explicitly turn on a flag missingValuesInExpressionsShouldEvaluateAsFailing so that such variants or genotypes will be marked as FAILing.

Your environment

• version of htsjdk 2.6.1

Steps to reproduce

See code block below. In particular, the exception handling part when the error message contains a substring "undefined variable".

JEXLMap.java.evaluateExpression() lines 126-137
try {
    final Boolean value = (Boolean) exp.exp.evaluate(jContext);
    // treat errors as no match
    jexl.put(exp, value == null ? false : value);
} catch (Exception e) {
    // if exception happens because variable is undefined (i.e. field in expression is not present), evaluate to FALSE
    // todo - might be safer if we explicitly checked for an exception type, but Apache's API doesn't seem to have that ability
    if (e.getMessage() != null && e.getMessage().contains("undefined variable"))
        jexl.put(exp,false);
    else
        throw new IllegalArgumentException(String.format("Invalid JEXL expression detected for %s with message %s", exp.name, (e.getMessage() == null ? "no message" : e.getMessage())));
}

Expected behaviour

Users should be given the option to handle exceptions, based on the type of exception thrown.
This requires better documentation on involved classes and methods.

Actual behaviour

For any record that misses the annotation that the expression acts on, the library decides for the user that the record should evaluate to false. And user has no way of knowing the cause.

[lbergelson]

solved by #772