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

200 response with even though response body is error #41434

Open
danielsnider opened this issue Apr 23, 2019 · 12 comments
Open

200 response with even though response body is error #41434

danielsnider opened this issue Apr 23, 2019 · 12 comments
Labels
:Core/Infra/REST API REST infrastructure and utilities >enhancement help wanted adoptme Team:Core/Infra Meta label for core/infra team triaged Issue has been looked at, and is being left open

Comments

@danielsnider
Copy link

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
Copy link
Author

Related #29169

@jkakavas jkakavas added the :Core/Infra/REST API REST infrastructure and utilities label Apr 23, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jpountz
Copy link
Contributor

jpountz commented Apr 24, 2019

@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
Copy link
Author

danielsnider commented Apr 24, 2019 via email

@dakrone
Copy link
Member

dakrone commented Apr 25, 2019

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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

danielsnider commented May 9, 2019

@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).

@dakrone dakrone removed the discuss label May 23, 2019
@rjernst rjernst added the Team:Core/Infra Meta label for core/infra team label May 4, 2020
@rjernst rjernst added the needs:triage Requires assignment of a team area label label Dec 3, 2020
@jaymode
Copy link
Member

jaymode commented Dec 14, 2020

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

@mosche
Copy link
Contributor

mosche commented Dec 13, 2024

Requires more discussion, see #60442

@mosche mosche added the triaged Issue has been looked at, and is being left open label Dec 13, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/REST API REST infrastructure and utilities >enhancement help wanted adoptme Team:Core/Infra Meta label for core/infra team triaged Issue has been looked at, and is being left open
Projects
None yet
Development

No branches or pull requests

10 participants