Handle multiple rpc-error in the same rpc-reply

When multiple rpc-error is seen in a reply it was not being
caught as an error and eventually falling through the cracks
to show up as an IllegalArgumentException.

This also adds a hack/workaround for passing back all the
error messages found from each of the multiple rpc-error(s)

Also, instead of this case not being recognized as an
rpc-error, it was causing the request to wait until
the device request timeout to expire at which point it's
noted as a full RPC failure and the device is disconnected
and reconnected. Now it is recognized as an error and the
response is handled as soon as it's received and there is
no connection flap.

JIRA: NETCONF-666
Signed-off-by: Jamo Luhrsen <jluhrsen@gmail.com>
Change-Id: I71829e04d7642cfd4d1c54596f1fc7670db0d292
(cherry picked from commit 8b2fc27e338bdd9cea9674204c1ec3d40a0c68c4)

diff --git a/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/DocumentedException.java b/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/DocumentedException.java
index 907fd2fb5d2b86c65072ce5c472c41c726010a95..1d906c4f3c37881c5a8a6ddd38b96101617f8768 100644 (file)
--- a/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/DocumentedException.java
+++ b/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/DocumentedException.java
@@ -208,33 +208,44 @@ public class DocumentedException extends Exception {
         ErrorSeverity errorSeverity = ErrorSeverity.ERROR;
         Map<String, String> errorInfo = null;
         String errorMessage = "";
+        String allErrorMessages = "";
 
         Node rpcReply = fromDoc.getDocumentElement();
 
-        // FIXME: BUG? - we only handle one rpc-error.
+        // FIXME: BUG? - we only handle one rpc-error. For now, shove extra errorMessages
+        // found in multiple rpc-error in the errorInfo Map to at least let them propagate
+        // back to caller.
+        int rpcErrorCount = 0;
 
         NodeList replyChildren = rpcReply.getChildNodes();
         for (int i = 0; i < replyChildren.getLength(); i++) {
             Node replyChild = replyChildren.item(i);
-            if (RPC_ERROR.equals(replyChild.getNodeName())) {
+            if (RPC_ERROR.equals(replyChild.getLocalName())) {
+                rpcErrorCount++;
                 NodeList rpcErrorChildren = replyChild.getChildNodes();
                 for (int j = 0; j < rpcErrorChildren.getLength(); j++) {
                     Node rpcErrorChild = rpcErrorChildren.item(j);
-                    if (ERROR_TYPE.equals(rpcErrorChild.getNodeName())) {
+                    if (ERROR_TYPE.equals(rpcErrorChild.getLocalName())) {
                         errorType = ErrorType.from(rpcErrorChild.getTextContent());
-                    } else if (ERROR_TAG.equals(rpcErrorChild.getNodeName())) {
+                    } else if (ERROR_TAG.equals(rpcErrorChild.getLocalName())) {
                         errorTag = ErrorTag.from(rpcErrorChild.getTextContent());
-                    } else if (ERROR_SEVERITY.equals(rpcErrorChild.getNodeName())) {
+                    } else if (ERROR_SEVERITY.equals(rpcErrorChild.getLocalName())) {
                         errorSeverity = ErrorSeverity.from(rpcErrorChild.getTextContent());
-                    } else if (ERROR_MESSAGE.equals(rpcErrorChild.getNodeName())) {
+                    } else if (ERROR_MESSAGE.equals(rpcErrorChild.getLocalName())) {
                         errorMessage = rpcErrorChild.getTextContent();
-                    } else if (ERROR_INFO.equals(rpcErrorChild.getNodeName())) {
+                        allErrorMessages = allErrorMessages + errorMessage;
+                    } else if (ERROR_INFO.equals(rpcErrorChild.getLocalName())) {
                         errorInfo = parseErrorInfo(rpcErrorChild);
                     }
                 }
+            }
+        }
 
-                break;
+        if (rpcErrorCount > 1) {
+            if (errorInfo == null) {
+                errorInfo = new HashMap<>();
             }
+            errorInfo.put("Multiple Errors Found", allErrorMessages);
         }
 
         return new DocumentedException(errorMessage, errorType, errorTag, errorSeverity, errorInfo);

diff --git a/netconf/netconf-util/src/main/java/org/opendaylight/netconf/util/messages/NetconfMessageUtil.java b/netconf/netconf-util/src/main/java/org/opendaylight/netconf/util/messages/NetconfMessageUtil.java
index f4795aea98995d02c0051b5cae024e5c1b8e8a68..a32cd277b6873ca69b354b461ff3de99dcf53e76 100644 (file)
--- a/netconf/netconf-util/src/main/java/org/opendaylight/netconf/util/messages/NetconfMessageUtil.java
+++ b/netconf/netconf-util/src/main/java/org/opendaylight/netconf/util/messages/NetconfMessageUtil.java
@@ -55,7 +55,15 @@ public final class NetconfMessageUtil {
     }
 
     public static boolean isErrorMessage(final XmlElement xmlElement) throws NetconfDocumentedException {
+
+        // In the case of multiple rpc-error messages, size will not be 1 but we still want to report as Error
         if (xmlElement.getChildElements().size() != 1) {
+            List<XmlElement> allResults = xmlElement.getChildElements();
+            for (XmlElement result : allResults) {
+                if (result.getName().equals(DocumentedException.RPC_ERROR)) {
+                    return true;
+                }
+            }
             return false;
         }
         try {

diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/AbstractWriteTx.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/AbstractWriteTx.java
index 874ad32ff2e77cba99e0b9afdba558c30c06c2d8..302962759181b550d4e4dbc881d17c90d4f488c6 100644 (file)
--- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/AbstractWriteTx.java
+++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/sal/tx/AbstractWriteTx.java
@@ -285,6 +285,7 @@ public abstract class AbstractWriteTx implements DOMDataTreeWriteTransaction {
                         break;
                 }
                 msgBuilder.append(error.getMessage());
+                msgBuilder.append(error.getInfo());
                 errorTag = error.getTag();
             }
         }

diff --git a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicatorTest.java b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicatorTest.java
index 0771258da80e7af34ef99909bcabba8e7ce258dc..942c31d25c66cdb7e77e21ac906407634d1aa54d 100644 (file)
--- a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicatorTest.java
+++ b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicatorTest.java
@@ -359,6 +359,29 @@ public class NetconfDeviceCommunicatorTest {
         assertTrue("Error info contains \"bar\"", errorInfo.contains("<bad-element>bar</bad-element>"));
     }
 
+    @Test
+    public void testOnResponseMessageWithMultipleErrors() throws Exception {
+        setupSession();
+
+        String messageID = UUID.randomUUID().toString();
+        ListenableFuture<RpcResult<NetconfMessage>> resultFuture = sendRequest(messageID, true);
+
+        communicator.onMessage(mockSession, createMultiErrorResponseMessage(messageID));
+
+        RpcError rpcError = verifyErrorRpcResult(resultFuture.get(), RpcError.ErrorType.PROTOCOL,
+                "operation-failed");
+
+        String errorInfo = rpcError.getInfo();
+        assertNotNull("RpcError info is null", errorInfo);
+
+        String errorInfoMessages = rpcError.getInfo();
+        String errMsg1 = "Number of member links configured, i.e [1], "
+                + "for interface [ae0]is lesser than the required minimum [2].";
+        String errMsg2 = "configuration check-out failed";
+        assertTrue(String.format("Error info contains \"%s\" or \"%s\'", errMsg1, errMsg2),
+                errorInfoMessages.contains(errMsg1) && errorInfoMessages.contains(errMsg2));
+    }
+
     /**
      * Test whether reconnect is scheduled properly.
      */
@@ -451,6 +474,37 @@ public class NetconfDeviceCommunicatorTest {
         assertNotNull("ListenableFuture is null", resultFuture);
     }
 
+    private static NetconfMessage createMultiErrorResponseMessage(final String messageID) throws Exception {
+        // multiple rpc-errors which simulate actual response like in NETCONF-666
+        String xmlStr = "<nc:rpc-reply xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\" xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\" xmlns:junos=\"http://xml.juniper.net/junos/18.4R1/junos\""
+                + "           message-id=\"" + messageID + "\">"
+                + "<nc:rpc-error>\n"
+                + "<nc:error-type>protocol</nc:error-type>\n"
+                + "<nc:error-tag>operation-failed</nc:error-tag>\n"
+                + "<nc:error-severity>error</nc:error-severity>\n"
+                + "<source-daemon>\n"
+                + "dcd\n"
+                + "</source-daemon>\n"
+                + "<nc:error-message>\n"
+                + "Number of member links configured, i.e [1], "
+                + "for interface [ae0]is lesser than the required minimum [2].\n"
+                + "</nc:error-message>\n"
+                + "</nc:rpc-error>\n"
+                + "<nc:rpc-error>\n"
+                + "<nc:error-type>protocol</nc:error-type>\n"
+                + "<nc:error-tag>operation-failed</nc:error-tag>\n"
+                + "<nc:error-severity>error</nc:error-severity>\n"
+                + "<nc:error-message>\n"
+                + "configuration check-out failed\n"
+                + "</nc:error-message>\n"
+                + "</nc:rpc-error>\n"
+                + "</nc:rpc-reply>";
+
+        ByteArrayInputStream bis = new ByteArrayInputStream(xmlStr.getBytes());
+        Document doc = UntrustedXML.newDocumentBuilder().parse(bis);
+        return new NetconfMessage(doc);
+    }
+
     private static NetconfMessage createErrorResponseMessage(final String messageID) throws Exception {
         String xmlStr = "<rpc-reply xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\""
                 + "           message-id=\"" + messageID + "\">"