Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid root_cause for mustache_exception #12292

Closed
spalger opened this issue Jul 16, 2015 · 4 comments
Closed

Invalid root_cause for mustache_exception #12292

spalger opened this issue Jul 16, 2015 · 4 comments
Labels
>bug :Core/Infra/Core Core issues without another label discuss

Comments

@spalger
Copy link
Contributor

spalger commented Jul 16, 2015

In the JavaScript tests I'm trying to validate the error output when specifying an invalid mustache template, and while the correct error is produced, it is not sent in the API response as the root cause.

@colings86 mentioned this is likely a bug in the exception XContent rendering. (whatever that means 😸)

Here is a trace of the request:

  -> POST http://localhost:9406/_render/template
  {
    "inline": "{ \"query\": { \"match\": { \"text\": \"{{{my_value}}\" } }, \"size\": {{my_size}} }",
    "params": {
      "my_value": "bar",
      "my_size": 100
    }
  }
  <- 500
  {
    "error": {
      "root_cause": [
        {
          "type": "script_exception",
          "reason": "Failed to compile inline script [{ \"query\": { \"match\": { \"text\": \"{{{my_value}}\" } }, \"size\": {{my_size}} }] using lang [mustache]"
        }
      ],
      "type": "script_exception",
      "reason": "Failed to compile inline script [{ \"query\": { \"match\": { \"text\": \"{{{my_value}}\" } }, \"size\": {{my_size}} }] using lang [mustache]",
      "caused_by": {
        "type": "mustache_exception",
        "reason": "Improperly closed variable in query-template:1"
      }
    },
    "status": 500
  }

And the test that produced it:

- do:
catch: /Improperly.closed.variable.in.query-template/
render_search_template:
body: { "inline": { "query": { "match": { "text": "{{{my_value}}" } }, "aggs": { "my_terms": { "terms": { "field": "{{my_field}}" } } } }, "params": { "my_value": "bar", "my_field": "field1" } }

@colings86
Copy link
Contributor

From talking to @clintongormley and @kimchy, the root_cause is actually correct here as it is supposed to stop at the script_exception since the mustache_exception is a implementation detail. The problem here is that there is a difference in implementation between the exception checking on the Java client REST tests and the JS REST tests (and possibly other clients too). The Java client REST tests implementation presumably gets an Exception object (rather than a JSON representation) and will check the messages for all causes (maybe @javanna can confirm that?) whereas the JS Client REST tests implementation checks the rendered JSON and looks only at the root_cause.reason field. So the question is which implementation to change. Should the Java implementation only check the root cause as defined by the ExceptionsHelper? or should the JS implementation (and other clients potentially) be changed to check all reason fields under error?

@colings86
Copy link
Contributor

Looking at DoSection it looks like we take the string value of the error object in the JSON response and try to match it to the regex.

@clintongormley
Copy link
Contributor

@spalger Shouldn't you just be including the rest of the structured data in the exception object that youc reate? and rendering it as part of stringification?

@spalger
Copy link
Contributor Author

spalger commented Jul 17, 2015

Yeah @clintongormley, I'm going to revamp the exception message a bit to include the related issue information so that the test will pass and the errors will be more informative. Thanks all :)

@spalger spalger closed this as completed Jul 17, 2015
@lcawl lcawl added :Core/Infra/Core Core issues without another label and removed :Exceptions labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Core Core issues without another label discuss
Projects
None yet
Development

No branches or pull requests

4 participants