From 13f4af6dbe333b53d5b59e5bf099f90f909ab627 Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Mon, 7 Apr 2025 11:21:57 +0200 Subject: [PATCH] ESQL: Fail with 500 not 400 for ValueExtractor bugs (#126296) 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. --- docs/changelog/126296.yaml | 5 +++++ .../elasticsearch/compute/operator/topn/ValueExtractor.java | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 docs/changelog/126296.yaml diff --git a/docs/changelog/126296.yaml b/docs/changelog/126296.yaml new file mode 100644 index 0000000000000..55affb6ab4c79 --- /dev/null +++ b/docs/changelog/126296.yaml @@ -0,0 +1,5 @@ +pr: 126296 +summary: Fail with 500 not 400 for `ValueExtractor` bugs +area: ES|QL +type: bug +issues: [] diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ValueExtractor.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ValueExtractor.java index b9336024eb404..40c94ff1327f0 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ValueExtractor.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ValueExtractor.java @@ -26,7 +26,10 @@ interface ValueExtractor { static ValueExtractor extractorFor(ElementType elementType, TopNEncoder encoder, boolean inKey, Block block) { if (false == (elementType == block.elementType() || ElementType.NULL == block.elementType())) { - throw new IllegalArgumentException("Expected [" + elementType + "] but was [" + block.elementType() + "]"); + // While this maybe should be an IllegalArgumentException, it's important to throw an exception that causes a 500 response. + // If we reach here, that's a bug. Arguably, the operators are in an illegal state because the layout doesn't match the + // actual pages. + throw new IllegalStateException("Expected [" + elementType + "] but was [" + block.elementType() + "]"); } return switch (block.elementType()) { case BOOLEAN -> ValueExtractorForBoolean.extractorFor(encoder, inKey, (BooleanBlock) block);