From de031532db194c73804e09cd48f5ebfda499dc23 Mon Sep 17 00:00:00 2001 From: Ben Chaplin Date: Fri, 7 Nov 2025 12:14:43 -0500 Subject: [PATCH] Fix error trace stack trace assertions (#137124) --- .../http/SearchErrorTraceIT.java | 12 +++---- .../search/ErrorTraceHelper.java | 33 ++++++++++++------- .../xpack/search/AsyncSearchErrorTraceIT.java | 15 +++------ 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java b/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java index f407592f63132..f1bd549e19796 100644 --- a/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java +++ b/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java @@ -68,8 +68,8 @@ public void testSearchFailingQueryErrorTraceDefault() throws IOException { } } """); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testSearchFailingQueryErrorTraceTrue() throws IOException { @@ -87,8 +87,8 @@ public void testSearchFailingQueryErrorTraceTrue() throws IOException { } """); searchRequest.addParameter("error_trace", "true"); + ErrorTraceHelper.expectStackTraceObserved(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceObserved(internalCluster()); } public void testSearchFailingQueryErrorTraceFalse() throws IOException { @@ -106,8 +106,8 @@ public void testSearchFailingQueryErrorTraceFalse() throws IOException { } """); searchRequest.addParameter("error_trace", "false"); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testDataNodeLogsStackTrace() throws IOException { @@ -155,8 +155,8 @@ public void testMultiSearchFailingQueryErrorTraceDefault() throws IOException { searchRequest.setEntity( new NByteArrayEntity(requestBody, ContentType.create(contentType.mediaTypeWithoutParameters(), (Charset) null)) ); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testMultiSearchFailingQueryErrorTraceTrue() throws IOException { @@ -172,8 +172,8 @@ public void testMultiSearchFailingQueryErrorTraceTrue() throws IOException { new NByteArrayEntity(requestBody, ContentType.create(contentType.mediaTypeWithoutParameters(), (Charset) null)) ); searchRequest.addParameter("error_trace", "true"); + ErrorTraceHelper.expectStackTraceObserved(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceObserved(internalCluster()); } public void testMultiSearchFailingQueryErrorTraceFalse() throws IOException { @@ -189,8 +189,8 @@ public void testMultiSearchFailingQueryErrorTraceFalse() throws IOException { new NByteArrayEntity(requestBody, ContentType.create(contentType.mediaTypeWithoutParameters(), (Charset) null)) ); searchRequest.addParameter("error_trace", "false"); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testDataNodeLogsStackTraceMultiSearch() throws IOException { diff --git a/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java b/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java index ab8308391b202..1c7c8a5960313 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java +++ b/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java @@ -42,15 +42,23 @@ public enum ErrorTraceHelper { ; - public static void assertStackTraceObserved(InternalTestCluster internalTestCluster) { - assertStackTraceObserved(internalTestCluster, true); + /** + * Sets up transport interception to assert that stack traces are present in error responses for batched query requests. + * Must be called before executing requests that are expected to generate errors. + */ + public static void expectStackTraceObserved(InternalTestCluster internalCluster) { + expectStackTraceObserved(internalCluster, true); } - public static void assertStackTraceCleared(InternalTestCluster internalTestCluster) { - assertStackTraceObserved(internalTestCluster, false); + /** + * Sets up transport interception to assert that stack traces are NOT present in error responses for batched query requests. + * Must be called before executing requests that are expected to generate errors. + */ + public static void expectStackTraceCleared(InternalTestCluster internalCluster) { + expectStackTraceObserved(internalCluster, false); } - private static void assertStackTraceObserved(InternalTestCluster internalCluster, boolean shouldObserveStackTrace) { + private static void expectStackTraceObserved(InternalTestCluster internalCluster, boolean shouldObserveStackTrace) { internalCluster.getDataNodeInstances(TransportService.class) .forEach( ts -> asInstanceOf(MockTransportService.class, ts).addRequestHandlingBehavior( @@ -80,21 +88,22 @@ public void sendResponse(TransportResponse response) { } catch (IOException e) { throw new UncheckedIOException(e); } finally { + // Always forward to the original channel + channel.sendResponse(response); if (nodeQueryResponse != null) { nodeQueryResponse.decRef(); } } - - // Forward to the original channel - channel.sendResponse(response); } @Override public void sendResponse(Exception error) { - inspectStackTraceAndAssert(error); - - // Forward to the original channel - channel.sendResponse(error); + try { + inspectStackTraceAndAssert(error); + } finally { + // Always forward to the original channel + channel.sendResponse(error); + } } private void inspectStackTraceAndAssert(Exception error) { diff --git a/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java b/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java index 83e7cf9e12096..2996289d87012 100644 --- a/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java +++ b/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java @@ -75,14 +75,13 @@ public void testAsyncSearchFailingQueryErrorTraceDefault() throws Exception { """); createAsyncRequest.addParameter("keep_on_completion", "true"); createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms"); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest); if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId); awaitAsyncRequestDoneRunning(getAsyncRequest); } - // check that the stack trace was not sent from the data node to the coordinating node - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testAsyncSearchFailingQueryErrorTraceTrue() throws Exception { @@ -102,6 +101,7 @@ public void testAsyncSearchFailingQueryErrorTraceTrue() throws Exception { createAsyncRequest.addParameter("error_trace", "true"); createAsyncRequest.addParameter("keep_on_completion", "true"); createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms"); + ErrorTraceHelper.expectStackTraceObserved(internalCluster()); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest); if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); @@ -109,8 +109,6 @@ public void testAsyncSearchFailingQueryErrorTraceTrue() throws Exception { getAsyncRequest.addParameter("error_trace", "true"); awaitAsyncRequestDoneRunning(getAsyncRequest); } - // check that the stack trace was sent from the data node to the coordinating node - ErrorTraceHelper.assertStackTraceObserved(internalCluster()); } public void testAsyncSearchFailingQueryErrorTraceFalse() throws Exception { @@ -130,6 +128,7 @@ public void testAsyncSearchFailingQueryErrorTraceFalse() throws Exception { createAsyncRequest.addParameter("error_trace", "false"); createAsyncRequest.addParameter("keep_on_completion", "true"); createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms"); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest); if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); @@ -137,8 +136,6 @@ public void testAsyncSearchFailingQueryErrorTraceFalse() throws Exception { getAsyncRequest.addParameter("error_trace", "false"); awaitAsyncRequestDoneRunning(getAsyncRequest); } - // check that the stack trace was not sent from the data node to the coordinating node - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testDataNodeLogsStackTrace() throws Exception { @@ -205,6 +202,7 @@ public void testAsyncSearchFailingQueryErrorTraceFalseOnSubmitAndTrueOnGet() thr createAsyncSearchRequest.addParameter("error_trace", "false"); createAsyncSearchRequest.addParameter("keep_on_completion", "true"); createAsyncSearchRequest.addParameter("wait_for_completion_timeout", "0ms"); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncSearchRequest); if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); @@ -212,8 +210,6 @@ public void testAsyncSearchFailingQueryErrorTraceFalseOnSubmitAndTrueOnGet() thr getAsyncRequest.addParameter("error_trace", "true"); awaitAsyncRequestDoneRunning(getAsyncRequest); } - // check that the stack trace was not sent from the data node to the coordinating node - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testAsyncSearchFailingQueryErrorTraceTrueOnSubmitAndFalseOnGet() throws Exception { @@ -233,6 +229,7 @@ public void testAsyncSearchFailingQueryErrorTraceTrueOnSubmitAndFalseOnGet() thr createAsyncSearchRequest.addParameter("error_trace", "true"); createAsyncSearchRequest.addParameter("keep_on_completion", "true"); createAsyncSearchRequest.addParameter("wait_for_completion_timeout", "0ms"); + ErrorTraceHelper.expectStackTraceObserved(internalCluster()); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncSearchRequest); if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); @@ -240,8 +237,6 @@ public void testAsyncSearchFailingQueryErrorTraceTrueOnSubmitAndFalseOnGet() thr getAsyncRequest.addParameter("error_trace", "false"); awaitAsyncRequestDoneRunning(getAsyncRequest); } - // check that the stack trace was sent from the data node to the coordinating node - ErrorTraceHelper.assertStackTraceObserved(internalCluster()); } private Map performRequestAndGetResponseEntity(Request r) throws IOException {