HADOOP-11103. Clean up RemoteException (Contributed by Sean Busbey)

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -588,6 +588,8 @@ Release 2.8.0 - UNRELEASED
 
     HADOOP-1540. Support file exclusion list in distcp. (Rich Haase via jing9)
 
+    HADOOP-11103. Clean up RemoteException (Sean Busbey via vinayakumarb)
+
   OPTIMIZATIONS
 
     HADOOP-11785. Reduce the number of listStatus operation in distcp

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java

@@ -1139,10 +1139,7 @@ public class Client {
           if (erCode == null) {
              LOG.warn("Detailed error code not set by server on rpc error");
           }
-          RemoteException re = 
-              ( (erCode == null) ? 
-                  new RemoteException(exceptionClassName, errorMsg) :
-              new RemoteException(exceptionClassName, errorMsg, erCode));
+          RemoteException re = new RemoteException(exceptionClassName, errorMsg, erCode);
           if (status == RpcStatusProto.ERROR) {
             calls.remove(callId);
             call.setException(re);

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RemoteException.java

@@ -25,31 +25,46 @@ import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos.RpcResponseHeaderProto.Rpc
 import org.xml.sax.Attributes;
 
 public class RemoteException extends IOException {
+  /** this value should not be defined in RpcHeader.proto so that protobuf will return a null */
+  private static final int UNSPECIFIED_ERROR = -1;
   /** For java.io.Serializable */
   private static final long serialVersionUID = 1L;
   private final int errorCode;
 
-  private String className;
+  private final String className;
   
+  /**
+   * @param className wrapped exception, may be null
+   * @param msg may be null
+   */
   public RemoteException(String className, String msg) {
-    super(msg);
-    this.className = className;
-    errorCode = -1;
+    this(className, msg, null);
   }
   
+  /**
+   * @param className wrapped exception, may be null
+   * @param msg may be null
+   * @param erCode may be null
+   */
   public RemoteException(String className, String msg, RpcErrorCodeProto erCode) {
     super(msg);
     this.className = className;
     if (erCode != null)
       errorCode = erCode.getNumber();
     else 
-      errorCode = -1;
+      errorCode = UNSPECIFIED_ERROR;
   }
   
+  /**
+   * @return the class name for the wrapped exception; may be null if none was given.
+   */
   public String getClassName() {
     return className;
   }
   
+  /**
+   * @return may be null if the code was newer than our protobuf definitions or none was given.
+   */
   public RpcErrorCodeProto getErrorCode() {
     return RpcErrorCodeProto.valueOf(errorCode);
   }
@@ -60,7 +75,7 @@ public class RemoteException extends IOException {
    * <p>
    * Unwraps any IOException.
    * 
-   * @param lookupTypes the desired exception class.
+   * @param lookupTypes the desired exception class. may be null.
    * @return IOException, which is either the lookupClass exception or this.
    */
   public IOException unwrapRemoteException(Class<?>... lookupTypes) {
@@ -108,7 +123,10 @@ public class RemoteException extends IOException {
     return ex;
   }
 
-  /** Create RemoteException from attributes */
+  /**
+   * Create RemoteException from attributes
+   * @param attrs may not be null
+   */
   public static RemoteException valueOf(Attributes attrs) {
     return new RemoteException(attrs.getValue("class"),
         attrs.getValue("message")); 

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java

@@ -64,6 +64,7 @@ import org.apache.hadoop.io.retry.RetryPolicies;
 import org.apache.hadoop.io.retry.RetryPolicy;
 import org.apache.hadoop.io.retry.RetryProxy;
 import org.apache.hadoop.ipc.Client.ConnectionId;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos.RpcResponseHeaderProto.RpcErrorCodeProto;
 import org.apache.hadoop.metrics2.MetricsRecordBuilder;
 import org.apache.hadoop.net.NetUtils;
 import org.apache.hadoop.security.AccessControlException;
@@ -589,6 +590,7 @@ public class TestRPC {
       }
     } catch (RemoteException e) {
       if (expectFailure) {
+        assertEquals("RPC error code should be UNAUTHORIZED", RpcErrorCodeProto.FATAL_UNAUTHORIZED, e.getErrorCode());
         assertTrue(e.unwrapRemoteException() instanceof AuthorizationException);
       } else {
         throw e;
@@ -728,6 +730,7 @@ public class TestRPC {
       proxy.echo("");
     } catch (RemoteException e) {
       LOG.info("LOGGING MESSAGE: " + e.getLocalizedMessage());
+      assertEquals("RPC error code should be UNAUTHORIZED", RpcErrorCodeProto.FATAL_UNAUTHORIZED, e.getErrorCode());
       assertTrue(e.unwrapRemoteException() instanceof AccessControlException);
       succeeded = true;
     } finally {
@@ -757,6 +760,7 @@ public class TestRPC {
       proxy.echo("");
     } catch (RemoteException e) {
       LOG.info("LOGGING MESSAGE: " + e.getLocalizedMessage());
+      assertEquals("RPC error code should be UNAUTHORIZED", RpcErrorCodeProto.FATAL_UNAUTHORIZED, e.getErrorCode());
       assertTrue(e.unwrapRemoteException() instanceof AccessControlException);
       succeeded = true;
     } finally {

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestHASafeMode.java

@@ -62,6 +62,7 @@ import org.apache.hadoop.hdfs.server.namenode.NameNodeAdapter;
 import org.apache.hadoop.io.IOUtils;
 import org.apache.hadoop.ipc.RemoteException;
 import org.apache.hadoop.ipc.StandbyException;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos.RpcResponseHeaderProto.RpcErrorCodeProto;
 import org.apache.hadoop.test.GenericTestUtils;
 import org.apache.log4j.Level;
 import org.junit.After;
@@ -774,6 +775,8 @@ public class TestHASafeMode {
       fail("StandBy should throw exception for isInSafeMode");
     } catch (IOException e) {
       if (e instanceof RemoteException) {
+        assertEquals("RPC Error code should indicate app failure.", RpcErrorCodeProto.ERROR_APPLICATION,
+            ((RemoteException) e).getErrorCode());
         IOException sbExcpetion = ((RemoteException) e).unwrapRemoteException();
         assertTrue("StandBy nn should not support isInSafeMode",
             sbExcpetion instanceof StandbyException);