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

Missing scroll ID no longer returns exception #5729

Closed
clintongormley opened this issue Apr 8, 2014 · 4 comments · Fixed by #6040
Closed

Missing scroll ID no longer returns exception #5729

clintongormley opened this issue Apr 8, 2014 · 4 comments · Fixed by #6040

Comments

@clintongormley
Copy link

As of commit 705c7e2 running a scroll request on a bad scroll ID no longer returns a 500 request error. Instead, each shard returns a failure but the overall request is a 200 OK.

/cc @kimchy

@Mpdreamz
Copy link
Member

Mpdreamz commented Apr 8, 2014

@clintongormley wondering why we don't respond with a 400 or even a 404 when the scroll id does not exist (contains out of date information) ?

@clintongormley
Copy link
Author

We used to respond with a 500, but maybe a 404 would be more RESTful

@Mpdreamz
Copy link
Member

Mpdreamz commented Apr 8, 2014

+1 on the 404 or 400.

@pickypg
Copy link
Member

pickypg commented Apr 11, 2014

500 represents an internal server error, so anything in the 400 range (404 in particular representing the resource was not found) would be a lot better, thus indicating to the end user that the problem was on their end (input), rather than putting the blame onto Elasticsearch.

So +1 to HTTP 404

@s1monw s1monw added v2.0.0 and removed v1.0.3 labels Apr 15, 2014
spinscale added a commit to spinscale/elasticsearch that referenced this issue May 5, 2014
A bad/non-existing scroll ID used to return a 200, however a 404 might be more useful.
Also, this PR returns the right Exception (SearchContextMissingException) in the Java API.

Additionally: Added StatusToXContent interface and RestStatusToXContentListener listener, so
the appropriate RestStatus can be returned

Closes elastic#5729
spinscale added a commit that referenced this issue May 5, 2014
A bad/non-existing scroll ID used to return a 200, however a 404 might be more useful.
Also, this PR returns the right Exception (SearchContextMissingException) in the Java API.

Additionally: Added StatusToXContent interface and RestStatusToXContentListener listener, so
the appropriate RestStatus can be returned

Closes #5729
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants