Merge pull request #33 from drwatson1/improve-error-messaging

Improve error messaging
drwatson1 · Jun 12, 2023 · 81ac424 · 81ac424
2 parents ce03738 + 210ac22
commit 81ac424
Show file tree

Hide file tree

Showing 16 changed files with 152 additions and 137 deletions.
diff --git a/README.md b/README.md
@@ -60,6 +60,7 @@ The tool has almost all the features the DbUp has, but without a single line of
 
 |Date|Version|Description|
 |-|-|-|
+|2023-06-12|1.8.1|Improve error reporting
 |2023-01-18|1.8.0|Add support of .Net 7.0
 |2022-06-11|1.7.0|Add support of CockroachDB, thanks to @lbguilherme
 |2022-05-10|1.6.6|Add support of .Net 6

diff --git a/src/dbup-cli.integration-tests/dbup-cli.integration-tests.csproj b/src/dbup-cli.integration-tests/dbup-cli.integration-tests.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-	  <TargetFrameworks>netcoreapp2.1;netcoreapp3.1;net5.0;net6.0</TargetFrameworks>
+	  <TargetFrameworks>netcoreapp2.1;netcoreapp3.1;net5.0;net6.0;net7.0</TargetFrameworks>
 	  <RootNamespace>DbUp.Cli.IntegrationTests</RootNamespace>
 
     <IsPackable>false</IsPackable>

diff --git a/src/dbup-cli.tests/ConfigLoaderTests.cs b/src/dbup-cli.tests/ConfigLoaderTests.cs
@@ -275,5 +275,27 @@ public void LoadMigration_ShouldSetValidJournalToNull()
                 x.JournalTo.Should().BeNull();
             });
         }
+
+        [TestMethod]
+        public void LoadMigration_ShouldPrintReadableError_WhenProviderIsInvalid()
+        {
+            var migration = ConfigLoader.LoadMigration(GetConfigPath("invalid-provider.yml").Some<string, Error>());
+
+            migration.MatchSome(x =>
+            {
+                Assert.Fail("Invalid provider in the configuration file should not be parsed");
+            });
+        }
+
+        [TestMethod]
+        public void LoadMigration_ShouldPrintReadableError_WhenTransactionIsInvalid()
+        {
+            var migration = ConfigLoader.LoadMigration(GetConfigPath("invalid-transaction.yml").Some<string, Error>());
+
+            migration.MatchSome(x =>
+            {
+                Assert.Fail("Invalid transaction in the configuration file should not be parsed");
+            });
+        }
     }
 }
diff --git a/src/dbup-cli.tests/Scripts/Config/invalid-provider.yml b/src/dbup-cli.tests/Scripts/Config/invalid-provider.yml
@@ -0,0 +1,4 @@
+dbUp:
+  version: 1
+  provider: postgre1
+  connectionString: (localdb)\dbup;Initial Catalog=DbUpTest;Integrated Security=True
diff --git a/src/dbup-cli.tests/Scripts/Config/invalid-transaction.yml b/src/dbup-cli.tests/Scripts/Config/invalid-transaction.yml
@@ -0,0 +1,5 @@
+dbUp:
+  version: 1
+  provider: sqlserver
+  connectionString: (localdb)\dbup;Initial Catalog=DbUpTest;Integrated Security=True
+  transaction: WrongTransaction
diff --git a/src/dbup-cli.tests/dbup-cli.tests.csproj b/src/dbup-cli.tests/dbup-cli.tests.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-	  <TargetFrameworks>netcoreapp2.1;netcoreapp3.1;net5.0;net6.0</TargetFrameworks>
+	  <TargetFrameworks>netcoreapp2.1;netcoreapp3.1;net5.0;net6.0;net7.0</TargetFrameworks>
 	  <RootNamespace>DbUp.Cli.Tests</RootNamespace>
 
     <IsPackable>false</IsPackable>
@@ -42,6 +42,9 @@
     <None Update="Scripts\Config\DotEnv-VarsOverride\CurrentFolder\.env">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
+    <None Update="Scripts\Config\invalid-transaction.yml">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
     <None Update="Scripts\Config\journalTo-null.yml">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
@@ -99,6 +102,9 @@
     <None Update="Scripts\Config\FilterWildcard\c001.sql">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
+    <None Update="Scripts\Config\invalid-provider.yml">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
     <None Update="Scripts\Config\naming.yml">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>

diff --git a/src/dbup-cli/ConfigFile/NamingOptions.cs b/src/dbup-cli/ConfigFile/NamingOptions.cs
@@ -21,6 +21,6 @@ public NamingOptions()
         {
         }
 
-        public static NamingOptions Default => new NamingOptions();
+        public static NamingOptions Default => new();
     }
 }
diff --git a/src/dbup-cli/ConfigLoader.cs b/src/dbup-cli/ConfigLoader.cs
@@ -44,30 +44,25 @@ public static Option<Migration, Error> LoadMigration(Option<string, Error> confi
                     {
                         migration = deserializer.Deserialize<ConfigFile>(input).DbUp;
                     }
-                    catch (SyntaxErrorException ex)
+                    catch (YamlException ex)
                     {
-                        return Option.None<Migration, Error>(Error.Create(Constants.ConsoleMessages.ParsingError, ex.Message));
-                    }
+                        var msg =  (ex.InnerException != null ? ex.InnerException.Message + " " : "") + ex.Message;
+                        return Option.None<Migration, Error>(Error.Create(Constants.ConsoleMessages.ParsingError, msg));
+                    }   
 
                     if( migration.Version != "1" )
                     {
                         return Option.None<Migration, Error>(Error.Create(Constants.ConsoleMessages.NotSupportedConfigFileVersion, "1"));
                     }
 
