Skip to content

Commit 2c9fce5

Browse files
tmgodinhotmgodinho
authored
IGNITE-28000 Reduce verbosity of client stacktraces (#7854)
- Simplify exceptions returns by TcpClientChannel#readError - Remove unnecessary calls to ensurePublicException - Improve TraceableExceptionMapper - Add copy method to some exceptions with fields. - Update ViewUtils ensurePublicException implementation - ClientTable now is responsible for ensuringPublicExceptions - Refactor error handling in ClientSQL - Reuse ViewUtils#sync in ClientCompute methods - Update SQL Exception Mapper - Do not wrap MarshallerException in MarshallerException in KeyValueViewImpl - Update ThinClientTests error checks - Update ItComputeTest error handling checks - Update ConnectionTest error check - Update error handling checks in tests - Update ItThinClientTransactionTest - Add CancelationException to ViewUtils#sync
1 parent c7908d8 commit 2c9fce5

26 files changed

Lines changed: 626 additions & 213 deletions

File tree

‎modules/api/src/main/java/org/apache/ignite/sql/SqlBatchException.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,15 @@ public SqlBatchException(UUID traceId, int code, String message, @Nullable Throw
7777
public long[] updateCounters() {
7878
return updCntrs;
7979
}
80+
81+
/**
82+
* Copy the exception.
83+
*
84+
* @param src Exception to copy.
85+
* @return new copied exception.
86+
*/
87+
@SuppressWarnings("PMD.UnusedPrivateMethod")
88+
private static SqlBatchException copy(SqlBatchException src) {
89+
return new SqlBatchException(src.traceId(), src.code(), src.updateCounters(), src.getMessage(), src.getCause());
90+
}
8091
}

‎modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ private static ErrorUiComponent connectionErrUiComponent(IgniteException e) {
6565
return fromIgniteException("Client error", e);
6666
}
6767

68-
if (e.getCause() instanceof IgniteClientConnectionException) {
69-
IgniteClientConnectionException cause = (IgniteClientConnectionException) e.getCause();
68+
if (e instanceof IgniteClientConnectionException) {
69+
IgniteClientConnectionException cause = (IgniteClientConnectionException) e;
7070

7171
SSLHandshakeException sslHandshakeException = findCause(cause, SSLHandshakeException.class);
7272
if (sslHandshakeException != null) {
@@ -86,9 +86,23 @@ private static ErrorUiComponent connectionErrUiComponent(IgniteException e) {
8686
private static ErrorUiComponent authnErrUiComponent(IgniteException e) {
8787
InvalidCredentialsException invalidCredentialsException = findCause(e, InvalidCredentialsException.class);
8888
if (invalidCredentialsException != null) {
89+
String msg = invalidCredentialsException.getMessage();
90+
91+
String details = msg;
92+
if (msg != null) {
93+
var headerIdx = msg.indexOf('\n');
94+
if (headerIdx != -1) {
95+
details = msg.substring(0, headerIdx);
96+
int traceInfoIdx = details.indexOf(" TraceId:");
97+
if (traceInfoIdx != -1) {
98+
details = details.substring(0, traceInfoIdx);
99+
}
100+
}
101+
}
102+
89103
return ErrorUiComponent.builder()
90104
.header("Could not connect to node. Check authentication configuration")
91-
.details(invalidCredentialsException.getMessage())
105+
.details(details)
92106
.verbose(extractCauseMessage(e.getMessage()))
93107
.build();
94108
}

‎modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientComputeTest.java‎

Lines changed: 37 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
import static org.apache.ignite.compute.JobStatus.EXECUTING;
2424
import static org.apache.ignite.compute.JobStatus.FAILED;
2525
import static org.apache.ignite.compute.JobStatus.QUEUED;
26-
import static org.apache.ignite.internal.IgniteExceptionTestUtils.traceableException;
26+
import static org.apache.ignite.internal.IgniteExceptionTestUtils.publicException;
27+
import static org.apache.ignite.internal.IgniteExceptionTestUtils.publicExceptionWithHint;
2728
import static org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrow;
2829
import static org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrowFast;
2930
import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.will;
@@ -49,7 +50,6 @@
4950
import static org.hamcrest.Matchers.oneOf;
5051
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
5152
import static org.junit.jupiter.api.Assertions.assertEquals;
52-
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
5353
import static org.junit.jupiter.api.Assertions.assertNotNull;
5454
import static org.junit.jupiter.api.Assertions.assertThrows;
5555
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -93,6 +93,7 @@
9393
import org.apache.ignite.compute.task.TaskExecution;
9494
import org.apache.ignite.compute.task.TaskExecutionContext;
9595
import org.apache.ignite.deployment.DeploymentUnit;
96+
import org.apache.ignite.internal.IgniteExceptionTestUtils.Cause;
9697
import org.apache.ignite.internal.compute.JobTaskStatusMapper;
9798
import org.apache.ignite.internal.runner.app.Jobs;
9899
import org.apache.ignite.internal.testframework.IgniteTestUtils;
@@ -438,14 +439,10 @@ void testIgniteExceptionInJobPropagatesToClientWithMessageAndCodeAndTraceIdAsync
438439
submit(JobTarget.node(node(0)), JobDescriptor.builder(Jobs.IgniteExceptionJob.class).build(), null)
439440
);
440441

441-
assertThat(cause.getMessage(), containsString("Custom job error"));
442-
assertEquals(Jobs.TRACE_ID, cause.traceId());
443-
assertEquals(COLUMN_NOT_FOUND_ERR, cause.code());
444-
assertInstanceOf(Jobs.CustomException.class, cause);
445-
assertNotNull(cause.getCause());
446-
String hint = cause.getCause().getMessage();
447-
448-
assertEquals("To see the full stack trace, set clientConnector.sendServerExceptionStackTraceToClient:true on the server", hint);
442+
assertThat(cause,
443+
publicExceptionWithHint(Jobs.CustomException.class, COLUMN_NOT_FOUND_ERR, "Custom job error")
444+
.withTraceId(is(Jobs.TRACE_ID))
445+
);
449446
}
450447

451448
@Test
@@ -455,14 +452,10 @@ void testIgniteExceptionInJobPropagatesToClientWithMessageAndCodeAndTraceIdSync(
455452
.execute(JobTarget.node(node(0)), JobDescriptor.builder(Jobs.IgniteExceptionJob.class).build(), null)
456453
);
457454

458-
assertThat(cause.getMessage(), containsString("Custom job error"));
459-
assertEquals(Jobs.TRACE_ID, cause.traceId());
460-
assertEquals(COLUMN_NOT_FOUND_ERR, cause.code());
461-
assertInstanceOf(Jobs.CustomException.class, cause);
462-
assertNotNull(cause.getCause());
463-
String hint = cause.getCause().getMessage();
464-
465-
assertEquals("To see the full stack trace, set clientConnector.sendServerExceptionStackTraceToClient:true on the server", hint);
455+
assertThat(cause,
456+
publicExceptionWithHint(Jobs.CustomException.class, COLUMN_NOT_FOUND_ERR, "Custom job error")
457+
.withTraceId(is(Jobs.TRACE_ID))
458+
);
466459
}
467460

468461
@ParameterizedTest
@@ -654,31 +647,31 @@ private static IgniteException getExceptionInTaskExecutionAsync(TaskExecution<St
654647
}
655648

656649
private static IgniteException getExceptionInJobExecutionSync(Supplier<String> execution) {
657-
IgniteException ex = assertThrows(IgniteException.class, execution::get);
658-
659-
return (IgniteException) ex.getCause();
650+
return assertThrows(IgniteException.class, execution::get);
660651
}
661652

662653
private static void assertComputeExceptionWithClassAndMessage(IgniteException cause) {
663-
String expectedMessage = "Job execution failed: java.lang.ArithmeticException: math err";
664-
assertThat(cause, is(traceableException(ComputeException.class, COMPUTE_JOB_FAILED_ERR, expectedMessage)));
665-
666-
assertNotNull(cause.getCause());
667-
String hint = cause.getCause().getMessage();
668-
669-
assertEquals("To see the full stack trace, set clientConnector.sendServerExceptionStackTraceToClient:true on the server", hint);
654+
assertThat(cause,
655+
publicExceptionWithHint(
656+
ComputeException.class,
657+
COMPUTE_JOB_FAILED_ERR,
658+
"Job execution failed: java.lang.ArithmeticException: math err"
659+
)
660+
);
670661
}
671662

672663
private static void assertComputeExceptionWithStackTrace(IgniteException cause) {
673-
String expectedMessage = "Job execution failed: java.lang.ArithmeticException: math err";
674-
assertThat(cause, is(traceableException(ComputeException.class, COMPUTE_JOB_FAILED_ERR, expectedMessage)));
675-
676-
assertNotNull(cause.getCause());
677-
678-
assertThat(cause.getCause().getMessage(), containsString(
679-
"Caused by: java.lang.ArithmeticException: math err" + System.lineSeparator()
680-
+ "\tat org.apache.ignite.internal.client.ItThinClientComputeTest$"
681-
+ "ExceptionJob.executeAsync(ItThinClientComputeTest.java:")
664+
assertThat(cause,
665+
publicException(
666+
ComputeException.class,
667+
COMPUTE_JOB_FAILED_ERR,
668+
"Job execution failed: java.lang.ArithmeticException: math err",
669+
List.of(
670+
Cause.of(ArithmeticException.class, "math err" + System.lineSeparator()
671+
+ "\tat org.apache.ignite.internal.client.ItThinClientComputeTest$"
672+
+ "ExceptionJob.executeAsync(ItThinClientComputeTest.java:")
673+
)
674+
)
682675
);
683676
}
684677

@@ -891,14 +884,13 @@ <I, M, T> void testExecuteMapReduceExceptionPropagation(Class<? extends MapReduc
891884
TaskDescriptor<I, String> taskDescriptor = TaskDescriptor.builder(taskClass).build();
892885
IgniteException cause = getExceptionInTaskExecutionAsync(client.compute().submitMapReduce(taskDescriptor, null));
893886

894-
assertThat(cause.getMessage(), containsString("Custom job error"));
895-
assertEquals(Jobs.TRACE_ID, cause.traceId());
896-
assertEquals(COLUMN_NOT_FOUND_ERR, cause.code());
897-
assertInstanceOf(Jobs.CustomException.class, cause);
898-
assertNotNull(cause.getCause());
899-
String hint = cause.getCause().getMessage();
900-
901-
assertEquals("To see the full stack trace, set clientConnector.sendServerExceptionStackTraceToClient:true on the server", hint);
887+
assertThat(cause,
888+
publicExceptionWithHint(
889+
Jobs.CustomException.class,
890+
COLUMN_NOT_FOUND_ERR,
891+
"Custom job error"
892+
).withTraceId(is(Jobs.TRACE_ID))
893+
);
902894
}
903895
}
904896

‎modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientComputeTypeCheckMarshallingTest.java‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,28 @@
1717

1818
package org.apache.ignite.internal.client;
1919

20+
import static java.util.Collections.emptyList;
2021
import static java.util.concurrent.CompletableFuture.completedFuture;
2122
import static org.apache.ignite.compute.JobStatus.COMPLETED;
2223
import static org.apache.ignite.compute.JobStatus.FAILED;
23-
import static org.apache.ignite.internal.IgniteExceptionTestUtils.hasMessage;
24+
import static org.apache.ignite.internal.IgniteExceptionTestUtils.publicException;
2425
import static org.apache.ignite.internal.IgniteExceptionTestUtils.traceableException;
2526
import static org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrow;
2627
import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
2728
import static org.apache.ignite.internal.testframework.matchers.JobStateMatcher.jobStateWithStatus;
2829
import static org.awaitility.Awaitility.await;
2930
import static org.hamcrest.MatcherAssert.assertThat;
30-
import static org.hamcrest.Matchers.containsString;
3131
import static org.hamcrest.Matchers.instanceOf;
3232

33+
import java.util.List;
3334
import java.util.concurrent.CompletableFuture;
3435
import org.apache.ignite.compute.ComputeException;
3536
import org.apache.ignite.compute.ComputeJob;
3637
import org.apache.ignite.compute.JobDescriptor;
3738
import org.apache.ignite.compute.JobExecution;
3839
import org.apache.ignite.compute.JobExecutionContext;
3940
import org.apache.ignite.compute.JobTarget;
41+
import org.apache.ignite.internal.IgniteExceptionTestUtils.Cause;
4042
import org.apache.ignite.internal.runner.app.Jobs.ArgMarshallingJob;
4143
import org.apache.ignite.internal.runner.app.Jobs.ResultMarshallingJob;
4244
import org.apache.ignite.lang.ErrorGroups.Compute;
@@ -139,7 +141,7 @@ void argumentMarshallerDoesNotMatch() {
139141
assertResultFailsWithErr(
140142
result, Compute.MARSHALLING_TYPE_MISMATCH_ERR,
141143
"Exception in user-defined marshaller",
142-
hasMessage(containsString("java.lang.RuntimeException: User defined error."))
144+
List.of(Cause.of(RuntimeException.class, "User defined error."))
143145
);
144146
}
145147

@@ -205,19 +207,35 @@ private static class IntegerMarshaller implements Marshaller<Integer, byte[]> {
205207
}
206208
}
207209

210+
private static void assertResultFailsWithErr(
211+
JobExecution<?> result,
212+
int errCode,
213+
String expectedMessage,
214+
@Nullable Matcher<? extends Throwable> causeMatcher
215+
) {
216+
assertThat(
217+
result.resultAsync(),
218+
willThrow(traceableException(ComputeException.class, errCode, expectedMessage).withCause(causeMatcher))
219+
);
220+
}
221+
208222
private static void assertResultFailsWithErr(JobExecution<?> result, int errCode, String expectedMessage) {
209-
assertResultFailsWithErr(result, errCode, expectedMessage, null);
223+
assertResultFailsWithErr(result, errCode, expectedMessage, emptyList());
210224
}
211225

212226
private static void assertResultFailsWithErr(
213227
JobExecution<?> result,
214228
int errCode,
215229
String expectedMessage,
216-
@Nullable Matcher<? extends Throwable> causeMatcher
230+
List<Cause> causes
217231
) {
218232
assertThat(
219233
result.resultAsync(),
220-
willThrow(traceableException(ComputeException.class, errCode, expectedMessage).withCause(causeMatcher))
234+
willThrow(
235+
publicException(
236+
ComputeException.class, errCode, expectedMessage, causes
237+
)
238+
)
221239
);
222240
}
223241
}

‎modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientConnectionTest.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
package org.apache.ignite.internal.client;
1919

20+
import static org.apache.ignite.internal.IgniteExceptionTestUtils.publicExceptionWithHint;
2021
import static org.apache.ignite.internal.eventlog.api.IgniteEventType.CLIENT_CONNECTION_CLOSED;
2122
import static org.apache.ignite.internal.eventlog.api.IgniteEventType.CLIENT_CONNECTION_ESTABLISHED;
2223
import static org.apache.ignite.lang.ErrorGroups.Table.TABLE_NOT_FOUND_ERR;
@@ -37,9 +38,11 @@
3738
import org.apache.ignite.client.IgniteClient;
3839
import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
3940
import org.apache.ignite.internal.testframework.log4j2.EventLogInspector;
41+
import org.apache.ignite.lang.ErrorGroups.Sql;
4042
import org.apache.ignite.lang.IgniteException;
4143
import org.apache.ignite.network.ClusterNode;
4244
import org.apache.ignite.sql.IgniteSql;
45+
import org.apache.ignite.sql.SqlException;
4346
import org.apache.ignite.table.RecordView;
4447
import org.apache.ignite.table.Table;
4548
import org.apache.ignite.table.Tuple;
@@ -126,17 +129,14 @@ void testHeartbeat() {
126129
@Test
127130
void testExceptionHasHint() {
128131
// Execute on all nodes to collect all types of exception.
129-
List<String> causes = IntStream.range(0, client().configuration().addresses().length)
132+
List<IgniteException> causes = IntStream.range(0, client().configuration().addresses().length)
130133
.mapToObj(i -> {
131-
IgniteException ex = assertThrows(IgniteException.class, () -> client().sql().execute("select x from bad"));
132-
133-
return ex.getCause().getCause().getCause().getCause().getMessage();
134+
return assertThrows(IgniteException.class, () -> client().sql().execute("select x from bad"));
134135
})
135136
.collect(Collectors.toList());
136137

137138
assertThat(causes,
138-
hasItem(containsString("To see the full stack trace, "
139-
+ "set clientConnector.sendServerExceptionStackTraceToClient:true on the server")));
139+
hasItem(publicExceptionWithHint(SqlException.class, Sql.STMT_VALIDATION_ERR, "Object 'BAD' not found")));
140140
}
141141

142142
@Test

‎modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientTransactionsTest.java‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@
2020
import static java.lang.String.format;
2121
import static java.util.Collections.emptyList;
2222
import static java.util.Comparator.comparing;
23+
import static org.apache.ignite.internal.IgniteExceptionTestUtils.publicException;
24+
import static org.apache.ignite.internal.IgniteExceptionTestUtils.publicExceptionWithHint;
2325
import static org.apache.ignite.internal.TestWrappers.unwrapIgniteImpl;
2426
import static org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrowWithCauseOrSuppressed;
2527
import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willSucceedFast;
2628
import static org.awaitility.Awaitility.await;
2729
import static org.hamcrest.MatcherAssert.assertThat;
28-
import static org.hamcrest.Matchers.containsString;
30+
import static org.hamcrest.Matchers.isA;
2931
import static org.hamcrest.Matchers.startsWith;
3032
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
3133
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -70,6 +72,7 @@
7072
import org.apache.ignite.internal.tx.TxState;
7173
import org.apache.ignite.internal.util.CollectionUtils;
7274
import org.apache.ignite.lang.ErrorGroups;
75+
import org.apache.ignite.lang.ErrorGroups.Common;
7376
import org.apache.ignite.lang.ErrorGroups.Transactions;
7477
import org.apache.ignite.lang.IgniteException;
7578
import org.apache.ignite.network.ClusterNode;
@@ -322,11 +325,14 @@ public boolean isReadOnly() {
322325
};
323326

324327
var ex = assertThrows(IgniteException.class, () -> kvView().put(tx, 1, "1"));
325-
326-
String expected = "Unsupported transaction implementation: "
327-
+ "'class org.apache.ignite.internal.client.ItThinClientTransactionsTest";
328-
329-
assertThat(ex.getMessage(), containsString(expected));
328+
assertThat(ex,
329+
publicException(
330+
IgniteException.class,
331+
Common.INTERNAL_ERR,
332+
format("Unsupported transaction implementation: 'class %s'", tx.getClass().getName()),
333+
emptyList()
334+
)
335+
);
330336
}
331337

