ptupitsyn commented on code in PR #7854:
URL: https://github.com/apache/ignite-3/pull/7854#discussion_r3008047833
##########
modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java:
##########
@@ -647,12 +647,39 @@ private static Throwable readError(ClientMessageUnpacker
unpacker) {
var errClassName = unpacker.unpackString();
var errMsg = unpacker.tryUnpackNil() ? null : unpacker.unpackString();
- boolean retriable = false;
+ @Nullable String causeStr = unpacker.tryUnpackNil() ? null :
unpacker.unpackString();
+
+ String msg;
+ if (causeStr == null) {
+ msg = errMsg;
+ } else if (errMsg == null) {
+ msg = causeStr;
+ } else {
+ // Remove some duplication between errorMsg and cause.
Review Comment:
Should we do this on the server, or is it specific to Java client?
##########
modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientConnectionTest.java:
##########
@@ -126,17 +129,14 @@ void testHeartbeat() {
@Test
void testExceptionHasHint() {
// Execute on all nodes to collect all types of exception.
- List<String> causes = IntStream.range(0,
client().configuration().addresses().length)
+ List<IgniteException> causes = IntStream.range(0,
client().configuration().addresses().length)
.mapToObj(i -> {
- IgniteException ex = assertThrows(IgniteException.class,
() -> client().sql().execute("select x from bad"));
-
- return
ex.getCause().getCause().getCause().getCause().getMessage();
+ return assertThrows(IgniteException.class, () ->
client().sql().execute("select x from bad"));
})
.collect(Collectors.toList());
assertThat(causes,
- hasItem(containsString("To see the full stack trace, "
- + "set
clientConnector.sendServerExceptionStackTraceToClient:true on the server")));
+ hasItem(publicExceptionWithHint(SqlException.class,
Sql.STMT_VALIDATION_ERR, "Object 'BAD' not found")));
Review Comment:
This changes the meaning of the test. We want to check for "To see the full
stack trace" here.
##########
modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java:
##########
@@ -647,12 +647,39 @@ private static Throwable readError(ClientMessageUnpacker
unpacker) {
var errClassName = unpacker.unpackString();
var errMsg = unpacker.tryUnpackNil() ? null : unpacker.unpackString();
- boolean retriable = false;
+ @Nullable String causeStr = unpacker.tryUnpackNil() ? null :
unpacker.unpackString();
+
+ String msg;
+ if (causeStr == null) {
+ msg = errMsg;
+ } else if (errMsg == null) {
+ msg = causeStr;
+ } else {
+ // Remove some duplication between errorMsg and cause.
+ int idx = causeStr.indexOf(errMsg);
+ msg = (idx == -1) ? errMsg + '\n' + causeStr :
causeStr.substring(idx);
+ }
+
+ Function<Boolean, Throwable> exceptionFactory = isRetriable -> {
Review Comment:
Can we extract a method instead of allocating a lambda?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