-                    if (migration.Scripts == null)
-                    {
-                        migration.Scripts = new List<ScriptBatch>();
-                    }
+                    migration.Scripts ??= new List<ScriptBatch>();
 
                     if (migration.Scripts.Count == 0)
                     {
                         migration.Scripts.Add(ScriptBatch.Default);
                     }
 
-                    if (migration.Vars == null)
-                    {
-                        migration.Vars = new Dictionary<string, string>();
-                    }
+                    migration.Vars ??= new Dictionary<string, string>();
 
                     if (!ValidateVarNames(migration.Vars, out var errMessage))
                     {
@@ -86,7 +81,7 @@ static bool ValidateVarNames(Dictionary<string, string> vars, out string errMess
         {
             errMessage = null;
 
-            Regex exp = new Regex("^[a-z0-9_-]+$", RegexOptions.IgnoreCase);
+            Regex exp = new("^[a-z0-9_-]+$", RegexOptions.IgnoreCase);
 
             foreach (var n in vars.Keys)
             {

diff --git a/src/dbup-cli/ConfigurationHelper.cs b/src/dbup-cli/ConfigurationHelper.cs
@@ -25,31 +25,25 @@ public static Option<UpgradeEngineBuilder, Error> SelectDbProvider(Provider prov
         {
             var timeout = TimeSpan.FromSeconds(connectionTimeoutSec);
 
-            switch (provider)
+            return provider switch
             {
-                case Provider.SqlServer:
-                    return DeployChanges.To.SqlDatabase(connectionString)
-                        .WithExecutionTimeout(timeout)
-                        .Some<UpgradeEngineBuilder, Error>();
-                case Provider.AzureSql:
-                    return DeployChanges.To.SqlDatabase(connectionString, null, UseAzureSqlIntegratedSecurity(connectionString))
-                        .WithExecutionTimeout(timeout)
-                        .Some<UpgradeEngineBuilder, Error>();
-                case Provider.PostgreSQL:
-                    return DeployChanges.To.PostgresqlDatabase(connectionString)
-                        .WithExecutionTimeout(timeout)
-                        .Some<UpgradeEngineBuilder, Error>();
-                case Provider.MySQL:
-                    return DeployChanges.To.MySqlDatabase(connectionString)
-                        .WithExecutionTimeout(timeout)
-                        .Some<UpgradeEngineBuilder, Error>();
-                case Provider.CockroachDB:
-                    return DeployChanges.To.CockroachDbDatabase(connectionString)
-                        .WithExecutionTimeout(timeout)
-                        .Some<UpgradeEngineBuilder, Error>();
-            }
-
-            return Option.None<UpgradeEngineBuilder, Error>(Error.Create(Constants.ConsoleMessages.UnsupportedProvider, provider.ToString()));
+                Provider.SqlServer => DeployChanges.To.SqlDatabase(connectionString)
+                                        .WithExecutionTimeout(timeout)
+                                        .Some<UpgradeEngineBuilder, Error>(),
+                Provider.AzureSql => DeployChanges.To.SqlDatabase(connectionString, null, UseAzureSqlIntegratedSecurity(connectionString))
+                                        .WithExecutionTimeout(timeout)
+                                        .Some<UpgradeEngineBuilder, Error>(),
+                Provider.PostgreSQL => DeployChanges.To.PostgresqlDatabase(connectionString)
+                                        .WithExecutionTimeout(timeout)
+                                        .Some<UpgradeEngineBuilder, Error>(),
+                Provider.MySQL => DeployChanges.To.MySqlDatabase(connectionString)
+                                        .WithExecutionTimeout(timeout)
+                                        .Some<UpgradeEngineBuilder, Error>(),
+                Provider.CockroachDB => DeployChanges.To.CockroachDbDatabase(connectionString)
+                                        .WithExecutionTimeout(timeout)
+                                        .Some<UpgradeEngineBuilder, Error>(),
+                _ => Option.None<UpgradeEngineBuilder, Error>(Error.Create(Constants.ConsoleMessages.UnsupportedProvider, provider.ToString())),
+            };
         }
 
         public static Option<bool, Error> EnsureDb(IUpgradeLog logger, Provider provider, string connectionString, int connectionTimeoutSec)
@@ -102,7 +96,7 @@ public static Option<bool, Error> DropDb(IUpgradeLog logger, Provider provider,
                     case Provider.AzureSql:
                         if (UseAzureSqlIntegratedSecurity(connectionString))
                         {
-                            DropDatabase.For.AzureSqlDatabase(connectionString, logger, connectionTimeoutSec);
+                            DropDatabase.For.AzureSqlDatabase(connectionString, logger);
                         }
                         else
                         {

diff --git a/src/dbup-cli/Constants.cs b/src/dbup-cli/Constants.cs
@@ -21,7 +21,7 @@ public static class ConsoleMessages
             public static string UnsupportedProvider => "Unsupported provider: {0}";
             public static string InvalidTransaction => "Unsupported transaction value: {0}";
             public static string ScriptShouldPresent => "At least one script should be present";
-            public static string ParsingError => "Parsing error: {0}";
+            public static string ParsingError => "Configuration file error: {0}";
             public static string InvalidEncoding => "Invalid encoding for scripts' folder '{0}': {1}";
             public static string NotSupportedConfigFileVersion => "The only supported version of a config file is '{0}'";
         }
-Original file line number
+Diff line change
@@ Expand Up / @@ -21,6 +21,6 @@ public NamingOptions() @@
             {
             }
-            public static NamingOptions Default => new NamingOptions();
+            public static NamingOptions Default => new();
         }
     }