Improve the error messaging by suppressing stack traces for expected errors.

RELNOTES: None
PiperOrigin-RevId: 177460834

8 files changed, 120 insertions, 50 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java b/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java
index 5739afa377..a6902c0e67 100644
--- a/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java
+++ b/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java
@@ -19,7 +19,6 @@ import static java.util.stream.Collectors.toList;
 import com.android.utils.StdLogger;
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.ListeningExecutorService;
-import com.google.devtools.build.android.AndroidResourceMerger.MergingException;
 import com.google.devtools.build.android.AndroidResourceProcessingAction.Options;
 import com.google.devtools.build.android.aapt2.Aapt2ConfigOptions;
 import com.google.devtools.build.android.aapt2.CompiledResources;
@@ -31,13 +30,11 @@ import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.ShellQuotedParamsFilePreProcessor;
 import com.google.devtools.common.options.TriState;
 import java.io.Closeable;
-import java.io.IOException;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collections;
 import java.util.List;
-import java.util.logging.Logger;
 
 /**
  * Provides an entry point for the resource processing using the AOSP build tools.
@@ -63,9 +60,6 @@ public class Aapt2ResourcePackagingAction {
 
   private static final StdLogger STD_LOGGER = new StdLogger(StdLogger.Level.WARNING);
 
-  private static final Logger logger =
-      Logger.getLogger(Aapt2ResourcePackagingAction.class.getName());
-
   private static Aapt2ConfigOptions aaptConfigOptions;
   private static Options options;
 
@@ -112,18 +106,18 @@ public class Aapt2ResourcePackagingAction {
       // Checks for merge conflicts.
       MergedAndroidData mergedAndroidData =
           AndroidResourceMerger.mergeData(
-              ParsedAndroidData.from(options.primaryData),
-              options.primaryData.getManifest(),
-              options.directData,
-              options.transitiveData,
-              mergedResources,
-              mergedAssets,
-              null /* cruncher. Aapt2 automatically chooses to crunch or not. */,
-              options.packageType,
-              options.symbolsOut,
-              null /* rclassWriter */,
-              dataDeserializer,
-              options.throwOnResourceConflict)
+                  ParsedAndroidData.from(options.primaryData),
+                  options.primaryData.getManifest(),
+                  options.directData,
+                  options.transitiveData,
+                  mergedResources,
+                  mergedAssets,
+                  null /* cruncher. Aapt2 automatically chooses to crunch or not. */,
+                  options.packageType,
+                  options.symbolsOut,
+                  null /* rclassWriter */,
+                  dataDeserializer,
+                  options.throwOnResourceConflict)
               .filter(
                   new DensitySpecificResourceFilter(
                       densitiesToFilter, filteredResources, mergedResources),
@@ -131,7 +125,6 @@ public class Aapt2ResourcePackagingAction {
 
       profiler.recordEndOf("merging");
 
-
       final ListeningExecutorService executorService = ExecutorServiceCloser.createDefaultService();
       try (final Closeable closeable = ExecutorServiceCloser.createWith(executorService)) {
         profiler.startTask("compile");
@@ -209,16 +202,6 @@ public class Aapt2ResourcePackagingAction {
           profiler.recordEndOf("package");
         }
       }
-    } catch (MergingException e) {
-      logger.severe("Merging exception: " + e.getMessage());
-      // throw an error, as system.exit will fail tests.
-      throw new RuntimeException();
-    } catch (IOException e) {
-      logger.severe("File error: " + e.getMessage());
-      // throw an error, as system.exit will fail tests.
-      throw new RuntimeException();
-    } catch (Exception e) {
-      throw e;
     }
   }
 }
diff --git a/src/tools/android/java/com/google/devtools/build/android/AaptCommandBuilder.java b/src/tools/android/java/com/google/devtools/build/android/AaptCommandBuilder.java
index f4b70fc930..4a7e711b0a 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AaptCommandBuilder.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AaptCommandBuilder.java
@@ -302,13 +302,17 @@ public class AaptCommandBuilder {
 
     final Process process = new ProcessBuilder().command(command).redirectErrorStream(true).start();
     processLog.append("Command: ");
-    Joiner.on("\n\t").appendTo(processLog, command);
-    processLog.append("\n");
+    Joiner.on("\\\n\t").appendTo(processLog, command);
+    processLog.append("\nOutput:\n");
     final InputStreamReader stdout =
         new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8);
     while (process.isAlive()) {
       processLog.append(CharStreams.toString(stdout));
     }
