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 JSON in Console causes confusing response #10504

Closed
colings86 opened this issue Feb 22, 2017 · 7 comments
Closed

Invalid JSON in Console causes confusing response #10504

colings86 opened this issue Feb 22, 2017 · 7 comments
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Console Dev Tools Console Feature Feature:Dev Tools

Comments

@colings86
Copy link
Contributor

Steps to reproduce:

From Console run the following:

DELETE test

POST test/doc/1
{
  "s": "some value"
}

GET test/_search
{
  "_source": {
    "include": ["s", "a"],
    "exclude": "t"
  }
  "query": {
    "match": {
      "s": "value"
    }
  }
}

Note that there is a comma missing in the search request

Response:

{
  "error": "Content-Type header [text/plain] is not supported",
  "status": 406
}

Whilst doing the request directly to ES using cURL gives:

$ curl -XGET "http://localhost:9200/test/_search" -H 'Content-Type: application/json' -d'
{
  "_source": {
    "include": ["s", "a"],
    "exclude": "t"
  }
  "query": {
    "match": {
      "s": "value"
    }
  }
}'
{"error":{"root_cause":[{"type":"json_parse_exception","reason":"Unexpected character ('\"' (code 34)): was expecting comma to separate Object entries\n at [Source: org.elasticsearch.transport.netty4.ByteBufStreamInput@7d758e66; line: 7, column: 4]"}],"type":"json_parse_exception","reason":"Unexpected character ('\"' (code 34)): was expecting comma to separate Object entries\n at [Source: org.elasticsearch.transport.netty4.ByteBufStreamInput@7d758e66; line: 7, column: 4]"},"status":500}%

It seems that if the JSON is invalid we send it as plain text. It would be better if we just send the request as JSON and show the above error (the cURL one), since the user may not have seen that they have an error on the request side.

@epixa
Copy link
Contributor

epixa commented Feb 23, 2017

What version of Kibana are you using? We recently changed the content-type handling in console, and it should never be sending as text/plain.

@epixa
Copy link
Contributor

epixa commented Feb 23, 2017

Sorry, that's not correct. I see there is still a potential fallback to text/plain even in the latest.

@epixa epixa added Feature:Dev Tools bug Fixes for quality problems that affect the customer experience Feature:Console Dev Tools Console Feature and removed feedback_needed labels Feb 23, 2017
@epixa
Copy link
Contributor

epixa commented Feb 23, 2017

@jbudz Can you think of any reason why we'd want to send anything as text/plain now in Console? I think we can avoid this issue if we our logic is simply: Use application/json if it is valid JSON, otherwise use application/x-ndjson.

@jbudz
Copy link
Member

jbudz commented Feb 23, 2017

I don't feel strongly about it, but the original reason was to avoid errors from any in between servers parsing the body based on content type. /cc @spalger

@jbudz jbudz self-assigned this Feb 23, 2017
@colings86
Copy link
Contributor Author

Use application/json if it is valid JSON, otherwise use application/x-ndjson.

Could we not always use application/json and then special case the msearch/bulk endpoints to use application/x-ndjson since they are the only two APIs that do not accept pure JSON?

@epixa
Copy link
Contributor

epixa commented Feb 23, 2017

I wish there was a future proof way of determining the encoding on the fly so that we don't need to worry about potential future mismatches in requirements between Elasticsearch and Kibana console, but clearly it's fragile one way or another, so leaning on the msearch/bulk endpoints may be the best option.

@spalger
Copy link
Contributor

spalger commented Feb 24, 2017

We could also retry requests (with a different content-type) that are rejected based on content type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Console Dev Tools Console Feature Feature:Dev Tools
Projects
None yet
Development

No branches or pull requests

4 participants