ptupitsyn commented on a change in pull request #8169:
URL: https://github.com/apache/ignite/pull/8169#discussion_r472998608
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/platform/client/ClientRequestHandler.java
##########
@@ -115,7 +116,7 @@
int status = e instanceof IgniteClientException ?
((IgniteClientException)e).statusCode() : ClientStatus.FAILED;
- return new ClientResponse(req.requestId(), status, e.getMessage());
+ return new ClientResponse(req.requestId(), status,
X.getFullStackTrace(e));
Review comment:
We do not include full stack traces into thin client responses. Thin
client users are supposed to check server logs (if they have access) to
understand the issue when error is returned.
##########
File path:
modules/core/src/test/java/org/apache/ignite/client/IgniteBinaryTest.java
##########
@@ -123,6 +130,59 @@ public void testBinaryObjectPutGet() throws Exception {
}
}
+ /**
+ * Tests that {@code
org.apache.ignite.cache.CacheInterceptor#onBeforePut(javax.cache.Cache.Entry,
java.lang.Object)}
+ * throws correct exception in case while cache operations are called from
thin client. Only BinaryObject`s are
+ * acceptable in this case.
+ */
+ @Test
+ public void testBinaryWithNotGenericInterceptor() throws Exception {
+ IgniteConfiguration ccfg = Config.getServerConfiguration()
+ .setCacheConfiguration(new
CacheConfiguration("test").setInterceptor(new ThinBinaryValueInterceptor()));
+
+ String castErr = "cannot be cast to";
+ String treeErr = "Tree is corrupted";
+
+ ListeningTestLogger srvLog = new ListeningTestLogger(log);
+
+ LogListener lsnrCast = LogListener.matches(castErr).
+ andMatches(str -> !str.contains(treeErr)).build();
+
+ srvLog.registerListener(lsnrCast);
+
+ ccfg.setGridLogger(srvLog);
+
+ try (Ignite ignored = Ignition.start(ccfg)) {
+ try (IgniteClient client =
+ Ignition.startClient(new
ClientConfiguration().setAddresses(Config.SERVER))
+ ) {
+ ClientCache<Integer, ThinBinaryValue> cache =
client.cache("test");
+
+ try {
+ cache.put(1, new ThinBinaryValue());
+ }
+ catch (Exception e) {
+ assertTrue(X.getFullStackTrace(e).contains(castErr));
Review comment:
We are mostly interested that exception message makes sense. Top-level
exception should be easy to understand. Please add an assertion for this.
##########
File path:
modules/core/src/test/java/org/apache/ignite/client/IgniteBinaryTest.java
##########
@@ -123,6 +130,59 @@ public void testBinaryObjectPutGet() throws Exception {
}
}
+ /**
+ * Tests that {@code
org.apache.ignite.cache.CacheInterceptor#onBeforePut(javax.cache.Cache.Entry,
java.lang.Object)}
+ * throws correct exception in case while cache operations are called from
thin client. Only BinaryObject`s are
+ * acceptable in this case.
+ */
+ @Test
+ public void testBinaryWithNotGenericInterceptor() throws Exception {
+ IgniteConfiguration ccfg = Config.getServerConfiguration()
+ .setCacheConfiguration(new
CacheConfiguration("test").setInterceptor(new ThinBinaryValueInterceptor()));
+
+ String castErr = "cannot be cast to";
+ String treeErr = "Tree is corrupted";
+
+ ListeningTestLogger srvLog = new ListeningTestLogger(log);
+
+ LogListener lsnrCast = LogListener.matches(castErr).
+ andMatches(str -> !str.contains(treeErr)).build();
+
+ srvLog.registerListener(lsnrCast);
+
+ ccfg.setGridLogger(srvLog);
+
+ try (Ignite ignored = Ignition.start(ccfg)) {
+ try (IgniteClient client =
+ Ignition.startClient(new
ClientConfiguration().setAddresses(Config.SERVER))
+ ) {
+ ClientCache<Integer, ThinBinaryValue> cache =
client.cache("test");
+
+ try {
+ cache.put(1, new ThinBinaryValue());
Review comment:
When there is no exception, this test won't fail and won't check
anything.
Please either insert `assert false`, or use one of the
`GridTestUtils.assertThrows` helpers.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
