Update DAB CLI to return appropriate exit codes based upon exit reason

src/Cli.Tests/EndToEndTests.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using Azure.DataApiBuilder.Product;
+using Cli.Constants;
 using Microsoft.Data.SqlClient;
 
 namespace Cli.Tests;
@@ -21,9 +22,10 @@ public class EndToEndTests
     public void TestInitialize()
     {
         MockFileSystem fileSystem = FileSystemUtils.ProvisionMockFileSystem();
-        fileSystem.AddFile(
-            TEST_SCHEMA_FILE,
-            new MockFileData(""));
+        // Mock GraphQL Schema File
+        fileSystem.AddFile(TEST_SCHEMA_FILE, new MockFileData(""));
+        // Empty runtime config file
+        fileSystem.AddFile("dab-config-empty.json", new MockFileData(""));
 
         _fileSystem = fileSystem;
 
@@ -691,20 +693,48 @@ public void TestEngineStartUpWithVerboseAndLogLevelOptions(string logLevelOption
     }
 
     /// <summary>
-    /// Test to verify that `--help` and `--version` along with know command/option produce the exit code 0,
-    /// while unknown commands/options have exit code -1.
+    /// Validates that valid usage of verbs and associated options produce exit code 0 (CliReturnCode.SUCCESS).
+    /// Verifies that explicitly implemented verbs (add, update, init, start) and appropriately
+    /// supplied options produce exit code 0.
+    /// Verifies that non-explicitly implemented DAB CLI options `--help` and `--version` produce exit code 0.
+    /// init --config "dab-config.MsSql.json" --database-type mssql --connection-string "InvalidConnectionString"
     /// </summary>
     [DataTestMethod]
-    [DataRow(new string[] { "--version" }, 0, DisplayName = "Checking version should have exit code 0.")]
-    [DataRow(new string[] { "--help" }, 0, DisplayName = "Checking commands with help should have exit code 0.")]
-    [DataRow(new string[] { "add", "--help" }, 0, DisplayName = "Checking options with help should have exit code 0.")]
-    [DataRow(new string[] { "initialize" }, -1, DisplayName = "Invalid Command should have exit code -1.")]
-    [DataRow(new string[] { "init", "--database-name", "mssql" }, -1, DisplayName = "Invalid Options should have exit code -1.")]
-    [DataRow(new string[] { "init", "--database-type", "mssql", "-c", TEST_RUNTIME_CONFIG_FILE }, 0,
-    DisplayName = "Correct command with correct options should have exit code 0.")]
-    public void VerifyExitCodeForCli(string[] cliArguments, int expectedErrorCode)
+    [DataRow(new string[] { "--version" }, DisplayName = "Checking version.")]
+    [DataRow(new string[] { "--help" }, DisplayName = "Valid verbs with help.")]
+    [DataRow(new string[] { "add", "--help" }, DisplayName = "Valid options with help.")]
+    [DataRow(new string[] { "init", "--database-type", "mssql", "-c", TEST_RUNTIME_CONFIG_FILE }, DisplayName = "Valid verb with supported option.")]
+    public void ValidVerbsAndOptionsReturnZero(string[] cliArguments)
     {
-        Assert.AreEqual(expectedErrorCode, Program.Execute(cliArguments, _cliLogger!, _fileSystem!, _runtimeConfigLoader!));
+        Assert.AreEqual(expected: CliReturnCode.SUCCESS, actual: Program.Execute(cliArguments, _cliLogger!, _fileSystem!, _runtimeConfigLoader!));
+    }
+
+    /// <summary>
+    /// Validates that invalid verbs and options produce exit code -1 (CliReturnCode.GENERAL_ERROR).
+    /// </summary>
+    /// <param name="cliArguments">cli verbs, options, and option values</param>
+    [DataTestMethod]
+    [DataRow(new string[] { "--remove-telemetry" }, DisplayName = "Usage of non-existent verb remove-telemetry")]
+    [DataRow(new string[] { "--initialize" }, DisplayName = "Usage of invalid verb (longform of init not supported) initialize")]
+    [DataRow(new string[] { "init", "--database-name", "mssql" }, DisplayName = "Invalid init options database-name")]
+    public void InvalidVerbsAndOptionsReturnNonZeroExitCode(string[] cliArguments)
+    {
+        Assert.AreEqual(expected: CliReturnCode.GENERAL_ERROR, actual: Program.Execute(cliArguments, _cliLogger!, _fileSystem!, _runtimeConfigLoader!));
+    }
+
+    /// <summary>
+    /// Usage of valid verbs and options with values triggering exceptions should produce a non-zero exit code.
+    /// - File read/write issues when reading/writing to the config file.
+    /// - DAB engine failure.
+    /// </summary>
+    /// <param name="cliArguments">cli verbs, options, and option values</param>
+    [DataTestMethod]
+    [DataRow(new string[] { "init", "--config", "dab-config-empty.json", "--database-type", "mssql", "--connection-string", "SampleValue" },
+ DisplayName = "Config file value used already exists on the file system and results in init failure.")]
+    [DataRow(new string[] { "start", "--config", "dab-config-empty.json" }, DisplayName = "Config file value used is empty and engine startup fails")]
+    public void CliAndEngineFailuresReturnNonZeroExitCode(string[] cliArguments)
+    {
+        Assert.AreEqual(expected: CliReturnCode.GENERAL_ERROR, actual: Program.Execute(cliArguments, _cliLogger!, _fileSystem!, _runtimeConfigLoader!));
     }
 
     /// <summary>

src/Cli/Commands/AddOptions.cs

@@ -4,6 +4,7 @@
 using System.IO.Abstractions;
 using Azure.DataApiBuilder.Config;
 using Azure.DataApiBuilder.Product;
+using Cli.Constants;
 using CommandLine;
 using Microsoft.Extensions.Logging;
 using static Cli.Utils;
@@ -56,12 +57,12 @@ public AddOptions(
         [Option("permissions", Required = true, Separator = ':', HelpText = "Permissions required to access the source table or container.")]
         public IEnumerable<string> Permissions { get; }
 
-        public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
+        public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
         {
             logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion());
             if (!IsEntityProvided(Entity, logger, command: "add"))
             {
-                return;
+                return -1;
             }
 
             bool isSuccess = ConfigGenerator.TryAddEntityToConfigWithOptions(this, loader, fileSystem);
@@ -74,6 +75,8 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS
             {
                 logger.LogError("Could not add entity: {Entity} with source: {Source} and permissions: {permissions}.", Entity, Source, string.Join(SEPARATOR, Permissions));
             }
+
+            return isSuccess ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR;
         }
     }
 }

