This is an automated email from the ASF dual-hosted git repository.
ptupitsyn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 3f8bc233e95 IGNITE-28239 Improve MessageTypeException text (#7792)
3f8bc233e95 is described below
commit 3f8bc233e95e07216e25b4e1016dcfaec07435bd
Author: Pavel Tupitsyn <[email protected]>
AuthorDate: Tue Mar 17 12:59:16 2026 +0100
IGNITE-28239 Improve MessageTypeException text (#7792)
* Add more details to help debugging protocol issues
* Add `consistentId` validation to ensure direct tx protocol correctness
---
.../internal/client/proto/ClientMessageUnpacker.java | 18 ++++++++++++++++--
.../handler/requests/table/ClientTableCommon.java | 8 +++++++-
.../requests/tx/ClientTransactionDiscardRequest.java | 2 +-
.../ignite/internal/client/TcpClientChannel.java | 20 ++++++++++++--------
.../ignite/internal/client/tx/DirectTxUtils.java | 6 +++---
.../apache/ignite/client/ClientKeyValueViewTest.java | 13 +++++++------
.../runner/app/client/ItThinClientSqlTest.java | 2 +-
.../ignite/internal/tx/PartitionEnlistment.java | 8 ++++++++
8 files changed, 55 insertions(+), 22 deletions(-)
diff --git
a/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientMessageUnpacker.java
b/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientMessageUnpacker.java
index 42a341fa812..4b010fd0fdb 100644
---
a/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientMessageUnpacker.java
+++
b/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientMessageUnpacker.java
@@ -20,6 +20,7 @@ package org.apache.ignite.internal.client.proto;
import static org.msgpack.core.MessagePack.Code;
import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufUtil;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.charset.StandardCharsets;
@@ -83,7 +84,7 @@ public class ClientMessageUnpacker implements AutoCloseable {
* @param b Actual format.
* @return Exception to throw.
*/
- private static MessagePackException unexpected(String expected, byte b) {
+ private MessagePackException unexpected(String expected, byte b) {
MessageFormat format = MessageFormat.valueOf(b);
if (format == MessageFormat.NEVER_USED) {
@@ -91,7 +92,20 @@ public class ClientMessageUnpacker implements AutoCloseable {
} else {
String name = format.getValueType().name();
String typeName = name.charAt(0) + name.substring(1).toLowerCase();
- return new MessageTypeException(String.format("Expected %s, but
got %s (%02x)", expected, typeName, b));
+
+ // Convert all bytes from the start of the buffer to the current
position to a string for debugging
+ ByteBuf slice = buf.slice(0, buf.readerIndex());
+
+ int maxBufSliceLen = 256;
+ if (slice.readableBytes() > maxBufSliceLen) {
+ slice = slice.slice(slice.readableBytes() - maxBufSliceLen,
maxBufSliceLen);
+ }
+
+ String bufContent = ByteBufUtil.hexDump(slice);
+ int problemPos = buf.readerIndex() - 1;
+
+ return new MessageTypeException(
+ String.format("Expected %s, but got %s (%02x) at pos %s:
'%s'", expected, typeName, b, problemPos, bufContent));
}
}
diff --git
a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTableCommon.java
b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTableCommon.java
index 3418ada28f5..8d8851dd094 100644
---
a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTableCommon.java
+++
b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTableCommon.java
@@ -385,7 +385,13 @@ public class ClientTableCommon {
out.packLong(tx.getTimeout());
} else if (tx.remote()) {
PendingTxPartitionEnlistment token = tx.enlistedPartition(null);
- out.packString(token.primaryNodeConsistentId());
+ String consistentId = token.primaryNodeConsistentId();
+
+ if (consistentId == null) {
+ throw new IllegalStateException("Primary node consistent ID
must not be null for remote transactions: " + tx);
+ }
+
+ out.packString(consistentId);
out.packLong(token.consistencyToken());
out.packBoolean(TxState.ABORTED == tx.state()); // No-op
enlistment.
diff --git
a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/tx/ClientTransactionDiscardRequest.java
b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/tx/ClientTransactionDiscardRequest.java
index e343917ea71..dc823da38ae 100644
---
a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/tx/ClientTransactionDiscardRequest.java
+++
b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/tx/ClientTransactionDiscardRequest.java
@@ -63,7 +63,7 @@ public class ClientTransactionDiscardRequest {
if (table != null) {
ZonePartitionId replicationGroupId =
table.internalTable().targetReplicationGroupId(partId);
- enlistedPartitions.computeIfAbsent(replicationGroupId, k ->
new PendingTxPartitionEnlistment(null, 0))
+ enlistedPartitions.computeIfAbsent(replicationGroupId, k ->
new PendingTxPartitionEnlistment("UNUSED", 0))
.addTableId(tableId);
}
}
diff --git
a/modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java
b/modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java
index 8a43ac6c517..bbc257ff6d8 100644
---
a/modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java
+++
b/modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java
@@ -432,7 +432,7 @@ class TcpClientChannel implements ClientChannel,
ClientMessageHandler, ClientCon
try {
ClientMessageUnpacker unpacker = fut.join();
- return completedFuture(complete(payloadReader,
notificationFut, unpacker));
+ return completedFuture(complete(payloadReader,
notificationFut, unpacker, opCode));
} catch (Throwable t) {
expectedException = true;
throw sneakyThrow(ViewUtils.ensurePublicException(t));
@@ -443,7 +443,7 @@ class TcpClientChannel implements ClientChannel,
ClientMessageHandler, ClientCon
CompletableFuture<T> resFut = new CompletableFuture<>();
fut.handle((unpacker, err) -> {
- completeAsync(payloadReader, notificationFut, unpacker, err,
resFut);
+ completeAsync(payloadReader, notificationFut, unpacker, err,
resFut, opCode);
return null;
});
@@ -476,7 +476,8 @@ class TcpClientChannel implements ClientChannel,
ClientMessageHandler, ClientCon
@Nullable CompletableFuture<PayloadInputChannel> notificationFut,
ClientMessageUnpacker unpacker,
@Nullable Throwable err,
- CompletableFuture<T> resFut
+ CompletableFuture<T> resFut,
+ int opCode
) {
if (err != null) {
assert unpacker == null : "unpacker must be null if err is not
null";
@@ -497,7 +498,7 @@ class TcpClientChannel implements ClientChannel,
ClientMessageHandler, ClientCon
// With handleAsync et al we can't close the unpacker in that case.
asyncContinuationExecutor.execute(() -> {
try {
- resFut.complete(complete(payloadReader, notificationFut,
unpacker));
+ resFut.complete(complete(payloadReader, notificationFut,
unpacker, opCode));
} catch (Throwable t) {
resFut.completeExceptionally(ViewUtils.ensurePublicException(t));
}
@@ -516,11 +517,13 @@ class TcpClientChannel implements ClientChannel,
ClientMessageHandler, ClientCon
* @param payloadReader Payload reader.
* @param notificationFut Notify future.
* @param unpacker Unpacked message.
+ * @param opCode Op code.
*/
private <T> @Nullable T complete(
@Nullable PayloadReader<T> payloadReader,
@Nullable CompletableFuture<PayloadInputChannel> notificationFut,
- ClientMessageUnpacker unpacker
+ ClientMessageUnpacker unpacker,
+ int opCode
) {
try (unpacker) {
if (payloadReader != null) {
@@ -529,9 +532,10 @@ class TcpClientChannel implements ClientChannel,
ClientMessageHandler, ClientCon
return null;
} catch (Throwable e) {
- log.error("Failed to deserialize server response [remoteAddress="
+ cfg.getAddress() + "]: " + e.getMessage(), e);
+ log.error("Failed to deserialize server response [remoteAddress="
+ cfg.getAddress() + ", opCode=" + opCode + "]: "
+ + e.getMessage(), e);
- throw new IgniteException(PROTOCOL_ERR, "Failed to deserialize
server response: " + e.getMessage(), e);
+ throw new IgniteException(PROTOCOL_ERR, "Failed to deserialize
server response for op " + opCode + ": " + e.getMessage(), e);
}
}
@@ -736,7 +740,7 @@ class TcpClientChannel implements ClientChannel,
ClientMessageHandler, ClientCon
CompletableFuture<Object> resFut = new CompletableFuture<>();
fut.handle((unpacker, err) -> {
- completeAsync(r -> handshakeRes(r.in()), null, unpacker, err,
resFut);
+ completeAsync(r -> handshakeRes(r.in()), null, unpacker, err,
resFut, -1);
return null;
});
diff --git
a/modules/client/src/main/java/org/apache/ignite/internal/client/tx/DirectTxUtils.java
b/modules/client/src/main/java/org/apache/ignite/internal/client/tx/DirectTxUtils.java
index a74f25fe2a0..4932455ad5a 100644
---
a/modules/client/src/main/java/org/apache/ignite/internal/client/tx/DirectTxUtils.java
+++
b/modules/client/src/main/java/org/apache/ignite/internal/client/tx/DirectTxUtils.java
@@ -225,11 +225,11 @@ public class DirectTxUtils {
"Encountered no-op on first direct enlistment,
server version upgrade is required"));
}
} else {
- String consistentId = payloadChannel.in().unpackString();
- long token = payloadChannel.in().unpackLong();
+ String consistentId = in.unpackString();
+ long token = in.unpackLong();
// Test if no-op enlistment.
- if (payloadChannel.in().unpackBoolean()) {
+ if (in.unpackBoolean()) {
payloadChannel.clientChannel().inflights().removeInflight(tx.txId(), null);
}
diff --git
a/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
b/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
index f4853007dc8..72c68f3e3df 100644
---
a/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
+++
b/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
@@ -579,31 +579,32 @@ public class ClientKeyValueViewTest extends
AbstractClientTableTest {
@Test
public void testGetNullValueThrows() {
- testNullValueThrows(view -> view.get(null, DEFAULT_ID), "getNullable");
+ testNullValueThrows(view -> view.get(null, DEFAULT_ID), "getNullable",
12);
}
@Test
public void testGetAndPutNullValueThrows() {
- testNullValueThrows(view -> view.getAndPut(null, DEFAULT_ID,
DEFAULT_NAME), "getNullableAndPut");
+ testNullValueThrows(view -> view.getAndPut(null, DEFAULT_ID,
DEFAULT_NAME), "getNullableAndPut", 16);
}
@Test
public void testGetAndRemoveNullValueThrows() {
- testNullValueThrows(view -> view.getAndRemove(null, DEFAULT_ID),
"getNullableAndRemove");
+ testNullValueThrows(view -> view.getAndRemove(null, DEFAULT_ID),
"getNullableAndRemove", 32);
}
@Test
public void testGetAndReplaceNullValueThrows() {
- testNullValueThrows(view -> view.getAndReplace(null, DEFAULT_ID,
DEFAULT_NAME), "getNullableAndReplace");
+ testNullValueThrows(view -> view.getAndReplace(null, DEFAULT_ID,
DEFAULT_NAME), "getNullableAndReplace", 26);
}
- private void testNullValueThrows(Consumer<KeyValueView<Long, String>> run,
String methodName) {
+ private void testNullValueThrows(Consumer<KeyValueView<Long, String>> run,
String methodName, int op) {
KeyValueView<Long, String> primitiveView =
defaultTable().keyValueView(Mapper.of(Long.class), Mapper.of(String.class));
primitiveView.put(null, DEFAULT_ID, null);
var ex = assertThrowsWithCause(() -> run.accept(primitiveView),
UnexpectedNullValueException.class);
assertEquals(
- format("Failed to deserialize server response: Got unexpected
null value: use `{}` sibling method instead.", methodName),
+ format("Failed to deserialize server response for op {}: Got
unexpected null value: use `{}` sibling method instead.",
+ op, methodName),
ex.getMessage());
}
diff --git
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientSqlTest.java
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientSqlTest.java
index 0b2a21923c6..ec7973ea7cc 100644
---
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientSqlTest.java
+++
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientSqlTest.java
@@ -626,7 +626,7 @@ public class ItThinClientSqlTest extends
ItAbstractThinClientTest {
IgniteException.class,
() -> client().sql().execute((Transaction) null,
Mapper.of(Pojo.class), query));
- assertEquals("Failed to deserialize server response: No mapped object
field found for column 'FOO'", e.getMessage());
+ assertEquals("Failed to deserialize server response for op 50: No
mapped object field found for column 'FOO'", e.getMessage());
}
@Test
diff --git
a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/PartitionEnlistment.java
b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/PartitionEnlistment.java
index 1b09f56f9ff..4917b047c11 100644
---
a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/PartitionEnlistment.java
+++
b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/PartitionEnlistment.java
@@ -30,7 +30,15 @@ public class PartitionEnlistment {
@IgniteToStringInclude
protected final Set<Integer> tableIds;
+ /**
+ * Constructs a {@code PartitionEnlistment} instance.
+ *
+ * @param primaryNodeConsistentId The consistent ID of the primary node.
+ * @param tableIds A set of table IDs for which the partition is enlisted.
+ */
public PartitionEnlistment(String primaryNodeConsistentId, Set<Integer>
tableIds) {
+ assert primaryNodeConsistentId != null : "Primary node consistent ID
cannot be null";
+
this.primaryNodeConsistentId = primaryNodeConsistentId;
this.tableIds = tableIds;
}
