Skip to content

ESQL: Fail with 500 not 400 for ValueExtractor bugs #126296

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

Conversation

alex-spies
Copy link
Contributor

@alex-spies alex-spies commented Apr 4, 2025

Fix #126198

In case of wrong layouts of ESQL's operators, it can happen that ValueExtractor.extractorFor encounters a data type mismatch. Currently, this throws IllegalArgumentException, which is treated like a user exception and triggers a 400 response.

We need to return a 500 status code for such errors; this is also important for observability of ES clusters, which can normally use 500 responses as an indicator of a bug.

Throw IllegalStateException instead, it's close enough.

In case of wrong layouts of ESQL's operators, it can happen that
ValueExtractor.extractorFor encounters a data type mismatch. Currently,
this throws IllegalArgumentException, which is treated like a user
exception and triggers a 400 response.

We need to return a 500 status code for such errors; this is also
important for observability of ES clusters, which can normally use 500
responses as an indicator of a bug.

Throw IllegalStateException instead, it's close enough.
@alex-spies alex-spies added >bug auto-backport Automatically create backport pull requests when merged :Analytics/ES|QL AKA ESQL v8.19.0 v9.1.0 labels Apr 4, 2025
@alex-spies alex-spies requested a review from nik9000 April 4, 2025 13:41
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Apr 4, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine
Copy link
Collaborator

Hi @alex-spies, I've created a changelog YAML for you.

Copy link
Contributor

@luigidellaquila luigidellaquila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alex-spies
Copy link
Contributor Author

Thanks for the double checking @luigidellaquila and @idegtiarenko !

@alex-spies alex-spies merged commit a152b4e into elastic:main Apr 7, 2025
17 checks passed
@alex-spies alex-spies deleted the throw-500-on-datatype-mismatch-in-value-source-reader branch April 7, 2025 09:22
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

elasticsearchmachine pushed a commit that referenced this pull request Apr 7, 2025
In case of wrong layouts of ESQL's operators, it can happen that
ValueExtractor.extractorFor encounters a data type mismatch. Currently,
this throws IllegalArgumentException, which is treated like a user
exception and triggers a 400 response.

We need to return a 500 status code for such errors; this is also
important for observability of ES clusters, which can normally use 500
responses as an indicator of a bug.

Throw IllegalStateException instead, it's close enough.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESQL: only 400 response on IllegalArgumentException in the compute engine
4 participants