HADOOP-13824. FsShell can suppress the real error if no error message is present. Contributed by John Zhuge.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShell.java

@@ -328,7 +328,12 @@ public int run(String argv[]) throws Exception {
           scope.close();
         }
       } catch (IllegalArgumentException e) {
-        displayError(cmd, e.getLocalizedMessage());
+        if (e.getMessage() == null) {
+          displayError(cmd, "Null exception message");
+          e.printStackTrace(System.err);
+        } else {
+          displayError(cmd, e.getLocalizedMessage());
+        }
         printUsage(System.err);
         if (instance != null) {
           printInstanceUsage(System.err, instance);

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShell.java

@@ -17,18 +17,19 @@
  */
 package org.apache.hadoop.fs;
 
-import java.io.ByteArrayOutputStream;
-import java.io.PrintStream;
-
 import junit.framework.AssertionFailedError;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.io.IOUtils;
+import org.apache.hadoop.fs.shell.Command;
+import org.apache.hadoop.fs.shell.CommandFactory;
+import org.apache.hadoop.test.GenericTestUtils;
 import org.apache.hadoop.tracing.SetSpanReceiver;
 import org.apache.hadoop.util.ToolRunner;
 import org.apache.htrace.core.AlwaysSampler;
 import org.apache.htrace.core.Tracer;
+import org.hamcrest.core.StringContains;
 import org.junit.Assert;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 public class TestFsShell {
 
@@ -73,30 +74,42 @@ public void testTracing() throws Throwable {
 
   @Test
   public void testDFSWithInvalidCommmand() throws Throwable {
-    Configuration conf = new Configuration();
-    FsShell shell = new FsShell(conf);
-    String[] args = new String[1];
-    args[0] = "dfs -mkdirs";
-    final ByteArrayOutputStream bytes = new ByteArrayOutputStream();
-    final PrintStream out = new PrintStream(bytes);
-    final PrintStream oldErr = System.err;
-    try {
-      System.setErr(out);
-      ToolRunner.run(shell, args);
-      String errorValue=new String(bytes.toString());
-      Assert
-      .assertTrue(
-          "FSShell dfs command did not print the error " +
-          "message when invalid command is passed",
-          errorValue.contains("-mkdirs: Unknown command"));
-      Assert
-          .assertTrue(
-              "FSShell dfs command did not print help " +
+    FsShell shell = new FsShell(new Configuration());
+    try (GenericTestUtils.SystemErrCapturer capture =
+             new GenericTestUtils.SystemErrCapturer()) {
+      ToolRunner.run(shell, new String[]{"dfs -mkdirs"});
+      Assert.assertThat("FSShell dfs command did not print the error " +
               "message when invalid command is passed",
-          errorValue.contains("Usage: hadoop fs [generic options]"));
-    } finally {
-      IOUtils.closeStream(out);
-      System.setErr(oldErr);
+          capture.getOutput(), StringContains.containsString(
+              "-mkdirs: Unknown command"));
+      Assert.assertThat("FSShell dfs command did not print help " +
+              "message when invalid command is passed",
+          capture.getOutput(), StringContains.containsString(
+              "Usage: hadoop fs [generic options]"));
+    }
+  }
+
+  @Test
+  public void testExceptionNullMessage() throws Exception {
+    final String cmdName = "-cmdExNullMsg";
+    final Command cmd = Mockito.mock(Command.class);
+    Mockito.when(cmd.run(Mockito.anyVararg())).thenThrow(
+        new IllegalArgumentException());
+    Mockito.when(cmd.getUsage()).thenReturn(cmdName);
+
+    final CommandFactory cmdFactory = Mockito.mock(CommandFactory.class);
+    final String[] names = {cmdName};
+    Mockito.when(cmdFactory.getNames()).thenReturn(names);
+    Mockito.when(cmdFactory.getInstance(cmdName)).thenReturn(cmd);
+
+    FsShell shell = new FsShell(new Configuration());
+    shell.commandFactory = cmdFactory;
+    try (GenericTestUtils.SystemErrCapturer capture =
+             new GenericTestUtils.SystemErrCapturer()) {
+      ToolRunner.run(shell, new String[]{cmdName});
+      Assert.assertThat(capture.getOutput(),
+          StringContains.containsString(cmdName
+              + ": Null exception message"));
     }
   }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/GenericTestUtils.java

@@ -18,10 +18,13 @@
 package org.apache.hadoop.test;
 
 import java.io.BufferedReader;
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.io.PrintStream;
 import java.io.StringWriter;
 import java.lang.management.ManagementFactory;
 import java.lang.management.ThreadInfo;
@@ -272,7 +275,74 @@ public static void waitFor(Supplier<Boolean> check,
         "Thread diagnostics:\n" +
         TimedOutTestsListener.buildThreadDiagnosticString());
   }
-  
+
+  /**
+   * Prints output to one {@link PrintStream} while copying to the other.
+   * <p>
+   * Closing the main {@link PrintStream} will NOT close the other.
+   */
+  public static class TeePrintStream extends PrintStream {
+    private final PrintStream other;
+
+    public TeePrintStream(OutputStream main, PrintStream other) {
+      super(main);
+      this.other = other;
+    }
+
+    @Override
+    public void flush() {
+      super.flush();
+      other.flush();
+    }
+
+    @Override
+    public void write(byte[] buf, int off, int len) {
+      super.write(buf, off, len);
+      other.write(buf, off, len);
+    }
+  }
+
+  /**
+   * Capture output printed to {@link System#err}.
+   * <p>
+   * Usage:
+   * <pre>
+   *   try (SystemErrCapturer capture = new SystemErrCapturer()) {
+   *     ...
+   *     // Call capture.getOutput() to get the output string
+   *   }
+   * </pre>
+   *
+   * TODO: Add lambda support once Java 8 is common.
+   * <pre>
+   *   SystemErrCapturer.withCapture(capture -> {
+   *     ...
+   *   })
+   * </pre>
+   */
+  public static class SystemErrCapturer implements AutoCloseable {
+    final private ByteArrayOutputStream bytes;
+    final private PrintStream bytesPrintStream;
+    final private PrintStream oldErr;
+
+    public SystemErrCapturer() {
+      bytes = new ByteArrayOutputStream();
+      bytesPrintStream = new PrintStream(bytes);
+      oldErr = System.err;
+      System.setErr(new TeePrintStream(oldErr, bytesPrintStream));
+    }
+
+    public String getOutput() {
+      return bytes.toString();
+    }
+
+    @Override
+    public void close() throws Exception {
+      IOUtils.closeQuietly(bytesPrintStream);
+      System.setErr(oldErr);
+    }
+  }
+
   public static class LogCapturer {
     private StringWriter sw = new StringWriter();
     private WriterAppender appender;