Use correct exit code on invalid aquery --output

illicitonion · copybara-github · commit 4158b61211e0 · 2021-07-21T01:19:50.000-07:00
Currently this exits 0, which is dangerous - scripts may assume a query succeeded with no results, where actually it never ran. Instead, properly exit with exit code 2. Closes #13660. PiperOrigin-RevId: 385959603
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/PostAnalysisQueryBuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/PostAnalysisQueryBuildTool.java
@@ -28,13 +28,15 @@
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.runtime.QueryRuntimeHelper;
 import com.google.devtools.build.lib.runtime.QueryRuntimeHelper.QueryRuntimeHelperException;
+import com.google.devtools.build.lib.server.FailureDetails.ActionQuery;
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
 import com.google.devtools.build.lib.server.FailureDetails.Query;
-import com.google.devtools.build.lib.server.FailureDetails.Query.Code;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutorWrappingWalkableGraph;
 import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.WalkableGraph;
+import com.google.devtools.common.options.OptionsParsingException;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Set;
@@ -68,7 +70,7 @@ protected void postProcessAnalysisResult(BuildRequest request, AnalysisResult an
                     .setMessage(
                         "Queries based on analysis results are not allowed if incrementality state"
                             + " is not being kept")
-                    .setQuery(Query.newBuilder().setCode(Code.ANALYSIS_QUERY_PREREQ_UNMET))
+                    .setQuery(Query.newBuilder().setCode(Query.Code.ANALYSIS_QUERY_PREREQ_UNMET))
                     .build()));
       }
       try (QueryRuntimeHelper queryRuntimeHelper =
@@ -92,13 +94,22 @@ protected void postProcessAnalysisResult(BuildRequest request, AnalysisResult an
           FailureDetail failureDetail =
               FailureDetail.newBuilder()
                   .setMessage(errorMessage + ": " + e.getMessage())
-                  .setQuery(Query.newBuilder().setCode(Code.OUTPUT_FORMATTER_IO_EXCEPTION))
+                  .setQuery(Query.newBuilder().setCode(Query.Code.OUTPUT_FORMATTER_IO_EXCEPTION))
                   .build();
           throw new ViewCreationFailedException(errorMessage, failureDetail, e);
         }
         env.getReporter().error(null, errorMessage, e);
       } catch (QueryRuntimeHelperException e) {
         throw new ExitException(DetailedExitCode.of(e.getFailureDetail()));
+      } catch (OptionsParsingException e) {
+        throw new ExitException(
+            DetailedExitCode.of(
+                ExitCode.COMMAND_LINE_ERROR,
+                FailureDetail.newBuilder()
+                    .setMessage(e.getMessage())
+                    .setActionQuery(
+                        ActionQuery.newBuilder().setCode(ActionQuery.Code.INCORRECT_ARGUMENTS))
+                    .build()));
       }
     }
   }
@@ -118,7 +129,8 @@ private void doPostAnalysisQuery(
       Collection<SkyKey> transitiveConfigurationKeys,
       QueryRuntimeHelper queryRuntimeHelper,
       QueryExpression queryExpression)
-      throws InterruptedException, QueryException, IOException, QueryRuntimeHelperException {
+      throws InterruptedException, QueryException, IOException, QueryRuntimeHelperException,
+          OptionsParsingException {
     WalkableGraph walkableGraph =
         SkyframeExecutorWrappingWalkableGraph.of(env.getSkyframeExecutor());
 
@@ -143,14 +155,10 @@ private void doPostAnalysisQuery(
     NamedThreadSafeOutputFormatterCallback<T> callback =
         NamedThreadSafeOutputFormatterCallback.selectCallback(outputFormat, callbacks);
     if (callback == null) {
-      env.getReporter()
-          .handle(
-              Event.error(
-                  String.format(
-                      "Invalid output format '%s'. Valid values are: %s",
-                      outputFormat,
-                      NamedThreadSafeOutputFormatterCallback.callbackNames(callbacks))));
-      return;
+      throw new OptionsParsingException(
+          String.format(
+              "Invalid output format '%s'. Valid values are: %s",
+              outputFormat, NamedThreadSafeOutputFormatterCallback.callbackNames(callbacks)));
     }
 
     // A certain subset of output formatters support "streaming" results - the formatter is called
