search

elastic / elasticsearch

Free and Open, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
68.51k stars 24.33k forks source link

`200` response with even though response body is error #41434

Open danielsnider opened 5 years ago

danielsnider commented 5 years ago

I am doing a string query POST with intentional syntax error "heart(":

Full POST request: 

Request URL: http://192.168.136.128:9200/image/_msearch?
Request Method: POST
Status Code: 200 OK
Remote Address: 192.168.136.128:9200
Referrer Policy: no-referrer-when-downgrade
accept: application/json
content-type: application/x-ndjson
Origin: http://192.168.136.128:3000
Referer: http://192.168.136.128:3000/
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3683.103 Safari/537.36
x-requested-with: 771100

{"preference":"modality-list"}
{"query":{"bool":{"must":[{"bool":{"must":[{"query_string":{"fields":["*"],"query":"heart("}}]}}]}},"highlight":{"pre_tags":["<mark>"],"post_tags":["</mark>"],"fields":{}},"size":0,"_source":{"includes":["*"],"excludes":[]},"from":0,"aggs":{"Modality.raw":{"terms":{"field":"Modality.raw","size":20,"order":{"_count":"desc"}}}}}
{"preference":"bodypart-list"}
{"query":{"bool":{"must":[{"bool":{"must":[{"query_string":{"fields":["*"],"query":"heart("}}]}}]}},"highlight":{"pre_tags":["<mark>"],"post_tags":["</mark>"],"fields":{}},"size":0,"_source":{"includes":["*"],"excludes":[]},"from":0,"aggs":{"BodyPartExamined.raw":{"terms":{"field":"BodyPartExamined.raw","size":100,"order":{"_count":"desc"}}}}}
{"preference":"gender-list"}
{"query":{"bool":{"must":[{"bool":{"must":[{"query_string":{"fields":["*"],"query":"heart("}}]}}]}},"highlight":{"pre_tags":["<mark>"],"post_tags":["</mark>"],"fields":{}},"size":0,"_source":{"includes":["*"],"excludes":[]},"from":0,"aggs":{"PatientSex.raw":{"terms":{"field":"PatientSex.raw","size":20,"order":{"_count":"desc"}}}}}
{"preference":"tagCloud"}
{"query":{"bool":{"must":[{"bool":{"must":[{"query_string":{"fields":["*"],"query":"heart("}}]}}]}},"highlight":{"pre_tags":["<mark>"],"post_tags":["</mark>"],"fields":{}},"size":0,"_source":{"includes":["*"],"excludes":[]},"from":0,"aggs":{"descriptions.raw":{"terms":{"field":"descriptions.raw","size":200,"order":{"_term":"asc"}}}}}
{"preference":"results"}
{"query":{"bool":{"must":[{"bool":{"must":[{"query_string":{"fields":["*"],"query":"heart("}}]}}]}},"highlight":{"pre_tags":["<mark>"],"post_tags":["</mark>"],"fields":{}},"size":12,"_source":{"includes":["*"],"excludes":[]},"from":0}

Full 200 response:

access-control-allow-credentials: true
access-control-allow-origin: *
content-encoding: gzip
content-length: 538
content-type: application/json; charset=UTF-8
Warning: 299 Elasticsearch-6.7.1-2f32220 "Deprecated aggregation order key [_term] used, replaced by [_key]"