+    // Make sure the full stdout is read.
+    while (stdout.ready()) {
+      processLog.append(CharStreams.toString(stdout));
+    }
     if (process.exitValue() != 0) {
       throw new RuntimeException(String.format("Error during %s:", action) + "\n" + processLog);
     }
diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceMerger.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceMerger.java
index bc9bf6d408..2013617964 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceMerger.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceMerger.java
@@ -38,7 +38,7 @@ public class AndroidResourceMerger {
     }
 
     private MergingException(String message) {
-      super(message);
+      super("Merging Error: \n" + message);
     }
 
     static MergingException wrapException(Throwable e) {
diff --git a/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox.java b/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox.java
index f1da34a520..a99da9b931 100644
--- a/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox.java
+++ b/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox.java
@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.android;
 
+import com.google.devtools.build.android.AndroidResourceMerger.MergingException;
+import com.google.devtools.build.android.aapt2.Aapt2Exception;
 import com.google.devtools.common.options.EnumConverter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
@@ -21,7 +23,10 @@ import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.ShellQuotedParamsFilePreProcessor;
+import java.io.IOException;
 import java.nio.file.FileSystems;
+import java.util.Arrays;
+import java.util.logging.Logger;
 
 /**
  * Provides an entry point for the resource processing stages.
@@ -144,6 +149,8 @@ public class ResourceProcessorBusyBox {
     abstract void call(String[] args) throws Exception;
   }
 
+  private static final Logger logger = Logger.getLogger(ResourceProcessorBusyBox.class.getName());
+
   /** Converter for the Tool enum. */
   public static final class ToolConverter extends EnumConverter<Tool> {
 
@@ -177,6 +184,18 @@ public class ResourceProcessorBusyBox {
         new ShellQuotedParamsFilePreProcessor(FileSystems.getDefault()));
     optionsParser.parse(args);
     Options options = optionsParser.getOptions(Options.class);
-    options.tool.call(optionsParser.getResidue().toArray(new String[0]));
+    try {
+      options.tool.call(optionsParser.getResidue().toArray(new String[0]));
+    } catch (MergingException | IOException e) {
+      logger.severe(e.getMessage());
+      logSuppressedAndExit(e);
+    } catch (Aapt2Exception e) {
+      logSuppressedAndExit(e);
+    }
+  }
+
+  private static void logSuppressedAndExit(Throwable e) {
+    Arrays.stream(e.getSuppressed()).map(Throwable::getMessage).forEach(logger::severe);
+    System.exit(1);
   }
 }
