GEODE-5884: Adding to function exception list if cause is FunctionInv…

…ocationTargetException (apache#2809)

geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionOp.java

@@ -335,6 +335,7 @@ static class ExecuteRegionFunctionOpImpl extends AbstractOp {
 
     private FunctionException functionException;
 
+
     public ExecuteRegionFunctionOpImpl(String region, Function function,
         ServerRegionFunctionExecutor serverRegionExecutor, ResultCollector rc, byte hasResult,
         Set<String> removedNodes) {
@@ -378,6 +379,22 @@ public ExecuteRegionFunctionOpImpl(String region, Function function,
       this.isHA = function.isHA();
     }
 
+    // For testing only
+    ExecuteRegionFunctionOpImpl() {
+      super(MessageType.EXECUTE_REGION_FUNCTION,
+          0);
+      resultCollector = null;
+      function = null;
+      isReExecute = (byte) 0;
+      regionName = "";
+      executor = null;
+      hasResult = (byte) 0;
+      failedNodes = null;
+      functionId = null;
+      executeOnBucketSet = true;
+      isHA = true;
+    }
+
     public ExecuteRegionFunctionOpImpl(String region, String function,
         ServerRegionFunctionExecutor serverRegionExecutor, ResultCollector rc, byte hasResult,
         Set<String> removedNodes, boolean isHA, boolean optimizeForWrite,
@@ -640,13 +657,13 @@ protected Object processResponse(Message msg) throws Exception {
       return null;
     }
 
-    private void addFunctionException(final FunctionException result) {
-      if (result instanceof FunctionInvocationTargetException) {
+    void addFunctionException(final FunctionException result) {
+      if (result.getCause() instanceof FunctionInvocationTargetException) {
         if (this.functionException == null) {
-          this.functionException = new FunctionException(result);
+          this.functionException = result;
         }
-        this.functionException.addException(result);
-      } else if (result instanceof InternalFunctionInvocationTargetException) {
+        this.functionException.addException(result.getCause());
+      } else if (result instanceof FunctionInvocationTargetException) {
         if (this.functionException == null) {
           this.functionException = new FunctionException(result);
         }
@@ -659,6 +676,10 @@ private void addFunctionException(final FunctionException result) {
       }
     }
 
+    FunctionException getFunctionException() {
+      return functionException;
+    }
+
     @Override
     protected boolean isErrorResponse(int msgType) {
       return msgType == MessageType.EXECUTE_REGION_FUNCTION_ERROR;

geode-core/src/test/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionOpTest.java

@@ -0,0 +1,63 @@
+/*
+ * 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 org.apache.geode.cache.client.internal;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.execute.FunctionException;
+import org.apache.geode.cache.execute.FunctionInvocationTargetException;
+import org.apache.geode.internal.cache.execute.InternalFunctionInvocationTargetException;
+import org.apache.geode.test.junit.categories.ClientServerTest;
+
+@Category({ClientServerTest.class})
+public class ExecuteRegionFunctionOpTest {
+
+  @Test
+  public void addFunctionExceptionWithFunctionTargetInvocationExceptionWrapsInPlainFunctionException() {
+    FunctionInvocationTargetException exception = mock(FunctionInvocationTargetException.class);
+    ExecuteRegionFunctionOp.ExecuteRegionFunctionOpImpl op =
+        new ExecuteRegionFunctionOp.ExecuteRegionFunctionOpImpl();
+    op.addFunctionException(exception);
+    assertThat(op.getFunctionException()).isInstanceOf(FunctionException.class);
+    assertThat(op.getFunctionException()).isNotInstanceOf(FunctionInvocationTargetException.class);
+  }
+
+  @Test
+  public void addFunctionExceptionWithInternalFunctionTargetInvocationExceptionWrapsInPlainFunctionException() {
+    FunctionInvocationTargetException exception =
+        mock(InternalFunctionInvocationTargetException.class);
+    ExecuteRegionFunctionOp.ExecuteRegionFunctionOpImpl op =
+        new ExecuteRegionFunctionOp.ExecuteRegionFunctionOpImpl();
+    op.addFunctionException(exception);
+    assertThat(op.getFunctionException()).isInstanceOf(FunctionException.class);
+    assertThat(op.getFunctionException())
+        .isNotInstanceOf(InternalFunctionInvocationTargetException.class);
+  }
+
+  @Test
+  public void addFunctionExceptionWithCauseFunctionTargetInvocationExceptionAddsToListOfException() {
+    FunctionInvocationTargetException cause = mock(FunctionInvocationTargetException.class);
+    FunctionException exception = new FunctionException(cause);
+    ExecuteRegionFunctionOp.ExecuteRegionFunctionOpImpl op =
+        new ExecuteRegionFunctionOp.ExecuteRegionFunctionOpImpl();
+    op.addFunctionException(exception);
+    assertThat(op.getFunctionException()).isInstanceOf(FunctionException.class);
+    assertThat(op.getFunctionException().getExceptions()).contains(cause);
+  }
+}