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

clear scroll throws 500 on array out of bounds exception #5730

Closed
Mpdreamz opened this issue Apr 8, 2014 · 7 comments
Closed

clear scroll throws 500 on array out of bounds exception #5730

Mpdreamz opened this issue Apr 8, 2014 · 7 comments
Assignees
Labels

Comments

@Mpdreamz
Copy link
Member

Mpdreamz commented Apr 8, 2014

DELETE http://127.0.0.1:9200/_search/scroll/asdasdadasdasd HTTP/1.1

Returns the following response

HTTP/1.1 500 Internal Server Error
Content-Type: application/json; charset=UTF-8
Content-Length: 73

{
  "error" : "ArrayIndexOutOfBoundsException[1]",
  "status" : 500
}

While a 404 is expected.

It would also be nice if we can allow the scroll id to be posted. I've had people hit problems with scroll ids that are too big in the past:

elastic/elasticsearch-net#318

@martijnvg martijnvg self-assigned this Apr 8, 2014
@martijnvg
Copy link
Member

@Mpdreamz Actually that specific scroll id is malformed and that is where the ArrayIndexOutOfBoundsException comes from, so I think a 400 should be returned?

If a non existent scroll_id is used, it will just return and act like everything is fine. I agree a 404 would be nice.

@clintongormley
Copy link

++404

@Mpdreamz
Copy link
Member Author

Mpdreamz commented Apr 8, 2014

++404 and +1 on implementing #5726 @martijnvg !

@martijnvg
Copy link
Member

PR #5738 only addresses invalid scroll ids. Returning a 404 for a valid, but non existing scroll id requires more work than just validation. The clear scoll api uses an internal free search context api, which for example the search api relies on. This internal api just always returns an empty response. I can change that, so that it includes whether it actually has removed a search context, but that requires a change in the transport layer, so I like to do separate that in a different PR.

@s1monw
Copy link
Contributor

s1monw commented Apr 14, 2014

LGTM

@s1monw s1monw added the bug label Apr 14, 2014
@s1monw
Copy link
Contributor

s1monw commented Apr 14, 2014

@martijnvg can you assign the fix version here please

@martijnvg
Copy link
Member

@s1monw PR #5738 only handles invalid scroll ids, but this issue is also about returning a 404 when a valid scroll id doesn't exist. I will assign the proper versions in PR and leave this issue open, once the missing scroll id has been addressed this issue can be closed.

martijnvg added a commit to martijnvg/elasticsearch that referenced this issue May 8, 2014
s1monw added a commit to s1monw/elasticsearch that referenced this issue May 13, 2014
Since elastic#5730 we write a boolean in the FreeContextResponse which should be deserialized

Closes elastic#6147
s1monw added a commit that referenced this issue May 13, 2014
Since #5730 we write a boolean in the FreeContextResponse which should be deserialized

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

No branches or pull requests

4 participants