fix: CLI should return non-zero error code on failure (#8892)

confluentinc · Mar 16, 2022 · 238f4fd · 238f4fd
1 parent d6ae542
commit 238f4fd
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 10 deletions.
diff --git a/ksqldb-cli/src/main/java/io/confluent/ksql/Ksql.java b/ksqldb-cli/src/main/java/io/confluent/ksql/Ksql.java
@@ -70,14 +70,22 @@ public static void main(final String[] args) throws IOException {
       options.setPassword(readPassword());
     }
 
+    int errorCode = 0;
     try {
-      new Ksql(options, System.getProperties(), KsqlRestClient::create, Cli::build).run();
+      errorCode = new Ksql(
+          options,
+          System.getProperties(),
+          KsqlRestClient::create,
+          Cli::build
+      ).run();
     } catch (final Exception e) {
       final String msg = ErrorMessageUtil.buildErrorMessage(e);
       LOGGER.error(msg);
       System.err.println(msg);
       System.exit(-1);
     }
+
+    System.exit(errorCode);
   }
 
   private static String readPassword() {
@@ -97,7 +105,7 @@ private static String readPassword() {
     return password;
   }
 
-  void run() {
+  int run() {
     final Map<String, String> configProps = options.getConfigFile()
         .map(Ksql::loadProperties)
         .orElseGet(Collections::emptyMap);
@@ -115,16 +123,16 @@ void run() {
         cli.addSessionVariables(sessionVariables);
 
         if (options.getExecute().isPresent()) {
-          cli.runCommand(options.getExecute().get());
+          return cli.runCommand(options.getExecute().get());
         } else if (options.getScriptFile().isPresent()) {
           final File scriptFile = new File(options.getScriptFile().get());
           if (scriptFile.exists() && scriptFile.isFile()) {
-            cli.runScript(scriptFile.getPath());
+            return cli.runScript(scriptFile.getPath());
           } else {
             throw new KsqlException("No such script file: " + scriptFile.getPath());
           }
         } else {
-          cli.runInteractively();
+          return cli.runInteractively();
         }
       }
     }

diff --git a/ksqldb-cli/src/main/java/io/confluent/ksql/cli/Cli.java b/ksqldb-cli/src/main/java/io/confluent/ksql/cli/Cli.java
@@ -101,6 +101,8 @@ public class Cli implements KsqlRequestExecutor, Closeable {
   private static final int MAX_RETRIES = 10;
   private static final String UNKNOWN_VERSION = "<unknown>";
   private static final String NO_WARNING = "";
+  private static final int NO_ERROR = 0;
+  private static final int ERROR = -1;
 
   private static final KsqlParser KSQL_PARSER = new DefaultKsqlParser();
 
@@ -244,7 +246,8 @@ private <R> RestResponse<R> makeKsqlRequest(
   }
 
   // called by '-f' command parameter
-  public void runScript(final String scriptFile) {
+  public int runScript(final String scriptFile) {
+    int errorCode = NO_ERROR;
     RemoteServerSpecificCommand.validateClient(terminal.writer(), restClient);
 
     try {
@@ -262,6 +265,8 @@ public void runScript(final String scriptFile) {
 
       handleLine(content);
     } catch (final Exception exception) {
+      errorCode = ERROR;
+
       LOGGER.error("An error occurred while running a script file. Error = "
           + exception.getMessage(), exception);
 
@@ -270,10 +275,14 @@ public void runScript(final String scriptFile) {
     }
 
     terminal.flush();
+
+    return errorCode;
   }
 
   // called by '-e' command parameter
-  public void runCommand(final String command) {
+  public int runCommand(final String command) {
+    int errorCode = NO_ERROR;
+
     RemoteServerSpecificCommand.validateClient(terminal.writer(), restClient);
     try {
       // Commands executed by the '-e' parameter do not need to execute specific CLI
@@ -282,6 +291,7 @@ public void runCommand(final String command) {
     } catch (final EndOfFileException exception) {
       // Ignore - only used by runInteractively() to exit the CLI
     } catch (final Exception exception) {
+      errorCode = ERROR;
       LOGGER.error("An error occurred while running a command. Error = "
           + exception.getMessage(), exception);
 
@@ -290,9 +300,11 @@ public void runCommand(final String command) {
     }
 
     terminal.flush();
+
+    return errorCode;
   }
 
-  public void runInteractively() {
+  public int runInteractively() {
     displayWelcomeMessage();
     RemoteServerSpecificCommand.validateClient(terminal.writer(), restClient);
     boolean eof = false;
@@ -311,6 +323,8 @@ public void runInteractively() {
       }
       terminal.flush();
     }
+
+    return NO_ERROR;
   }
 
   private void displayWelcomeMessage() {

diff --git a/ksqldb-cli/src/test/java/io/confluent/ksql/cli/CliTest.java b/ksqldb-cli/src/test/java/io/confluent/ksql/cli/CliTest.java
@@ -854,9 +854,10 @@ public void shouldPrintErrorIfCantConnectToRestServerOnRunCommand() throws Excep
     when(mockRestClient.getServerInfo())
         .thenThrow(new KsqlRestClientException("Boom", new IOException("")));
 
-    new Cli(1L, 1L, mockRestClient, console)
+    final int error_code = new Cli(1L, 1L, mockRestClient, console)
         .runCommand("this is a command");
 
+    assertThat(error_code, is(-1));
     assertThat(terminal.getOutputString(),
         containsString("Please ensure that the URL provided is for an active KSQL server."));
   }
@@ -1228,13 +1229,14 @@ public void shouldPrintOnlyFailedStatementFromScriptFile() throws Exception {
         + "partitions=1);\n").getBytes(StandardCharsets.UTF_8));
 
     // When:
-    localCli.runScript(scriptFile.getPath());
+    final int error_code = localCli.runScript(scriptFile.getPath());
 
     // Then:
     final String out = terminal.getOutputString();
     final String expected = "Statement: create stream if not exist s1(id int) "
         + "with (kafka_topic='s1', value_format='json', partitions=1);\n"
         + "Caused by: line 2:22: no viable alternative at input 'create stream if not";
+    assertThat(error_code, is(-1));
     assertThat(out, containsString(expected));
     assertThat(out, not(containsString("drop stream if exists")));
   }