Guard against RPC/action implementation throwing

When an RPC or action implementation throws an exception, we should not
just fail -- we need to intercept it and report it in the RPC future.

JIRA: MDSAL-873
Change-Id: Idc15cba076b82e01500a06b1338f05256d186208
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/OperationInvocation.java b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/OperationInvocation.java
index 032a2d7752773860e5a07f5c7f1e8301b8fcf199..e99abdd93547632d9644c99e260a871161627b44 100644 (file)
--- a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/OperationInvocation.java
+++ b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/OperationInvocation.java
@@ -9,11 +9,14 @@ package org.opendaylight.mdsal.dom.broker;
 
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
+import org.opendaylight.mdsal.dom.api.DOMActionImplementation;
 import org.opendaylight.mdsal.dom.api.DOMActionNotAvailableException;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier;
 import org.opendaylight.mdsal.dom.api.DOMRpcIdentifier;
+import org.opendaylight.mdsal.dom.api.DOMRpcImplementation;
 import org.opendaylight.mdsal.dom.api.DOMRpcImplementationNotAvailableException;
 import org.opendaylight.mdsal.dom.api.DOMRpcResult;
+import org.opendaylight.mdsal.dom.api.DefaultDOMRpcException;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodes;
@@ -39,7 +42,7 @@ final class OperationInvocation {
             }
         }
 
