Skip to content

Commit 1a7c69d

Browse files
committed
Fail with 500 not 400 for ValueExtractor bugs
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.
1 parent 3575bdb commit 1a7c69d

File tree

1 file changed

+4
-1
lines changed
  • x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn

1 file changed

+4
-1
lines changed

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ValueExtractor.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ interface ValueExtractor {
2727

2828
static ValueExtractor extractorFor(ElementType elementType, TopNEncoder encoder, boolean inKey, Block block) {
2929
if (false == (elementType == block.elementType() || ElementType.NULL == block.elementType())) {
30-
throw new IllegalArgumentException("Expected [" + elementType + "] but was [" + block.elementType() + "]");
30+
// While this maybe should be an IllegalArgumentException, it's important to throw an exception that causes a 500 response.
31+
// If we reach here, that's a bug. Arguably, the operators are in an illegal state because the layout doesn't match the
32+
// actual pages.
33+
throw new IllegalStateException("Expected [" + elementType + "] but was [" + block.elementType() + "]");
3134
}
3235
return switch (block.elementType()) {
3336
case BOOLEAN -> ValueExtractorForBoolean.extractorFor(encoder, inKey, (BooleanBlock) block);

0 commit comments

Comments
 (0)