Skip to content

Commit 743961a

Browse files
benchaplinbenchaplin
authored
Fix error trace stack trace assertions (#137124) (#137760)
1 parent 2816ef5 commit 743961a

File tree

3 files changed

+32
-28
lines changed

3 files changed

+32
-28
lines changed

‎qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ public void testSearchFailingQueryErrorTraceDefault() throws IOException {
6868
}
6969
}
7070
""");
71+
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
7172
getRestClient().performRequest(searchRequest);
72-
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
7373
}
7474

7575
public void testSearchFailingQueryErrorTraceTrue() throws IOException {
@@ -87,8 +87,8 @@ public void testSearchFailingQueryErrorTraceTrue() throws IOException {
8787
}
8888
""");
8989
searchRequest.addParameter("error_trace", "true");
90+
ErrorTraceHelper.expectStackTraceObserved(internalCluster());
9091
getRestClient().performRequest(searchRequest);
91-
ErrorTraceHelper.assertStackTraceObserved(internalCluster());
9292
}
9393

9494
public void testSearchFailingQueryErrorTraceFalse() throws IOException {
@@ -106,8 +106,8 @@ public void testSearchFailingQueryErrorTraceFalse() throws IOException {
106106
}
107107
""");
108108
searchRequest.addParameter("error_trace", "false");
109+
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
109110
getRestClient().performRequest(searchRequest);
110-
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
111111
}
112112

113113
public void testDataNodeLogsStackTrace() throws IOException {
@@ -155,8 +155,8 @@ public void testMultiSearchFailingQueryErrorTraceDefault() throws IOException {
155155
searchRequest.setEntity(
156156
new NByteArrayEntity(requestBody, ContentType.create(contentType.mediaTypeWithoutParameters(), (Charset) null))
157157
);
158+
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
158159
getRestClient().performRequest(searchRequest);
159-
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
160160
}
161161

162162
public void testMultiSearchFailingQueryErrorTraceTrue() throws IOException {
@@ -172,8 +172,8 @@ public void testMultiSearchFailingQueryErrorTraceTrue() throws IOException {
172172
new NByteArrayEntity(requestBody, ContentType.create(contentType.mediaTypeWithoutParameters(), (Charset) null))
173173
);
174174
searchRequest.addParameter("error_trace", "true");
175+
ErrorTraceHelper.expectStackTraceObserved(internalCluster());
175176
getRestClient().performRequest(searchRequest);
176-
ErrorTraceHelper.assertStackTraceObserved(internalCluster());
177177
}
178178

179179
public void testMultiSearchFailingQueryErrorTraceFalse() throws IOException {
@@ -189,8 +189,8 @@ public void testMultiSearchFailingQueryErrorTraceFalse() throws IOException {
189189
new NByteArrayEntity(requestBody, ContentType.create(contentType.mediaTypeWithoutParameters(), (Charset) null))
190190
);
191191
searchRequest.addParameter("error_trace", "false");
192+
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
192193
getRestClient().performRequest(searchRequest);
193-
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
194194
}
195195

196196
public void testDataNodeLogsStackTraceMultiSearch() throws IOException {

‎test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,23 @@
4242
public enum ErrorTraceHelper {
4343
;
4444

45-
public static void assertStackTraceObserved(InternalTestCluster internalTestCluster) {
46-
assertStackTraceObserved(internalTestCluster, true);
45+
/**
46+
* Sets up transport interception to assert that stack traces are present in error responses for batched query requests.
47+
* Must be called before executing requests that are expected to generate errors.
48+
*/
49+
public static void expectStackTraceObserved(InternalTestCluster internalCluster) {
50+
expectStackTraceObserved(internalCluster, true);
4751
}
4852

49-
public static void assertStackTraceCleared(InternalTestCluster internalTestCluster) {
50-
assertStackTraceObserved(internalTestCluster, false);
53+
/**
54+
* Sets up transport interception to assert that stack traces are NOT present in error responses for batched query requests.
55+
* Must be called before executing requests that are expected to generate errors.
56+
*/
57+
public static void expectStackTraceCleared(InternalTestCluster internalCluster) {
58+
expectStackTraceObserved(internalCluster, false);
5159
}
5260

53-
private static void assertStackTraceObserved(InternalTestCluster internalCluster, boolean shouldObserveStackTrace) {
61+
private static void expectStackTraceObserved(InternalTestCluster internalCluster, boolean shouldObserveStackTrace) {
5462
internalCluster.getDataNodeInstances(TransportService.class)
5563
.forEach(
5664
ts -> asInstanceOf(MockTransportService.class, ts).addRequestHandlingBehavior(
@@ -80,21 +88,22 @@ public void sendResponse(TransportResponse response) {
8088
} catch (IOException e) {
8189
throw new UncheckedIOException(e);
8290
} finally {
91+
// Always forward to the original channel
92+
channel.sendResponse(response);
8393
if (nodeQueryResponse != null) {
8494
nodeQueryResponse.decRef();
8595
}
8696
}
87-
88-
// Forward to the original channel
89-
channel.sendResponse(response);
9097
}
9198

9299
@Override
93100
public void sendResponse(Exception error) {
94-
inspectStackTraceAndAssert(error);
95-
96-
// Forward to the original channel
97-
channel.sendResponse(error);
101+
try {
102+
inspectStackTraceAndAssert(error);
103+
} finally {
104+
// Always forward to the original channel
105+
channel.sendResponse(error);
106+
}
98107
}
99108

100109
private void inspectStackTraceAndAssert(Exception error) {

‎x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ public void testAsyncSearchFailingQueryErrorTraceDefault() throws Exception {
7676
""");
7777
createAsyncRequest.addParameter("keep_on_completion", "true");
7878
createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms");
79+
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
7980
Map<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest);
8081