-        return impls.get(0).invokeAction(type, path, input);
+        return invokeAction(impls.get(0), type, path, input);
     }
 
     static ListenableFuture<? extends DOMRpcResult> invoke(final AbstractDOMRpcRoutingTableEntry entry,
@@ -68,8 +71,7 @@ final class OperationInvocation {
                 // Find a DOMRpcImplementation for a specific iid
                 final var specificImpls = entry.getImplementations(iid);
                 if (specificImpls != null) {
-                    return specificImpls.get(0)
-                        .invokeRpc(DOMRpcIdentifier.create(entry.getType(), iid), input);
+                    return invokeRpc(specificImpls.get(0), DOMRpcIdentifier.create(entry.getType(), iid), input);
                 }
 
                 LOG.debug("No implementation for context {} found will now look for wildcard id", iid);
@@ -78,10 +80,8 @@ final class OperationInvocation {
                 // implementation this way
                 final var mayBeRemoteImpls = entry.getImplementations(YangInstanceIdentifier.of());
                 if (mayBeRemoteImpls != null) {
-                    return mayBeRemoteImpls.get(0)
-                        .invokeRpc(DOMRpcIdentifier.create(entry.getType(), iid), input);
+                    return invokeRpc(mayBeRemoteImpls.get(0), DOMRpcIdentifier.create(entry.getType(), iid), input);
                 }
-
             } else {
                 LOG.warn("Ignoring wrong context value {}", value);
             }
@@ -89,7 +89,7 @@ final class OperationInvocation {
 
         final var impls = entry.getImplementations(null);
         if (impls != null) {
-            return impls.get(0).invokeRpc(entry.getRpcId(), input);
+            return invokeRpc(impls.get(0), entry.getRpcId(), input);
         }
 
         return Futures.immediateFailedFuture(new DOMRpcImplementationNotAvailableException(
@@ -98,6 +98,28 @@ final class OperationInvocation {
 
     private static ListenableFuture<? extends DOMRpcResult> invokeGlobalRpc(
             final GlobalDOMRpcRoutingTableEntry entry, final ContainerNode input) {
-        return entry.getImplementations(YangInstanceIdentifier.of()).get(0).invokeRpc(entry.getRpcId(), input);
+        return invokeRpc(entry.getImplementations(YangInstanceIdentifier.of()).get(0), entry.getRpcId(), input);
+    }
+
+    @SuppressWarnings("checkstyle:illegalCatch")
+    private static ListenableFuture<? extends DOMRpcResult> invokeAction(final DOMActionImplementation impl,
+            final Absolute type, final DOMDataTreeIdentifier path, final ContainerNode input) {
+        try {
+            return impl.invokeAction(type, path, input);
+        } catch (Exception e) {
+            LOG.debug("{} failed on {} with {}", impl, path, input.prettyTree(), e);
+            return Futures.immediateFailedFuture(new DefaultDOMRpcException("Action implementation failed: " + e, e));
+        }
+    }
+
+    @SuppressWarnings("checkstyle:illegalCatch")
+    private static ListenableFuture<? extends DOMRpcResult> invokeRpc(final DOMRpcImplementation impl,
+            final DOMRpcIdentifier rpc, final ContainerNode input) {
+        try {
+            return impl.invokeRpc(rpc, input);
+        } catch (Exception e) {
+            LOG.debug("{} failed on {} with {}", impl, rpc, input.prettyTree(), e);
+            return Futures.immediateFailedFuture(new DefaultDOMRpcException("RPC implementation failed: " + e, e));
+        }
     }
 }
\ No newline at end of file

diff --git a/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/DOMRpcRouterTest.java b/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/DOMRpcRouterTest.java
index 4d859fc141063f28467d34b21827f76cf3295e33..6d350d0fd85650be9504651442e8fa759d98e3f3 100644 (file)
--- a/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/DOMRpcRouterTest.java
+++ b/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/DOMRpcRouterTest.java
@@ -12,6 +12,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doCallRealMethod;
@@ -41,10 +42,12 @@ import org.opendaylight.mdsal.dom.api.DOMActionNotAvailableException;
 import org.opendaylight.mdsal.dom.api.DOMActionService;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier;
 import org.opendaylight.mdsal.dom.api.DOMRpcAvailabilityListener;
+import org.opendaylight.mdsal.dom.api.DOMRpcException;
 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.DOMSchemaService;
+import org.opendaylight.mdsal.dom.api.DefaultDOMRpcException;
 import org.opendaylight.mdsal.dom.spi.DefaultDOMRpcResult;
 import org.opendaylight.yangtools.concepts.Registration;
 import org.opendaylight.yangtools.yang.common.QName;
@@ -114,8 +117,26 @@ public class DOMRpcRouterTest {
     @Test
     public void testFailedInvokeRpc() {
         try (var rpcRouter = rpcsRouter()) {
-            final ListenableFuture<?> future = rpcRouter.rpcService().invokeRpc(Rpcs.FOO, null);
-            final Throwable cause = assertThrows(ExecutionException.class, () -> Futures.getDone(future)).getCause();
+            final var input = ImmutableNodes.newContainerBuilder()
+                .withNodeIdentifier(new NodeIdentifier(Actions.INPUT))
+                .build();
+            final var thrown = new RuntimeException("mumble-mumble");
+
+            try (var reg = rpcRouter.rpcProviderService().registerRpcImplementation(
+                    (rpc, unused) -> {
+                        throw thrown;
+                    }, DOMRpcIdentifier.create(Rpcs.FOO))) {
+
+                final var future = rpcRouter.rpcService().invokeRpc(Rpcs.FOO, input);
+                final var cause = assertThrows(ExecutionException.class, () -> Futures.getDone(future)).getCause();
+                assertThat(cause, instanceOf(DefaultDOMRpcException.class));
+                assertEquals("RPC implementation failed: java.lang.RuntimeException: mumble-mumble",
+                    cause.getMessage());
+                assertSame(thrown, cause.getCause());
+            }
+
+            final var future = rpcRouter.rpcService().invokeRpc(Rpcs.FOO, input);
+            final var cause = assertThrows(ExecutionException.class, () -> Futures.getDone(future)).getCause();
             assertThat(cause, instanceOf(DOMRpcImplementationNotAvailableException.class));
             assertEquals("No implementation of RPC (rpcs)foo available", cause.getMessage());
         }
@@ -231,6 +252,31 @@ public class DOMRpcRouterTest {
         }
     }
 
+    @Test
+    public void testActionInstanceThrowing() throws ExecutionException {
+        try (var rpcRouter = actionsRouter()) {
+            final var actionProvider = rpcRouter.actionProviderService();
+            assertNotNull(actionProvider);
+            final var actionConsumer = rpcRouter.actionService();
+            assertNotNull(actionConsumer);
+
+            final var thrown = new RuntimeException("test-two-three");
+
+            try (var reg = actionProvider.registerActionImplementation(
+                (type, path, input) -> {
+                    throw thrown;
+                }, DOMActionInstance.of(Actions.BAZ_TYPE, LogicalDatastoreType.OPERATIONAL, BAZ_PATH_GOOD))) {
+
+                final var future = invokeBaz(actionConsumer, BAZ_PATH_GOOD);
+                final var ex = assertThrows(ExecutionException.class, () -> Futures.getDone(future)).getCause();
+                assertThat(ex, instanceOf(DOMRpcException.class));
+                assertEquals("Action implementation failed: java.lang.RuntimeException: test-two-three",
+                    ex.getMessage());
+                assertSame(thrown, ex.getCause());
+            }
+        }
+    }
+
     private static DOMRpcRouter actionsRouter() {
         final DOMRpcRouter router = new DOMRpcRouter();
         router.onModelContextUpdated(Actions.CONTEXT);