{"responses":[{"error":{"root_cause":[{"type":"query_shard_exception","reason":"Failed to parse query [heart(]","index_uuid":"FqZfR415QIuajKOY32gXYg","index":"image"}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"image","node":"X6uClAg5SimP2fxx3L872g","reason":{"type":"query_shard_exception","reason":"Failed to parse query [heart(]","index_uuid":"FqZfR415QIuajKOY32gXYg","index":"image","caused_by":{"type":"parse_exception","reason":"Cannot parse 'heart(': Encountered \"<EOF>\" at line 1, column 6.\nWas expecting one of:\n    <NOT> ...\n    \"+\" ...\n    \"-\" ...\n    <BAREOPER> ...\n    \"(\" ...\n    \"*\" ...\n    <QUOTED> ...\n    <TERM> ...\n    <PREFIXTERM> ...\n    <WILDTERM> ...\n    <REGEXPTERM> ...\n    \"[\" ...\n    \"{\" ...\n    <NUMBER> ...\n    <TERM> ...\n    ","caused_by":{"type":"parse_exception","reason":"Encountered \"<EOF>\" at line 1, column 6.\nWas expecting one of:\n    <NOT> ...\n    \"+\" ...\n    \"-\" ...\n    <BAREOPER> ...\n    \"(\" ...\n    \"*\" ...\n    <QUOTED> ...\n    <TERM> ...\n    <PREFIXTERM> ...\n    <WILDTERM> ...\n    <REGEXPTERM> ...\n    \"[\" ...\n    \"{\" ...\n    <NUMBER> ...\n    <TERM> ...\n    "}}}}]},"status":400},{"error":{"root_cause":[{"type":"query_shard_exception","reason":"Failed to parse query [heart(]","index_uuid":"FqZfR415QIuajKOY32gXYg","index":"image"}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"image","node":"X6uClAg5SimP2fxx3L872g","reason":{"type":"query_shard_exception","reason":"Failed to parse query [heart(]","index_uuid":"FqZfR415QIuajKOY32gXYg","index":"image","caused_by":{"type":"parse_exception","reason":"Cannot parse 'heart(': Encountered \"<EOF>\" at line 1, column 6.\nWas expecting one of:\n    <NOT> ...\n    \"+\" ...\n    \"-\" ...\n    <BAREOPER> ...\n    \"(\" ...\n    \"*\" ...\n    <QUOTED> ...\n    <TERM> ...\n    <PREFIXTERM> ...\n    <WILDTERM> ...\n    <REGEXPTERM> ...\n    \"[\" ...\n    \"{\" ...\n    <NUMBER> ...\n    <TERM> ...\n    ","caused_by":{"type":"parse_exception","reason":"Encountered \"<EOF>\" at line 1, column 6.\nWas expecting one of:\n    <NOT> ...\n    \"+\" ...\n    \"-\" ...\n    <BAREOPER> ...\n    \"(\" ...\n    \"*\" ...\n    <QUOTED> ...\n    <TERM> ...\n    <PREFIXTERM> ...\n    <WILDTERM> ...\n    <REGEXPTERM> ...\n    \"[\" ...\n    \"{\" ...\n    <NUMBER> ...\n    <TERM> ...\n    "}}}}]},"status":400},{"error":{"root_cause":[{"type":"query_shard_exception","reason":"Failed to parse query [heart(]","index_uuid":"FqZfR415QIuajKOY32gXYg","index":"image"}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"image","node":"X6uClAg5SimP2fxx3L872g","reason":{"type":"query_shard_exception","reason":"Failed to parse query [heart(]","index_uuid":"FqZfR415QIuajKOY32gXYg","index":"image","caused_by":{"type":"parse_exception","reason":"Cannot parse 'heart(': Encountered \"<EOF>\" at line 1, column 6.\nWas expecting one of:\n    <NOT> ...\n    \"+\" ...\n    \"-\" ...\n    <BAREOPER> ...\n    \"(\" ...\n    \"*\" ...\n    <QUOTED> ...\n    <TERM> ...\n    <PREFIXTERM> ...\n    <WILDTERM> ...\n    <REGEXPTERM> ...\n    \"[\" ...\n    \"{\" ...\n    <NUMBER> ...\n    <TERM> ...\n    ","caused_by":{"type":"parse_exception","reason":"Encountered \"<EOF>\" at line 1, column 6.\nWas expecting one of:\n    <NOT> ...\n    \"+\" ...\n    \"-\" ...\n    <BAREOPER> ...\n    \"(\" ...\n    \"*\" ...\n    <QUOTED> ...\n    <TERM> ...\n    <PREFIXTERM> ...\n    <WILDTERM> ...\n    <REGEXPTERM> ...\n    \"[\" ...\n    \"{\" ...\n    <NUMBER> ...\n    <TERM> ...\n    "}}}}]},"status":400},{"error":{"root_cause":[{"type":"query_shard_exception","reason":"Failed to parse query [heart(]","index_uuid":"FqZfR415QIuajKOY32gXYg","index":"image"}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"image","node":"X6uClAg5SimP2fxx3L872g","reason":{"type":"query_shard_exception","reason":"Failed to parse query [heart(]","index_uuid":"FqZfR415QIuajKOY32gXYg","index":"image","caused_by":{"type":"parse_exception","reason":"Cannot parse 'heart(': Encountered \"<EOF>\" at line 1, column 6.\nWas expecting one of:\n    <NOT> ...\n    \"+\" ...\n    \"-\" ...\n    <BAREOPER> ...\n    \"(\" ...\n    \"*\" ...\n    <QUOTED> ...\n    <TERM> ...\n    <PREFIXTERM> ...\n    <WILDTERM> ...\n    <REGEXPTERM> ...\n    \"[\" ...\n    \"{\" ...\n    <NUMBER> ...\n    <TERM> ...\n    ","caused_by":{"type":"parse_exception","reason":"Encountered \"<EOF>\" at line 1, column 6.\nWas expecting one of:\n    <NOT> ...\n    \"+\" ...\n    \"-\" ...\n    <BAREOPER> ...\n    \"(\" ...\n    \"*\" ...\n    <QUOTED> ...\n    <TERM> ...\n    <PREFIXTERM> ...\n    <WILDTERM> ...\n    <REGEXPTERM> ...\n    \"[\" ...\n    \"{\" ...\n    <NUMBER> ...\n    <TERM> ...\n    "}}}}]},"status":400},{"error":{"root_cause":[{"type":"query_shard_exception","reason":"Failed to parse query [heart(]","index_uuid":"FqZfR415QIuajKOY32gXYg","index":"image"}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"image","node":"X6uClAg5SimP2fxx3L872g","reason":{"type":"query_shard_exception","reason":"Failed to parse query [heart(]","index_uuid":"FqZfR415QIuajKOY32gXYg","index":"image","caused_by":{"type":"parse_exception","reason":"Cannot parse 'heart(': Encountered \"<EOF>\" at line 1, column 6.\nWas expecting one of:\n    <NOT> ...\n    \"+\" ...\n    \"-\" ...\n    <BAREOPER> ...\n    \"(\" ...\n    \"*\" ...\n    <QUOTED> ...\n    <TERM> ...\n    <PREFIXTERM> ...\n    <WILDTERM> ...\n    <REGEXPTERM> ...\n    \"[\" ...\n    \"{\" ...\n    <NUMBER> ...\n    <TERM> ...\n    ","caused_by":{"type":"parse_exception","reason":"Encountered \"<EOF>\" at line 1, column 6.\nWas expecting one of:\n    <NOT> ...\n    \"+\" ...\n    \"-\" ...\n    <BAREOPER> ...\n    \"(\" ...\n    \"*\" ...\n    <QUOTED> ...\n    <TERM> ...\n    <PREFIXTERM> ...\n    <WILDTERM> ...\n    <REGEXPTERM> ...\n    \"[\" ...\n    \"{\" ...\n    <NUMBER> ...\n    <TERM> ...\n    "}}}}]},"status":400}]}

