GEODE-10402: Fix FunctionException handling (#981)
* GEODE-10402: Fix FunctionException handling
- Fixed handling for FunctionException.
- Added InternalFunctionInvocationTargetException and replaced
GF_FUNCTION_EXCEPTION by
GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION, so function
exceptions are properly handled.
- Code modified to adapt to the above changes.
* GEODE-10402: Revision 1
- Actually added handling for FunctionException exception in
ThinClientRegion.
- Removed InternalFunctionInvocationTargetException as part of the
public API since this is part of the internal Java API.
- Added a new IT to verify that FunctionExceptions thrown by the user
are actually thrown in the native library as FunctionException's.
* GEODE-10402: Format code
* GEODE-10402: Revision 2
- Fixed clang-tidy warnings.
- Fixed format on one of the Java classes for testing.
diff --git a/cppcache/include/geode/ExceptionTypes.hpp b/cppcache/include/geode/ExceptionTypes.hpp
index e18ea63..783d6e0 100644
--- a/cppcache/include/geode/ExceptionTypes.hpp
+++ b/cppcache/include/geode/ExceptionTypes.hpp
@@ -681,14 +681,15 @@
/**
*@brief Thrown if function execution failed
**/
-class APACHE_GEODE_EXPORT FunctionExecutionException : public Exception {
+class APACHE_GEODE_EXPORT FunctionException : public Exception {
public:
using Exception::Exception;
- ~FunctionExecutionException() noexcept override {}
+ ~FunctionException() noexcept override {}
std::string getName() const override {
- return "apache::geode::client::FunctionExecutionException";
+ return "apache::geode::client::FunctionException";
}
};
+
/**
*@brief Thrown if the No locators are active to reply for new connection.
**/
@@ -850,6 +851,8 @@
}
};
+using FunctionExecutionException = FunctionException;
+
} // namespace client
} // namespace geode
} // namespace apache
diff --git a/cppcache/integration/test/FunctionExecutionTest.cpp b/cppcache/integration/test/FunctionExecutionTest.cpp
index bc509b8..fa4d3ce 100644
--- a/cppcache/integration/test/FunctionExecutionTest.cpp
+++ b/cppcache/integration/test/FunctionExecutionTest.cpp
@@ -44,12 +44,16 @@
using apache::geode::client::CacheImpl;
using apache::geode::client::CacheRegionHelper;
using apache::geode::client::Exception;
-using apache::geode::client::FunctionExecutionException;
+using apache::geode::client::FunctionException;
using apache::geode::client::FunctionService;
using apache::geode::client::NotConnectedException;
using apache::geode::client::Region;
using apache::geode::client::RegionShortcut;
using apache::geode::client::ResultCollector;
+
+using ::testing::Eq;
+using ::testing::Le;
+
const int ON_SERVERS_TEST_REGION_ENTRIES_SIZE = 34;
const int PARTITION_REGION_ENTRIES_SIZE = 113;
@@ -103,7 +107,7 @@
ASSERT_THROW(FunctionService::onServer(region->getRegionService())
.execute("I_Don_t_Exist"),
- FunctionExecutionException);
+ FunctionException);
}
TEST(FunctionExecutionTest, UnknownFunctionOnRegion) {
@@ -122,7 +126,7 @@
auto region = setupRegion(cache);
ASSERT_THROW(FunctionService::onRegion(region).execute("I_Don_t_Exist"),
- FunctionExecutionException);
+ FunctionException);
}
TEST(FunctionExecutionTest, UnknownFunctionAsyncOnServer) {
@@ -143,7 +147,7 @@
ASSERT_THROW(FunctionService::onServer(region->getRegionService())
.withCollector(std::make_shared<TestResultCollector>())
.execute("I_Don_t_Exist"),
- FunctionExecutionException);
+ FunctionException);
}
TEST(FunctionExecutionTest, UnknownFunctionAsyncOnRegion) {
@@ -164,7 +168,7 @@
ASSERT_THROW(FunctionService::onRegion(region)
.withCollector(std::make_shared<TestResultCollector>())
.execute("I_Don_t_Exist"),
- FunctionExecutionException);
+ FunctionException);
}
TEST(FunctionExecutionTest,
@@ -356,8 +360,7 @@
auto functionService = FunctionService::onRegion(region);
auto execute =
functionService.withCollector(std::make_shared<TestResultCollector>());
- ASSERT_THROW(execute.execute("MultiGetAllFunctionNonHA"),
- FunctionExecutionException);
+ ASSERT_THROW(execute.execute("MultiGetAllFunctionNonHA"), FunctionException);
}
TEST(FunctionExecutionTest,
@@ -415,8 +418,7 @@
auto execute =
functionService.withCollector(std::make_shared<TestResultCollector>())
.withFilter(filter);
- ASSERT_THROW(execute.execute("MultiGetAllFunctionNonHA"),
- FunctionExecutionException);
+ ASSERT_THROW(execute.execute("MultiGetAllFunctionNonHA"), FunctionException);
}
TEST(FunctionExecutionTest, OnServersWithReplicatedRegionsInPool) {
@@ -558,3 +560,35 @@
threadAux->join();
}
+
+TEST(FunctionExecutionTest, testUserFunctionExceptionThrowsRightException) {
+ Cluster cluster{
+ InitialLocators{{{"localhost", Framework::getAvailablePort()}}},
+ InitialServers{{{"localhost", Framework::getAvailablePort()},
+ {"localhost", Framework::getAvailablePort()},
+ {"localhost", Framework::getAvailablePort()}}}};
+
+ cluster.start([&]() {
+ cluster.getGfsh()
+ .deploy()
+ .jar(getFrameworkString(FrameworkVariable::JavaObjectJarPath))
+ .execute();
+ });
+
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("REPLICATE")
+ .execute();
+
+ auto cache = cluster.createCache();
+ auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+ .setPoolName("default")
+ .create("region");
+
+ auto functionService = FunctionService::onRegion(region);
+ auto execute =
+ functionService.withCollector(std::make_shared<TestResultCollector>());
+ EXPECT_THROW(execute.execute("UserExceptionFunction"), FunctionException);
+}
diff --git a/cppcache/integration/test/PdxJsonTypeTest.cpp b/cppcache/integration/test/PdxJsonTypeTest.cpp
index 9c79178..0d54094 100644
--- a/cppcache/integration/test/PdxJsonTypeTest.cpp
+++ b/cppcache/integration/test/PdxJsonTypeTest.cpp
@@ -45,9 +45,7 @@
using apache::geode::client::CacheableKey;
using apache::geode::client::CacheableString;
using apache::geode::client::CacheRegionHelper;
-using apache::geode::client::CacheServerException;
using apache::geode::client::FunctionService;
-using apache::geode::client::IllegalStateException;
using apache::geode::client::LocalRegion;
using apache::geode::client::PdxFieldTypes;
using apache::geode::client::PdxInstance;
diff --git a/cppcache/src/DefaultResultCollector.cpp b/cppcache/src/DefaultResultCollector.cpp
index c81c0c0..5da7dac 100644
--- a/cppcache/src/DefaultResultCollector.cpp
+++ b/cppcache/src/DefaultResultCollector.cpp
@@ -32,7 +32,7 @@
return resultList;
}
- throw FunctionExecutionException(
+ throw FunctionException(
"Result is not ready, endResults callback is called before invoking "
"getResult() method");
}
diff --git a/cppcache/src/ErrType.hpp b/cppcache/src/ErrType.hpp
index e15c38a..2dbaab9 100644
--- a/cppcache/src/ErrType.hpp
+++ b/cppcache/src/ErrType.hpp
@@ -103,7 +103,7 @@
GF_CACHE_ENTRY_UPDATED = 133,
GF_CACHE_LOCATOR_EXCEPTION = 134, /** Exception in Locator */
GF_INVALID_DELTA = 135,
- GF_FUNCTION_EXCEPTION = 136,
+ GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION = 136,
GF_ROLLBACK_EXCEPTION = 137,
GF_COMMIT_CONFLICT_EXCEPTION = 138,
GF_TRANSACTION_DATA_NODE_HAS_DEPARTED_EXCEPTION = 139,
@@ -111,6 +111,7 @@
GF_PUTALL_PARTIAL_RESULT_EXCEPTION = 141,
GF_LOW_MEMORY_EXCEPTION = 142,
GF_QUERY_EXECUTION_LOW_MEMORY_EXCEPTION = 143,
+ GF_FUNCTION_EXCEPTION = 144,
GF_EUNDEF = 999 /**< unknown exception */
} GfErrType;
diff --git a/cppcache/src/ExceptionTypes.cpp b/cppcache/src/ExceptionTypes.cpp
index 252a0ee..9f8033b 100644
--- a/cppcache/src/ExceptionTypes.cpp
+++ b/cppcache/src/ExceptionTypes.cpp
@@ -42,7 +42,7 @@
using apache::geode::client::EntryDestroyedException;
using apache::geode::client::EntryExistsException;
using apache::geode::client::EntryNotFoundException;
-using apache::geode::client::FunctionExecutionException;
+using apache::geode::client::FunctionException;
using apache::geode::client::GeodeIOException;
using apache::geode::client::IllegalArgumentException;
using apache::geode::client::IllegalStateException;
@@ -316,11 +316,11 @@
throw AllConnectionsInUseException{message};
}
-[[noreturn]] void functionExecutionException(std::string message,
- std::string& exMsg, GfErrType,
- std::string) {
+[[noreturn]] void functionException(std::string message,
+ const std::string& exMsg, GfErrType,
+ std::string) {
message.append(!exMsg.empty() ? exMsg : ": Function execution failed");
- throw FunctionExecutionException{message};
+ throw FunctionException{message};
}
[[noreturn]] void diskFailureException(std::string message, std::string& exMsg,
@@ -440,7 +440,8 @@
{GF_REMOTE_QUERY_EXCEPTION, queryException},
{GF_CACHE_LOCATOR_EXCEPTION, noAvailableLocatorsException},
{GF_ALL_CONNECTIONS_IN_USE_EXCEPTION, allConnectionsInUseException},
- {GF_FUNCTION_EXCEPTION, functionExecutionException},
+ {GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION, functionException},
+ {GF_FUNCTION_EXCEPTION, functionException},
{GF_DISKFULL, diskFailureException},
{GF_ROLLBACK_EXCEPTION, rollbackException},
{GF_COMMIT_CONFLICT_EXCEPTION, commitConflictException},
diff --git a/cppcache/src/ExecutionImpl.cpp b/cppcache/src/ExecutionImpl.cpp
index c35fb19..d6d38bc 100644
--- a/cppcache/src/ExecutionImpl.cpp
+++ b/cppcache/src/ExecutionImpl.cpp
@@ -381,7 +381,7 @@
}
case TcrMessage::REQUEST_DATA_ERROR: {
LOGERROR("Error message from server: " + reply.getValue()->toString());
- throw FunctionExecutionException(reply.getValue()->toString());
+ throw FunctionException(reply.getValue()->toString());
}
default: {
LOGERROR("Unknown message type %d while getting function attributes.",
@@ -427,7 +427,7 @@
} else {
message = "Execute: failed to execute function with server.";
}
- throw FunctionExecutionException(message);
+ throw FunctionException(message);
} else {
throwExceptionIfError("Execute", err);
}
diff --git a/cppcache/src/NoResult.hpp b/cppcache/src/NoResult.hpp
index 119cc66..161ef0f 100644
--- a/cppcache/src/NoResult.hpp
+++ b/cppcache/src/NoResult.hpp
@@ -52,7 +52,7 @@
inline std::shared_ptr<CacheableVector> getResult(
std::chrono::milliseconds) final {
- throw FunctionExecutionException(
+ throw FunctionException(
"Cannot return any result, as Function.hasResult() is false");
}
diff --git a/cppcache/src/ThinClientRegion.cpp b/cppcache/src/ThinClientRegion.cpp
index 682e01a..3957514 100644
--- a/cppcache/src/ThinClientRegion.cpp
+++ b/cppcache/src/ThinClientRegion.cpp
@@ -2648,6 +2648,13 @@
} else if (exceptionMsg.find("org.apache.geode.internal.cache.execute."
"InternalFunctionInvocationTargetException") !=
std::string::npos) {
+ error = GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION;
+ } else if (exceptionMsg.find("org.apache.geode.internal.cache.execute."
+ "InternalFunctionInvocationTargetException") !=
+ std::string::npos) {
+ error = GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION;
+ } else if (exceptionMsg.find("org.apache.geode.cache.execute."
+ "FunctionException") != std::string::npos) {
error = GF_FUNCTION_EXCEPTION;
} else if (exceptionMsg.find(
"org.apache.geode.cache.CommitConflictException") !=
@@ -2881,15 +2888,16 @@
if (ThinClientBaseDM::isFatalClientError(err)) {
throwExceptionIfError("ExecuteOnRegion:", err);
} else if (err != GF_NOERR) {
- if (err == GF_FUNCTION_EXCEPTION) {
+ if (err == GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION) {
reExecute = true;
rc->clearResults();
std::shared_ptr<CacheableHashSet> failedNodesIds(reply.getFailedNode());
failedNodes->clear();
if (failedNodesIds) {
LOGDEBUG(
- "ThinClientRegion::executeFunction with GF_FUNCTION_EXCEPTION "
- "failedNodesIds size = %zu ",
+ "ThinClientRegion::executeFunction with "
+ "GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION failedNodesIds "
+ "size = %zu ",
failedNodesIds->size());
failedNodes->insert(failedNodesIds->begin(), failedNodesIds->end());
}
@@ -2975,7 +2983,7 @@
if (ThinClientBaseDM::isFatalClientError(err)) {
throwExceptionIfError("ExecuteOnRegion:", err);
} else if (err != GF_NOERR) {
- if (err == GF_FUNCTION_EXCEPTION) {
+ if (err == GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION) {
reExecute = true;
rc->clearResults();
std::shared_ptr<CacheableHashSet> failedNodesIds(reply.getFailedNode());
@@ -2983,8 +2991,8 @@
if (failedNodesIds) {
LOGDEBUG(
"ThinClientRegion::reExecuteFunction with "
- "GF_FUNCTION_EXCEPTION "
- "failedNodesIds size = %zu ",
+ "GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION failedNodesIds "
+ "size = %zu ",
failedNodesIds->size());
failedNodes->insert(failedNodesIds->begin(), failedNodesIds->end());
}
@@ -3053,7 +3061,7 @@
}
if (err != GF_NOERR) {
- if (err == GF_FUNCTION_EXCEPTION) {
+ if (err == GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION) {
if (auto poolDM =
std::dynamic_pointer_cast<ThinClientPoolDM>(m_tcrdm)) {
if (poolDM->getClientMetaDataService()) {
@@ -3077,7 +3085,7 @@
if (failedNodeIds) {
LOGDEBUG(
"ThinClientRegion::executeFunctionSH with "
- "GF_FUNCTION_EXCEPTION "
+ "GF_INTERNAL_FUNCTION_INVOCATION_TARGET_EXCEPTION "
"failedNodeIds size = %zu ",
failedNodeIds->size());
failedNodes->insert(failedNodeIds->begin(), failedNodeIds->end());
@@ -3153,7 +3161,7 @@
}
case TcrMessage::REQUEST_DATA_ERROR: {
LOGERROR("Error message from server: " + reply.getValue()->toString());
- throw FunctionExecutionException(reply.getValue()->toString());
+ throw FunctionException(reply.getValue()->toString());
}
default: {
LOGERROR("Unknown message type %d while getting function attributes.",
diff --git a/tests/javaobject/UserExceptionFunction.java b/tests/javaobject/UserExceptionFunction.java
new file mode 100644
index 0000000..59af51e
--- /dev/null
+++ b/tests/javaobject/UserExceptionFunction.java
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package javaobject;
+
+import java.io.Serializable;
+
+import org.apache.geode.cache.Declarable;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.execute.FunctionAdapter;
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.execute.FunctionException;
+import org.apache.geode.cache.execute.RegionFunctionContext;
+
+import java.util.Properties;
+
+/**
+ * Function that returns the value of the key passed in through the filter,
+ * using the default result collector.
+ */
+public class UserExceptionFunction extends FunctionAdapter implements Declarable {
+
+ private static final String ID = "GetKeyFunction";
+
+ public void execute(FunctionContext context) {
+ throw new FunctionException("This is an user expected exception");
+ }
+
+ public String getId() {
+ return ID;
+ }
+
+ public boolean hasResult() {
+ return true;
+ }
+
+ public void init(Properties p) {
+ }
+ public boolean isHA() {
+ return false;
+ }
+
+}
+