src/Cli/Commands/AddTelemetryOptions.cs

@@ -5,6 +5,7 @@
 using Azure.DataApiBuilder.Config;
 using Azure.DataApiBuilder.Config.ObjectModel;
 using Azure.DataApiBuilder.Product;
+using Cli.Constants;
 using CommandLine;
 using Microsoft.Extensions.Logging;
 using static Cli.Utils;
@@ -32,7 +33,7 @@ public AddTelemetryOptions(string appInsightsConnString, CliBool appInsightsEnab
         [Option("app-insights-enabled", Default = CliBool.True, Required = false, HelpText = "(Default: true) Enable/Disable Application Insights")]
         public CliBool AppInsightsEnabled { get; }
 
-        public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
+        public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
         {
             logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion());
 
@@ -46,6 +47,8 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS
             {
                 logger.LogError("Failed to add telemetry to the configuration file.");
             }
+
+            return isSuccess ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR;
         }
     }
 }

src/Cli/Commands/InitOptions.cs

@@ -5,6 +5,7 @@
 using Azure.DataApiBuilder.Config;
 using Azure.DataApiBuilder.Config.ObjectModel;
 using Azure.DataApiBuilder.Product;
+using Cli.Constants;
 using CommandLine;
 using Microsoft.Extensions.Logging;
 using static Cli.Utils;
