rethrow script compilation exceptions into ingest configuration exceptions

[talevy]

Currently, mustache compile exceptions in pipelines are reported as regular ES exceptions, e.g.

{
  "error": {
    "root_cause": [
      {
        "type": "general_script_exception",
        "reason": "Failed to compile inline script [{{#join}}{{/join}}] using lang [mustache]"
      }
    ],
    "type": "general_script_exception",
    "reason": "Failed to compile inline script [{{#join}}{{/join}}] using lang [mustache]",
    "caused_by": {
      "type": "mustache_exception",
      "reason": "Mustache function [join] must contain one and only one identifier"
    }
  },
  "status": 500
}

This makes it difficult to gather which property in the pipeline configuration is causing this exception. This PR wraps these exceptions in ElasticsearchParseException so that we can relay the information

[rmuir]

Why lose the original exception?

[jdconrad]

Please don't lose the original exception. It's already difficult enough to debug script exceptions without them being swallowed.

[talevy]

Ok, thought I could re-use some headers configuration stuff and enough information would make its way through the current exception's message

[talevy]

@rmuir @jdconrad updated the ElasticsearchParseException to carry the original cause with it. thanks!

[martijnvg]

LGTM

[jasontedor]

If I understand this code path correctly, this converts every exception thrown during template compilation into an ElasticsearchParseException and I'm not comfortable with that. For example, it means bugs in compilation get converted into a parse exception that sounds like the user can correct? What's the reasoning here?

[talevy]

we've been using ElasticsearchParseException as the exception container for all our pipeline build-time configuration exceptions.

it means bugs in compilation get converted into a parse exception that sounds like the user can correct

why would the user not be able to correct these exceptions?

[jasontedor]

why would the user not be able to correct these exceptions?

Because I'm specifically referring to exceptions being thrown because of bugs in the compilation explicitly not due to the user's template.

[talevy]

I see, I guess I am not familiar with such exceptions. If all such exceptions extend ElasticsearchException then I suppose there is no real need to wrap the exception, but instead just add the appropriate ingest headers. So, I can check for whether an ElasticsearchException is thrown and simply add the headers and pass it along. If other exceptions are thrown, I would need to wrap it in some sort of ElasticsearchException so that I can relay the proper headers to the user. what do you think?

[jasontedor]

We can't safely say that all such exceptions will extend ElasticsearchException (e.g., a bad NullPointerException), but I like your idea of wrapping the ones that do not extend (as long as it's not wrapping it in an exception that sounds like the user can do something about it).

[martijnvg]

@jasontedor Would using ExceptionsHelper#convertToElastic(...) helper method in ConfigurationUtils#newConfigurationException(...) or similar here be sufficient?

I think the excepting being thrown if the mustache template couldn't be compiled is a MustacheException, which gets wrapped by GeneralScriptException in ScriptService#compileInternal(...), which is what also should be fixed. Instead the lang-mustache should catch MustacheException exceptions with ScriptException.

[jasontedor]

Would using ExceptionsHelper#convertToElastic(...) helper method in ConfigurationUtils#newConfigurationException(...) or similar here be sufficient?

+1

[talevy]

I'll update to use this helper. didn't know about it