diff --git a/src/tools/android/java/com/google/devtools/build/android/ValidateAndLinkResourcesAction.java b/src/tools/android/java/com/google/devtools/build/android/ValidateAndLinkResourcesAction.java
index 31f7741122..5af2c67a59 100644
--- a/src/tools/android/java/com/google/devtools/build/android/ValidateAndLinkResourcesAction.java
+++ b/src/tools/android/java/com/google/devtools/build/android/ValidateAndLinkResourcesAction.java
@@ -51,18 +51,17 @@ public class ValidateAndLinkResourcesAction {
     public Path compiled;
 
     @Option(
-        name = "compiledDep",
-        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-        effectTags = {OptionEffectTag.UNKNOWN},
-        defaultValue = "",
-        converter = Converters.PathListConverter.class,
-        category = "input",
-        allowMultiple = true,
-        help = "Compiled resource dependencies to link."
+      name = "compiledDep",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      defaultValue = "",
+      converter = Converters.PathListConverter.class,
+      category = "input",
+      allowMultiple = true,
+      help = "Compiled resource dependencies to link."
     )
     public List<Path> compiledDeps;
 
-
     @Option(
       name = "manifest",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
@@ -188,9 +187,12 @@ public class ValidateAndLinkResourcesAction {
       ResourceLinker.create(aapt2Options.aapt2, scopedTmp.getPath())
           .profileUsing(profiler)
           .dependencies(Optional.ofNullable(options.deprecatedLibraries).orElse(options.libraries))
-          .include(options.compiledDeps.stream()
-              .map(CompiledResources::from)
-              .collect(Collectors.toList()))
+          .include(
+              options
+                  .compiledDeps
+                  .stream()
+                  .map(CompiledResources::from)
+                  .collect(Collectors.toList()))
           .buildVersion(aapt2Options.buildToolsVersion)
           .linkStatically(resources)
           .copyLibraryTo(options.staticLibraryOut)
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2Exception.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2Exception.java
new file mode 100644
index 0000000000..4781698e8c
--- /dev/null
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/Aapt2Exception.java
@@ -0,0 +1,27 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// Copyright 2017 The Bazel Authors. All rights reserved.
+
+package com.google.devtools.build.android.aapt2;
+
+/** Represents an error found during an aapt2 execution pass. */
+public abstract class Aapt2Exception extends RuntimeException {
+  protected Aapt2Exception(Throwable e) {
+    super(e);
+  }
+
+  public Aapt2Exception() {
+    super();
+  }
+}
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceCompiler.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceCompiler.java
index 5f8b842859..e51192ce1e 100644
--- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceCompiler.java
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceCompiler.java
@@ -17,7 +17,8 @@ package com.google.devtools.build.android.aapt2;
 import com.android.builder.core.VariantType;
 import com.android.repository.Revision;
 import com.google.common.base.Preconditions;
-import com.google.common.util.concurrent.Futures;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableList.Builder;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.devtools.build.android.AaptCommandBuilder;
@@ -29,12 +30,30 @@ import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.logging.Logger;
 
 /** Invokes aapt2 to compile resources. */
 public class ResourceCompiler {
+  static class CompileError extends Aapt2Exception {
+
+    protected CompileError(Throwable e) {
+      super(e);
+    }
+
+    private CompileError() {
+      super();
+    }
+
+    public static CompileError of(List<Throwable> compilationErrors) {
+      final CompileError compileError = new CompileError();
+      compilationErrors.forEach(compileError::addSuppressed);
+      return compileError;
+    }
+  }
+
   private static final Logger logger = Logger.getLogger(ResourceCompiler.class.getName());
 
   private final CompilingVisitor compilingVisitor;
@@ -131,7 +150,19 @@ public class ResourceCompiler {
     }
 
     List<Path> getCompiledArtifacts() throws InterruptedException, ExecutionException {
-      return Futures.allAsList(tasks).get();
+      Builder<Path> builder = ImmutableList.builder();
+      List<Throwable> compilationErrors = new ArrayList<>();
+      for (ListenableFuture<Path> task : tasks) {
+        try {
+          builder.add(task.get());
+        } catch (InterruptedException | ExecutionException e) {
+          compilationErrors.add(Optional.ofNullable(e.getCause()).orElse(e));
+        }
+      }
+      if (compilationErrors.isEmpty()) {
+        return builder.build();
+      }
+      throw CompileError.of(compilationErrors);
     }
   }
 
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
index 2787722c36..99af38766b 100644
--- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
@@ -39,11 +39,15 @@ import java.util.stream.Collectors;
 public class ResourceLinker {
 
   /** Represents errors thrown during linking. */
-  public static class LinkError extends RuntimeException {
+  public static class LinkError extends Aapt2Exception {
 
-    public LinkError(Throwable e) {
+    private LinkError(Throwable e) {
       super(e);
     }
+
+    public static LinkError of(IOException e) {
+      return new LinkError(e);
+    }
   }
 
   private static Logger logger = Logger.getLogger(ResourceLinker.class.getName());
@@ -192,7 +196,7 @@ public class ResourceLinker {
       profiler.recordEndOf("sourcejar");
       return StaticLibrary.from(outPath, rTxt, ImmutableList.of(), sourceJar);
     } catch (IOException e) {
-      throw new LinkError(e);
+      throw LinkError.of(e);
     }
   }