@@ -125,18 +126,20 @@ public InitOptions(
         [Option("graphql.multiple-create.enabled", Required = false, HelpText = "(Default: false) Enables multiple create operation for GraphQL. Supported values: true, false.")]
         public CliBool MultipleCreateOperationEnabled { get; }
 
-        public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
+        public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
         {
             logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion());
             bool isSuccess = ConfigGenerator.TryGenerateConfig(this, loader, fileSystem);
             if (isSuccess)
             {
                 logger.LogInformation("Config file generated.");
                 logger.LogInformation("SUGGESTION: Use 'dab add [entity-name] [options]' to add new entities in your config.");
+                return CliReturnCode.SUCCESS;
             }
             else
             {
                 logger.LogError("Could not generate config file.");
+                return CliReturnCode.GENERAL_ERROR;
             }
         }
     }

src/Cli/Commands/StartOptions.cs

@@ -4,6 +4,7 @@
 using System.IO.Abstractions;
 using Azure.DataApiBuilder.Config;
 using Azure.DataApiBuilder.Product;
+using Cli.Constants;
 using CommandLine;
 using Microsoft.Extensions.Logging;
 using static Cli.Utils;
@@ -37,7 +38,7 @@ public StartOptions(bool verbose, LogLevel? logLevel, bool isHttpsRedirectionDis
         [Option("no-https-redirect", Required = false, HelpText = "Disables automatic https redirects.")]
         public bool IsHttpsRedirectionDisabled { get; }
 
-        public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
+        public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
         {
             logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion());
             bool isSuccess = ConfigGenerator.TryStartEngineWithOptions(this, loader, fileSystem);
@@ -46,6 +47,8 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS
             {
                 logger.LogError("Failed to start the engine.");
             }
+
+            return isSuccess ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR;
         }
     }
 }

src/Cli/Commands/UpdateOptions.cs

@@ -4,6 +4,7 @@
 using System.IO.Abstractions;
 using Azure.DataApiBuilder.Config;
 using Azure.DataApiBuilder.Product;
+using Cli.Constants;
 using CommandLine;
 using Microsoft.Extensions.Logging;
 using static Cli.Utils;
@@ -96,12 +97,12 @@ public UpdateOptions(
         [Option('m', "map", Separator = ',', Required = false, HelpText = "Specify mappings between database fields and GraphQL and REST fields. format: --map \"backendName1:exposedName1,backendName2:exposedName2,...\".")]
         public IEnumerable<string>? Map { get; }
 
-        public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
+        public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
         {
             logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion());
             if (!IsEntityProvided(Entity, logger, command: "update"))
             {
-                return;
+                return CliReturnCode.GENERAL_ERROR;
             }
 
             bool isSuccess = ConfigGenerator.TryUpdateEntityWithOptions(this, loader, fileSystem);
@@ -114,6 +115,8 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS
             {
                 logger.LogError("Could not update the entity: {Entity}.", Entity);
             }
+
+            return isSuccess ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR;
         }
     }
 }

src/Cli/Commands/ValidateOptions.cs

@@ -4,6 +4,7 @@
 using System.IO.Abstractions;
 using Azure.DataApiBuilder.Config;
 using Azure.DataApiBuilder.Product;
+using Cli.Constants;
 using CommandLine;
 using Microsoft.Extensions.Logging;
 using static Cli.Utils;
@@ -24,7 +25,7 @@ public ValidateOptions(string config)
         /// This Handler method is responsible for validating the config file and is called when `validate`
         /// command is invoked.
         /// </summary>
-        public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
+        public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
         {
             logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion());
             bool isValidConfig = ConfigGenerator.IsConfigValid(this, loader, fileSystem);
@@ -37,6 +38,8 @@ public void Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileS
             {
                 logger.LogError("Config is invalid. Check above logs for details.");
             }
+
+            return isValidConfig ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR;
         }
     }
 }

src/Cli/Constants/CliReturnCode.cs

@@ -0,0 +1,14 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System.Runtime.CompilerServices;
+
+[assembly: InternalsVisibleTo("Cli.Tests")]
+namespace Cli.Constants
+{
+    internal class CliReturnCode
+    {
+        public const int SUCCESS = 0;
+        public const int GENERAL_ERROR = -1;
+    }
+}

src/Cli/DabCliParserErrorHandler.cs

@@ -0,0 +1,47 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using CommandLine;
+
+namespace Cli
+{
+    /// <summary>
+    /// Processes errors that occur during parsing of CLI verbs (start, init, export, add, update, etc) and their arguments.
+    /// </summary>
+    public class DabCliParserErrorHandler
+    {
+        /// <summary>
+        /// Processes errors accumulated by each parser in parser.ParseArguments<parsers>().
+        /// For DAB CLI, this only includes scenarios where the user provides invalid DAB CLI input.
+        /// e.g. incorrectly formed or missing options and parameters.
+        /// Additionally, an error is tracked if the user uses:
+        /// -> an unsupported CLI verb
+        /// -> --help.
+        /// -> --version
+        /// </summary>
+        /// <param name="err">Collection of Error objects collected by the CLI parser.</param>
+        /// <returns>Return code: 0 when --help is used, otherwise -1.</returns>
+        public static int ProcessErrorsAndReturnExitCode(IEnumerable<Error> err)
+        {
+            // To know if `--help` or `--version` was requested.
+            bool isHelpOrVersionRequested = false;
+
+            /// System.CommandLine considers --help and --version as NonParsed Errors
+            /// ref: https://github.com/commandlineparser/commandline/issues/630
+            /// This is a workaround to make sure our app exits with exit code 0,
+            /// when user does --help or --versions.
+            /// dab --help -> ErrorType.HelpVerbRequestedError
+            /// dab [command-name] --help -> ErrorType.HelpRequestedError
+            /// dab --version -> ErrorType.VersionRequestedError
+            List<Error> errors = err.ToList();
+            if (errors.Any(e => e.Tag == ErrorType.VersionRequestedError
+                                || e.Tag == ErrorType.HelpRequestedError
+                                || e.Tag == ErrorType.HelpVerbRequestedError))
+            {
+                isHelpOrVersionRequested = true;
+            }
+
+            return isHelpOrVersionRequested ? 0 : -1;
+        }
+    }
+}

src/Cli/Exporter.cs

@@ -13,7 +13,7 @@ namespace Cli
 {
     internal static class Exporter
     {
-        public static void Export(ExportOptions options, ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
+        public static int Export(ExportOptions options, ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
         {
             StartOptions startOptions = new(false, LogLevel.None, false, options.Config!);
 
@@ -23,7 +23,7 @@ public static void Export(ExportOptions options, ILogger logger, FileSystemRunti
             if (!TryGetConfigFileBasedOnCliPrecedence(loader, options.Config, out string runtimeConfigFile))
             {
                 logger.LogError("Failed to find the config file provided, check your options and try again.");
-                return;
+                return -1;
             }
 
             if (!loader.TryLoadConfig(
@@ -32,14 +32,15 @@ public static void Export(ExportOptions options, ILogger logger, FileSystemRunti
                     replaceEnvVar: true) || runtimeConfig is null)
             {
                 logger.LogError("Failed to read the config file: {runtimeConfigFile}.", runtimeConfigFile);
-                return;
+                return -1;
             }
 
             Task server = Task.Run(() =>
             {
                 _ = ConfigGenerator.TryStartEngineWithOptions(startOptions, loader, fileSystem);
             }, cancellationToken);
 
+            bool isSuccess = false;
             if (options.GraphQL)
             {
                 int retryCount = 5;
@@ -50,6 +51,7 @@ public static void Export(ExportOptions options, ILogger logger, FileSystemRunti
                     try
                     {
                         ExportGraphQL(options, runtimeConfig, fileSystem);
+                        isSuccess = true;
                         break;
                     }
                     catch
@@ -65,6 +67,7 @@ public static void Export(ExportOptions options, ILogger logger, FileSystemRunti
             }
 
             cancellationTokenSource.Cancel();
+            return isSuccess ? 0 : -1;
         }
 
         private static void ExportGraphQL(ExportOptions options, RuntimeConfig runtimeConfig, System.IO.Abstractions.IFileSystem fileSystem)