Skip to content

Commit de03153

Browse files
benchaplinbenchaplin
committed
Fix error trace stack trace assertions (elastic#137124)
1 parent 42408b5 commit de03153

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
@@ -75,14 +75,13 @@ public void testAsyncSearchFailingQueryErrorTraceDefault() throws Exception {
7575
""");
7676
createAsyncRequest.addParameter("keep_on_completion", "true");
7777
createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms");
78+
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
7879
Map<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest);
7980
if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) {
8081
String asyncExecutionId = (String) createAsyncResponseEntity.get("id");
8182
Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId);
8283
awaitAsyncRequestDoneRunning(getAsyncRequest);
8384
}
84-
// check that the stack trace was not sent from the data node to the coordinating node
85-
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
8685
}
8786

8887
public void testAsyncSearchFailingQueryErrorTraceTrue() throws Exception {
@@ -102,15 +101,14 @@ public void testAsyncSearchFailingQueryErrorTraceTrue() throws Exception {
102101
createAsyncRequest.addParameter("error_trace", "true");
103102
createAsyncRequest.addParameter("keep_on_completion", "true");
104103
createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms");
104+
ErrorTraceHelper.expectStackTraceObserved(internalCluster());
105105
Map<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest);
106106
if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) {
107107
String asyncExecutionId = (String) createAsyncResponseEntity.get("id");
108108
Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId);
109109
getAsyncRequest.addParameter("error_trace", "true");
110110
awaitAsyncRequestDoneRunning(getAsyncRequest);
111111
}
112-
// check that the stack trace was sent from the data node to the coordinating node
113-
ErrorTraceHelper.assertStackTraceObserved(internalCluster());
114112
}
115113

116114
public void testAsyncSearchFailingQueryErrorTraceFalse() throws Exception {
@@ -130,15 +128,14 @@ public void testAsyncSearchFailingQueryErrorTraceFalse() throws Exception {
130128
createAsyncRequest.addParameter("error_trace", "false");
131129
createAsyncRequest.addParameter("keep_on_completion", "true");
132130
createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms");
131+
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
133132
Map<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest);
134133
if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) {
135134
String asyncExecutionId = (String) createAsyncResponseEntity.get("id");
136135
Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId);
137136
getAsyncRequest.addParameter("error_trace", "false");
138137
awaitAsyncRequestDoneRunning(getAsyncRequest);
139138
}
140-
// check that the stack trace was not sent from the data node to the coordinating node
141-
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
142139
}
143140

144141
public void testDataNodeLogsStackTrace() throws Exception {
@@ -205,15 +202,14 @@ public void testAsyncSearchFailingQueryErrorTraceFalseOnSubmitAndTrueOnGet() thr
205202
createAsyncSearchRequest.addParameter("error_trace", "false");
206203
createAsyncSearchRequest.addParameter("keep_on_completion", "true");
207204
createAsyncSearchRequest.addParameter("wait_for_completion_timeout", "0ms");
205+
ErrorTraceHelper.expectStackTraceCleared(internalCluster());
208206
Map<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncSearchRequest);
209207
if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) {
210208
String asyncExecutionId = (String) createAsyncResponseEntity.get("id");
211209
Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId);
212210
getAsyncRequest.addParameter("error_trace", "true");
213211
awaitAsyncRequestDoneRunning(getAsyncRequest);
214212
}
215-
// check that the stack trace was not sent from the data node to the coordinating node
216-
ErrorTraceHelper.assertStackTraceCleared(internalCluster());
217213
}
218214

219215
public void testAsyncSearchFailingQueryErrorTraceTrueOnSubmitAndFalseOnGet() throws Exception {
@@ -233,15 +229,14 @@ public void testAsyncSearchFailingQueryErrorTraceTrueOnSubmitAndFalseOnGet() thr
233229
createAsyncSearchRequest.addParameter("error_trace", "true");
234230
createAsyncSearchRequest.addParameter("keep_on_completion", "true");
235231
createAsyncSearchRequest.addParameter("wait_for_completion_timeout", "0ms");
232+
ErrorTraceHelper.expectStackTraceObserved(internalCluster());
236233
Map<String, Object> createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncSearchRequest);
237234
if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) {
238235
String asyncExecutionId = (String) createAsyncResponseEntity.get("id");
239236
Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId);
240237
getAsyncRequest.addParameter("error_trace", "false");
241238
awaitAsyncRequestDoneRunning(getAsyncRequest);
242239
}
243-
// check that the stack trace was sent from the data node to the coordinating node
244-
ErrorTraceHelper.assertStackTraceObserved(internalCluster());
245240
}
246241

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

0 commit comments

Comments
 (0)