Catch exception when transformer fails to parse

There is a deadlock if a transformer fails to parse a reply
and uses that to invoke the rpc. The future gets stuck and
never completes. This catches the case of a bad toRpcResult
from the transformer and marks the future with a
DefaultDOMRpcException

JIRA: NETCONF-695
Signed-off-by: Jamo Luhrsen <jluhrsen@gmail.com>
Change-Id: Ied3e0ac4b08e128c41f54e01df91543e4d3fc3a4

diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceRpc.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceRpc.java
index 19de2565df65ff525b9ec31882d376f4a0bac3b6..990bc242a47ca7ab748c41fc592e03e76e9d64f8 100644 (file)
--- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceRpc.java
+++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceRpc.java
@@ -20,6 +20,7 @@ import org.opendaylight.mdsal.dom.api.DOMRpcIdentifier;
 import org.opendaylight.mdsal.dom.api.DOMRpcImplementationNotAvailableException;
 import org.opendaylight.mdsal.dom.api.DOMRpcResult;
 import org.opendaylight.mdsal.dom.api.DOMRpcService;
+import org.opendaylight.mdsal.dom.api.DefaultDOMRpcException;
 import org.opendaylight.mdsal.dom.spi.DefaultDOMRpcResult;
 import org.opendaylight.netconf.api.NetconfMessage;
 import org.opendaylight.netconf.sal.connect.api.MessageTransformer;
@@ -49,6 +50,7 @@ public final class NetconfDeviceRpc implements DOMRpcService {
     }
 
     @Override
+    @SuppressWarnings("checkstyle:IllegalCatch")
     public ListenableFuture<DOMRpcResult> invokeRpc(final SchemaPath type, final NormalizedNode<?, ?> input) {
         final ListenableFuture<RpcResult<NetconfMessage>> delegateFuture = communicator.sendRequest(
             transformer.toRpcRequest(type, input), type.getLastComponent());
@@ -57,8 +59,13 @@ public final class NetconfDeviceRpc implements DOMRpcService {
         Futures.addCallback(delegateFuture, new FutureCallback<RpcResult<NetconfMessage>>() {
             @Override
             public void onSuccess(final RpcResult<NetconfMessage> result) {
-                ret.set(result.isSuccessful() ? transformer.toRpcResult(result.getResult(), type)
-                        : new DefaultDOMRpcResult(result.getErrors()));
+                try {
+                    ret.set(result.isSuccessful() ? transformer.toRpcResult(result.getResult(), type)
+                            : new DefaultDOMRpcResult(result.getErrors()));
+                } catch (Exception cause) {
+                    ret.setException(new DefaultDOMRpcException(
+                            "Unable to parse rpc reply. type: " + type + " input: " + input, cause));
+                }
             }
 
             @Override

diff --git a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceRpcTest.java b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceRpcTest.java
index 9fdc31de1be17394d0f44719423ff5dede7d1917..297eb84d81f4ae041801da78f49371bf7cb1c81d 100644 (file)
--- a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceRpcTest.java
+++ b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceRpcTest.java
@@ -8,14 +8,18 @@
 package org.opendaylight.netconf.sal.connect.netconf.sal;
 
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import com.google.common.util.concurrent.Futures;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Set;
+import java.util.concurrent.ExecutionException;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.Before;
@@ -30,6 +34,7 @@ import org.opendaylight.mdsal.dom.api.DOMRpcIdentifier;
 import org.opendaylight.mdsal.dom.api.DOMRpcResult;
 import org.opendaylight.netconf.api.NetconfMessage;
 import org.opendaylight.netconf.api.xml.XmlUtil;
+import org.opendaylight.netconf.sal.connect.api.MessageTransformer;
 import org.opendaylight.netconf.sal.connect.api.RemoteDeviceCommunicator;
 import org.opendaylight.netconf.sal.connect.netconf.schema.mapping.NetconfMessageTransformer;
 import org.opendaylight.netconf.sal.connect.netconf.util.NetconfMessageTransformUtil;
@@ -94,6 +99,20 @@ public class NetconfDeviceRpcTest {
         expectedReply = transformer.toRpcResult(reply, path);
     }
 
+    @Test
+    public void testDeadlock() throws Exception {
+        // when rpc is successful, but transformer fails for some reason
+        final MessageTransformer<NetconfMessage> failingTransformer = mock(MessageTransformer.class);
+        final RemoteDeviceCommunicator<NetconfMessage> communicatorMock = mock(RemoteDeviceCommunicator.class);
+        final NetconfMessage msg = null;
+        final RpcResult<NetconfMessage> result = RpcResultBuilder.success(msg).build();
+        when(communicatorMock.sendRequest(any(), any())).thenReturn(Futures.immediateFuture(result));
+        when(failingTransformer.toRpcResult(any(), any())).thenThrow(new RuntimeException("FAIL"));
+        final NetconfDeviceRpc failingRpc = new NetconfDeviceRpc(SCHEMA_CONTEXT, communicatorMock, failingTransformer);
+        assertThrows(ExecutionException.class, () -> failingRpc.invokeRpc(path, mock(ContainerNode.class)).get());
+        assertThrows(ExecutionException.class, () -> failingRpc.invokeRpc(path, null).get());
+    }
+
     @Test
     public void testInvokeRpc() throws Exception {
         NormalizedNode<?, ?> input = createNode("urn:ietf:params:xml:ns:netconf:base:1.0", "2011-06-01", "filter");