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
Change-Id: I71829e04d7642cfd4d1c54596f1fc7670db0d292
Signed-off-by: Jamo Luhrsen <jluhrsen@gmail.com>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(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 15d4dfe443882b280f9a6f83db4d4db93b3ea9d5..ee8b03be58b49ca344bc325acf6f0e450c22d998 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
@@ -209,33 +209,44 @@ public class DocumentedException extends Exception {
         ErrorSeverity errorSeverity = ErrorSeverity.ERROR;
         Map<String, String> errorInfo = null;
         String errorMessage = "";
+        StringBuilder allErrorMessages = new StringBuilder();
 
         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.append(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.toString());
         }
 
         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 06ffd70e92682d67daf237f4337600f3756c38a9..3c9c604cd61149ab0b48be0c3bac96222f6164e6 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
@@ -282,6 +282,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 + "\">"