-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Catch InputCoercionException thrown by Jackson parser #57287
Conversation
Pinging @elastic/es-search (:Search/Mapping) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left a small question around potentially moving the test elsewhere, but feel free to decide.
import org.elasticsearch.rest.RestStatus; | ||
import org.elasticsearch.test.ESSingleNodeTestCase; | ||
|
||
public class LongIndexingDocTests extends ESSingleNodeTestCase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Since this is testing NumberFieldMapper behaviour, would it make sense to put this test case in NumberFieldMapperTests
instead? We wouldn't need a new isolated test class with all the setup/teardown overhead for it in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can only reproduce if the test is of the type ESSingleNodeTestCase
which is not the case for NumberFieldMapperTests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange, from my IDE it looked like it extends "ESSingleNodeTestCase". Let me try again...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me :scratcheshead: I moved your test over and only had to change the index name since the "test" index seems to be in use elsewhere already. Failed when removing the new exception type from the catch statement, passes otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are totally right, I have moved the test so we don't add a new whole class. Thanks @cbuescher!
…elastic#57330) Jackson 2.10 library has added a new type of error that is thrown when a numeric value is out of range. This error should be catch and handle properly in case the flag ignore_malformed has been set to true.
…elastic#57330) Jackson 2.10 library has added a new type of error that is thrown when a numeric value is out of range. This error should be catch and handle properly in case the flag ignore_malformed has been set to true.
Jackson 2.10 library has added a new type of error that is thrown when a numeric value is out of range. This error should be catched and handle properly in case the flag
ignore_malformed
has been set to true.fixes #57272