332338
@Test
@@ -338,8 +344,14 @@ void testTransactionFromAnotherChannelThrows() {
338344
RecordView<Tuple> recordView = client2.tables().tables().get(0).recordView();
339345

340346
var ex = assertThrows(IgniteException.class, () -> recordView.upsert(tx, Tuple.create()));
341-
342-
assertThat(ex.getMessage(), containsString("Transaction belongs to a different client instance"));
347+
assertThat(ex,
348+
publicException(
349+
IgniteException.class,
350+
Common.INTERNAL_ERR,
351+
"Transaction belongs to a different client instance",
352+
emptyList()
353+
).withCause(isA(IllegalArgumentException.class))
354+
);
343355
}
344356
}
345357

@@ -372,8 +384,13 @@ void testUpdateInReadOnlyTxThrows() {
372384
Transaction tx = client().transactions().begin(new TransactionOptions().readOnly(true));
373385
var ex = assertThrows(TransactionException.class, () -> kvView.put(tx, 1, "2"));
374386

375-
assertThat(ex.getMessage(), containsString("Failed to enlist read-write operation into read-only transaction"));
376-
assertEquals(ErrorGroups.Transactions.TX_FAILED_READ_WRITE_OPERATION_ERR, ex.code());
387+
assertThat(ex,
388+
publicExceptionWithHint(
389+
TransactionException.class,
390+
ErrorGroups.Transactions.TX_FAILED_READ_WRITE_OPERATION_ERR,
391+
"Failed to enlist read-write operation into read-only transaction"
392+
)
393+
);
377394
}
378395

379396
@ParameterizedTest

0 commit comments

Comments
 (0)