I receive response 200 with even though the body is a clear error 😢. How can we make the 200 response be the 400 or 500 error that it should (and conform to standard HTTP practice)? Many thanks! This is a major blocker for me. It's causing havoc in my app because errors are silent and that's the worst kind of error!

Standards say that 200 means "the response will contain an entity corresponding to the requested resource" and ElasticSearch is not behaving that way.

danielsnider commented 5 years ago

Related https://github.com/elastic/elasticsearch/issues/29169

elasticmachine commented 5 years ago

Pinging @elastic/es-core-infra

jpountz commented 5 years ago

@danielsnider The bulk and msearch APIs are a bit special due to the fact that they are ways to perform multiple requests in one single request. What is your expectation for the case that some requests succeed but other requests fail?

danielsnider commented 5 years ago

I’d love a 400 level error if some requests fail. A configuration option to respond 400 or 200 would be great. Errors should fail fast and loudly. Silent failure is the worst. The tool I’m using only cares about the HTTP status code (like a lot of tools).

On Apr 24, 2019, at 4:32 PM, Adrien Grand notifications@github.com wrote:

@danielsnider The bulk and msearch APIs are a bit special due to the fact that they are ways to perform multiple requests in one single request. What is your expectation for the case that some requests succeed but other requests fail?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

dakrone commented 5 years ago

We discussed this today among a group of us and brainstormed some ideas.

There isn't as much interest in making a breaking change by changing the response as a breaking change, that's something we'd like to avoid if possible.

What we are interested in doing is this potentially being part of a change to versioned APIs, in particular, once we have versioned APIs we could introduce a different status code.

The status code we are interested in introducing is the 207 Multi-Status response code, from RFC 4918:

The 207 (Multi-Status) status code provides status for multiple independent operations

In particular, from the RFC's implementation guidelines:

A Multi-Status response conveys information about multiple resources in situations where multiple status codes might be appropriate. The default Multi-Status response body is a text/xml or application/xml HTTP entity with a 'multistatus' root element. Further elements contain 200, 300, 400, and 500 series status codes generated during the method invocation.

Although '207' is used as the overall response status code, the recipient needs to consult the contents of the multistatus response body for further information about the success or failure of the method execution. The response MAY be used in success, partial success and also in failure situations.

The 'multistatus' root element holds zero or more 'response' elements in any order, each with information about an individual resource.

While we aren't going to implement the response as an XML response, we could potentially come up with a standardized way of returning a JSON response where separate status responses are captured.

danielsnider commented 5 years ago

Hi @dakrone! That may be a nice solution but a did you discuss adding have configuration option to enabling responding with a top-level 400 level HTTP error? For my system a 207 Multi-Status will probably not be noticed as an error since it's 200 level. So it does not solve the problem of silent errors that I'm having currently with ElasticSearch. Thanks!

danielsnider commented 5 years ago

Hi @dakrone, any thoughts on my last comment? I'm wondering if your team has considered a configuration option to enable responding with a top-level 400 level HTTP error? For my system a 207 Multi-Status will probably not be noticed as an error since it's 200 level.

jasontedor commented 5 years ago

We are unlikely to add a configuration options for this. We prefer to keep the number of configuration options low, since it only adds complexity from a user-facing perspective, and from a maintenance perspective. A configuration option like this would have to be respected on a large number of APIs, old and new and that increases the surface area for bugs.

I think that what is missing for me is that in your example, you have submitted a search request. Presumably you executed this search request to get back a response. The responses object contains the responses to each search request, and you have to parse this for the response to be useful. There is also an errors field there, that you can check quickly whether or not it contains any errors?

danielsnider commented 5 years ago

@jasontedor Thanks a lot for the reply. The reason I'm bugging you and that I cannot check quickly whether or not it contains any errors is because I'm building on someone else's tool (ReactiveSearch, related issue) and so I'd have to dive into their code base an figure out where to put the checking code. I've asked them to handle this and I'll ask again. Maybe it is a quick fix (for them).

jaymode commented 3 years ago

This issue would ultimately be handled as part of implementing #60442.