8182
try {
@@ -87,8 +88,6 @@ public void testAsyncSearchFailingQueryErrorTraceDefault() throws Exception {
8788
} finally {
8889
deleteAsyncSearchIfPresent(createAsyncResponseEntity);
8990
}
90-
// check that the stack trace was not sent from the data node to the coordinating node
91-
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
9291
}
9392

9493
public void testAsyncSearchFailingQueryErrorTraceTrue() throws Exception {
@@ -108,6 +107,7 @@ public void testAsyncSearchFailingQueryErrorTraceTrue() throws Exception {
108107
createAsyncRequest.addParameter("error_trace", "true");
109108
createAsyncRequest.addParameter("keep_on_completion", "true");
110109
createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms");
110+
ErrorTraceHelper.expectStackTraceObserved(internalCluster());
111111
Map<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest);
112112

113113
try {
@@ -120,8 +120,6 @@ public void testAsyncSearchFailingQueryErrorTraceTrue() throws Exception {
120120
} finally {
121121
deleteAsyncSearchIfPresent(createAsyncResponseEntity);
122122
}
123-
// check that the stack trace was sent from the data node to the coordinating node
124-
ErrorTraceHelper.assertStackTraceObserved(internalCluster());
125123
}
126124

127125
public void testAsyncSearchFailingQueryErrorTraceFalse() throws Exception {
@@ -141,6 +139,7 @@ public void testAsyncSearchFailingQueryErrorTraceFalse() throws Exception {
141139
createAsyncRequest.addParameter("error_trace", "false");
142140
createAsyncRequest.addParameter("keep_on_completion", "true");
143141
createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms");
142+
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
144143
Map<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest);
145144

146145
try {
@@ -153,8 +152,6 @@ public void testAsyncSearchFailingQueryErrorTraceFalse() throws Exception {
153152
} finally {
154153
deleteAsyncSearchIfPresent(createAsyncResponseEntity);
155154
}
156-
// check that the stack trace was not sent from the data node to the coordinating node
157-
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
158155
}
159156

160157
public void testDataNodeLogsStackTrace() throws Exception {
@@ -226,6 +223,7 @@ public void testAsyncSearchFailingQueryErrorTraceFalseOnSubmitAndTrueOnGet() thr
226223
createAsyncSearchRequest.addParameter("error_trace", "false");
227224
createAsyncSearchRequest.addParameter("keep_on_completion", "true");
228225
createAsyncSearchRequest.addParameter("wait_for_completion_timeout", "0ms");
226+
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
229227
Map<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncSearchRequest);
230228

231229
try {
@@ -238,8 +236,6 @@ public void testAsyncSearchFailingQueryErrorTraceFalseOnSubmitAndTrueOnGet() thr
238236
} finally {
239237
deleteAsyncSearchIfPresent(createAsyncResponseEntity);
240238
}
241-
// check that the stack trace was not sent from the data node to the coordinating node
242-
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
243239
}
244240

245241
public void testAsyncSearchFailingQueryErrorTraceTrueOnSubmitAndFalseOnGet() throws Exception {
@@ -259,6 +255,7 @@ public void testAsyncSearchFailingQueryErrorTraceTrueOnSubmitAndFalseOnGet() thr
259255
createAsyncSearchRequest.addParameter("error_trace", "true");
260256
createAsyncSearchRequest.addParameter("keep_on_completion", "true");
261257
createAsyncSearchRequest.addParameter("wait_for_completion_timeout", "0ms");
258+
ErrorTraceHelper.expectStackTraceObserved(internalCluster());
262259
Map<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncSearchRequest);
263260
try {
264261
if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) {
@@ -270,8 +267,6 @@ public void testAsyncSearchFailingQueryErrorTraceTrueOnSubmitAndFalseOnGet() thr
270267
} finally {
271268
deleteAsyncSearchIfPresent(createAsyncResponseEntity);
272269
}
273-
// check that the stack trace was sent from the data node to the coordinating node
274-
ErrorTraceHelper.assertStackTraceObserved(internalCluster());
275270
}
276271

277272
private Map<String, Object> performRequestAndGetResponseEntity(Request r) throws IOException {

0 commit comments

Comments
 (0)