Skip to content

Fix error trace stack trace assertions #137124

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+32 −28
Open

Fix error trace stack trace assertions #137124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a68e709
Fix error trace tests
benchaplin Oct 24, 2025
3b708eb
Merge branch 'main' into fix_error_trace_tests
benchaplin Oct 24, 2025
f7f1423
Clear added handling after each test
benchaplin Oct 24, 2025
2626c52
Remove clean up (it's already done in ESIntegTestCase)
benchaplin Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions ...oke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ public void testSearchFailingQueryErrorTraceDefault() throws IOException {
}
}
""");
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
getRestClient().performRequest(searchRequest);
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
}

public void testSearchFailingQueryErrorTraceTrue() throws IOException {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
33 changes: 21 additions & 12 deletions test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving these into the finally block so that AssertionErrors don't stop execution.

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) {
Expand Down
15 changes: 5 additions & 10 deletions .../src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> 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 {
Expand All @@ -102,15 +101,14 @@ 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<String, Object> 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);
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 {
Expand All @@ -130,15 +128,14 @@ 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<String, Object> 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);
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 {
Expand Down Expand Up @@ -205,15 +202,14 @@ 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<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncSearchRequest);
if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) {
String asyncExecutionId = (String) createAsyncResponseEntity.get("id");
Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId);
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 {
Expand All @@ -233,15 +229,14 @@ 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<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncSearchRequest);
if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) {
String asyncExecutionId = (String) createAsyncResponseEntity.get("id");
Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId);
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<String, Object> performRequestAndGetResponseEntity(Request r) throws IOException {
Expand Down