Add basic error handling to gRPC Device SB service

Change-Id: Idd19bd4e0d35d8eda06acb4cb631c5d4bcd21980

Add basic error handling to gRPC Device SB service
Change-Id: Idd19bd4e0d35d8eda06acb4cb631c5d4bcd21980
Yuta HIGUCHI · Gerrit Code Review
Commit ad4861ec0cce12499d756db543d50b98dac878a9 ad4861ec 1 parent 54f28e23
Showing 2 changed files with 60 additions and 12 deletions
incubator/rpc-grpc/src/main/java/org/onosproject/incubator/rpc/grpc/DeviceProviderServiceClientProxy.java
incubator/rpc-grpc/src/main/java/org/onosproject/incubator/rpc/grpc/GrpcRemoteServiceServer.java
--- a/incubator/rpc-grpc/src/main/java/org/onosproject/incubator/rpc/grpc/DeviceProviderServiceClientProxy.java
View file @ad4861e
+++ b/incubator/rpc-grpc/src/main/java/org/onosproject/incubator/rpc/grpc/DeviceProviderServiceClientProxy.java
View file @ad4861e
@@ -65,6 +65,8 @@ final class DeviceProviderServiceClientProxy
     private final Channel channel;
+    private Throwable error;
+
     DeviceProviderServiceClientProxy(DeviceProvider provider, Channel channel) {
         super(provider);
         this.channel = channel;
@@ -194,8 +196,26 @@ final class DeviceProviderServiceClientProxy
             log.error("Shutting down session over {}", channel.authority());
             // initiate abnormal termination from client
             devProvService.onError(t);
-            invalidate();
+            invalidate(t);
+        }
+    }
+
+    /**
+     * Invalidates the ProviderService indicating Failure.
+     * @param t {@link Throwable} describing last failure
+     */
+    private void invalidate(Throwable t) {
+        this.error = t;
+        invalidate();
+    }
+
+    @Override
+    public void checkValidity() {
+        if (error != null) {
+            throw new IllegalStateException("DeviceProviderService no longer valid",
+                                            error);
         }
+        super.checkValidity();
     }
     @Override
@@ -272,17 +292,14 @@ final class DeviceProviderServiceClientProxy
         public void onCompleted() {
             log.info("DeviceProviderClientProxy completed");
             // session terminated from remote
-            // TODO unregister...? how?
+            invalidate();
-
-            //devProvService.onCompleted();
         }
         @Override
         public void onError(Throwable t) {
             log.error("DeviceProviderClientProxy#onError", t);
             // session terminated from remote
-            // TODO unregister...? how?
+            invalidate(t);
-            //devProvService.onError(t);
         }
         @Override
--- a/incubator/rpc-grpc/src/main/java/org/onosproject/incubator/rpc/grpc/GrpcRemoteServiceServer.java
View file @ad4861e
+++ b/incubator/rpc-grpc/src/main/java/org/onosproject/incubator/rpc/grpc/GrpcRemoteServiceServer.java
View file @ad4861e
@@ -397,6 +397,16 @@ public class GrpcRemoteServiceServer {
         @Override
         public void triggerProbe(DeviceId deviceId) {
+            try {
+                onTriggerProbe(deviceId);
+            } catch (Exception e) {
+                log.error("Exception caught handling triggerProbe({})",
+                          deviceId, e);
+                toDeviceProvider.onError(e);
+            }
+        }
+
+        private void onTriggerProbe(DeviceId deviceId) {
             log.trace("triggerProbe({})", deviceId);
             DeviceProviderMsg.Builder msgBuilder = DeviceProviderMsg.newBuilder();
             msgBuilder.setTriggerProbe(msgBuilder.getTriggerProbeBuilder()
@@ -404,11 +414,20 @@ public class GrpcRemoteServiceServer {
                                        .build());
             DeviceProviderMsg triggerProbeMsg = msgBuilder.build();
             toDeviceProvider.onNext(triggerProbeMsg);
-            // TODO Catch Exceptions and call onError()
         }
         @Override
         public void roleChanged(DeviceId deviceId, MastershipRole newRole) {
+            try {
+                onRoleChanged(deviceId, newRole);
+            } catch (Exception e) {
+                log.error("Exception caught handling onRoleChanged({}, {})",
+                          deviceId, newRole, e);
+                toDeviceProvider.onError(e);
+            }
+        }
+
+        private void onRoleChanged(DeviceId deviceId, MastershipRole newRole) {
             log.trace("roleChanged({}, {})", deviceId, newRole);
             DeviceProviderMsg.Builder msgBuilder = DeviceProviderMsg.newBuilder();
             msgBuilder.setRoleChanged(msgBuilder.getRoleChangedBuilder()
@@ -416,11 +435,22 @@ public class GrpcRemoteServiceServer {
                                           .setNewRole(translate(newRole))
                                           .build());
             toDeviceProvider.onNext(msgBuilder.build());
-            // TODO Catch Exceptions and call onError()
         }
         @Override
         public boolean isReachable(DeviceId deviceId) {
+            try {
+                return onIsReachable(deviceId);
+            } catch (Exception e) {
+                log.error("Exception caught handling onIsReachable({})",
+                          deviceId, e);
+                toDeviceProvider.onError(e);
+                return false;
+            }
+        }
+
+        private boolean onIsReachable(DeviceId deviceId) {
+
             log.trace("isReachable({})", deviceId);
             CompletableFuture<Boolean> result = new CompletableFuture<>();
             final int xid = xidPool.incrementAndGet();
@@ -450,10 +480,10 @@ public class GrpcRemoteServiceServer {
                 log.warn("isReachable({}) Timed out", deviceId, e);
             } catch (ExecutionException e) {
                 log.error("isReachable({}) Execution failed", deviceId, e);
-                // close session?
+                // close session
+                toDeviceProvider.onError(e);
             }
             return false;
-            // TODO Catch Exceptions and call onError()
         }
         @Override
@@ -464,8 +494,9 @@ public class GrpcRemoteServiceServer {
         @Override
         public void changePortState(DeviceId deviceId, PortNumber portNumber,
                                     boolean enable) {
-            // TODO if required
+            // TODO Implement if required
-
+            log.error("changePortState not supported yet");
+            toDeviceProvider.onError(new UnsupportedOperationException("not implemented yet"));
         }
     }