Merge pull request #738 from SMI/fix/console-exception-logging

Fix/console exception logging
SMI · May 6, 2021 · 282132b · 282132b
2 parents 0e084c8 + 0ce731a
commit 282132b
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 29 deletions.
diff --git a/news/738-feature.md b/news/738-feature.md
@@ -0,0 +1,3 @@
+Improvements to CLI user experience
+-   Immediately verify that the config file (GlobalOptions) we've loaded is somewhat valid
+-   Improve visibility of exception messages on CLI exit
diff --git a/src/applications/Applications.SmiRunner/Program.cs b/src/applications/Applications.SmiRunner/Program.cs
@@ -39,35 +39,47 @@ internal static int Main(string[] args)
             allTypes.AddRange(AllApplications);
             allTypes.AddRange(AllServices);
 
-            int res = SmiCliInit.ParseServiceVerbAndRun(
-                args.Take(1),
-                allTypes.ToArray(),
-                service =>
-                {
-                    // TODO(rkm 2021-02-26) Probably want to test that every case is covered here
-                    return service switch
+            int res;
+
+            try
+            {
+                res = SmiCliInit.ParseServiceVerbAndRun(
+                    args.Take(1),
+                    allTypes.ToArray(),
+                    service =>
                     {
-                        // Applications
-                        TriggerUpdatesVerb _ => Applications.TriggerUpdates.Program.Main(rest),
-                        DicomDirectoryProcessorVerb _ => Applications.DicomDirectoryProcessor.Program.Main(rest),
-                        IsIdentifiableReviewerVerb _ => Applications.IsIdentifiableReviewer.Program.Main(rest),
-                        ExtractImagesVerb _ => ExtractImages.Program.Main(rest),
-                        // Microservices
-                        CohortExtractorVerb _ => Microservices.CohortExtractor.Program.Main(rest),
-                        CohortPackagerVerb _ => Microservices.CohortPackager.Program.Main(rest),
-                        DeadLetterReprocessorVerb _ => Microservices.DeadLetterReprocessor.Program.Main(rest),
-                        DicomRelationalMapperVerb _ => Microservices.DicomRelationalMapper.Program.Main(rest),
-                        DicomReprocessorVerb _ => Microservices.DicomReprocessor.Program.Main(rest),
-                        DicomTagReaderVerb _ => Microservices.DicomTagReader.Program.Main(rest),
-                        FileCopierVerb _ => Microservices.FileCopier.Program.Main(rest),
-                        IdentifierMapperVerb _ => Microservices.IdentifierMapper.Program.Main(rest),
-                        IsIdentifiableVerb _ => Microservices.IsIdentifiable.Program.Main(rest),
-                        MongoDbPopulatorVerb _ => Microservices.MongoDBPopulator.Program.Main(rest),
-                        UpdateValuesVerb _ => Microservices.UpdateValues.Program.Main(rest),
-                        _ => throw new ArgumentException($"No case for {nameof(service)}")
-                    };
-                }
-            );
+                        // TODO(rkm 2021-02-26) Probably want to test that every case is covered here
+                        return service switch
+                        {
+                            // Applications
+                            TriggerUpdatesVerb _ => Applications.TriggerUpdates.Program.Main(rest),
+                            DicomDirectoryProcessorVerb _ => Applications.DicomDirectoryProcessor.Program.Main(rest),
+                            IsIdentifiableReviewerVerb _ => Applications.IsIdentifiableReviewer.Program.Main(rest),
+                            ExtractImagesVerb _ => ExtractImages.Program.Main(rest),
+                            // Microservices
+                            CohortExtractorVerb _ => Microservices.CohortExtractor.Program.Main(rest),
+                            CohortPackagerVerb _ => Microservices.CohortPackager.Program.Main(rest),
+                            DeadLetterReprocessorVerb _ => Microservices.DeadLetterReprocessor.Program.Main(rest),
+                            DicomRelationalMapperVerb _ => Microservices.DicomRelationalMapper.Program.Main(rest),
+                            DicomReprocessorVerb _ => Microservices.DicomReprocessor.Program.Main(rest),
+                            DicomTagReaderVerb _ => Microservices.DicomTagReader.Program.Main(rest),
+                            FileCopierVerb _ => Microservices.FileCopier.Program.Main(rest),
+                            IdentifierMapperVerb _ => Microservices.IdentifierMapper.Program.Main(rest),
+                            IsIdentifiableVerb _ => Microservices.IsIdentifiable.Program.Main(rest),
+                            MongoDbPopulatorVerb _ => Microservices.MongoDBPopulator.Program.Main(rest),
+                            UpdateValuesVerb _ => Microservices.UpdateValues.Program.Main(rest),
+                            _ => throw new ArgumentException($"No case for {nameof(service)}")
+                        };
+                    }
+                );
+            }
+            catch (Exception e)
+            {
+                Console.Error.WriteLine(e);
+                const int rc = 1;
+                Console.Error.WriteLine($"\nError (exit code {rc}): {e.Message}");
+                return rc;
+            }
 
             if(args.Any(a=>a.Equals("--help")))
             {

diff --git a/src/common/Smi.Common/Options/GlobalOptionsFactory.cs b/src/common/Smi.Common/Options/GlobalOptionsFactory.cs
@@ -45,6 +45,9 @@ public GlobalOptions Load(string hostProcessName, string configFilePath = "defau
             using var sr = new StringReader(yamlContents);
             var globals = deserializer.Deserialize<GlobalOptions>(sr);
 
+            if (globals.LoggingOptions == null)
+                throw new Exception($"Loaded YAML did not contain a {nameof(globals.LoggingOptions)} key. Did you provide a valid config file?");
+
             globals.HostProcessName = hostProcessName;
 
             return Decorate(globals);

diff --git a/tests/microservices/Microservices.CohortExtractor.Tests/ModalitySpecificRejectorsTests.cs b/tests/microservices/Microservices.CohortExtractor.Tests/ModalitySpecificRejectorsTests.cs
@@ -17,6 +17,8 @@ public void TestDeserialization()
             string file;
             var yaml =
             @"
+LoggingOptions:
+    LogConfigFile:
 CohortExtractorOptions:
     QueueName: 'TEST.RequestQueue'
     AllCatalogues: true
@@ -46,6 +48,8 @@ public void TestValidation_MissingModalityRouting()
             string file;
             var yaml =
             @"
+LoggingOptions:
+    LogConfigFile:
 CohortExtractorOptions:
     QueueName: 'TEST.RequestQueue'
     AllCatalogues: true

diff --git a/...croservices.CohortPackager.Tests/Execution/JobProcessing/Reporting/JobReporterBaseTest.cs b/...croservices.CohortPackager.Tests/Execution/JobProcessing/Reporting/JobReporterBaseTest.cs
@@ -763,7 +763,8 @@ public void Constructor_NoNewLine_SetToEnvironment()
         public void ReportNewLine_LoadFromYaml_EscapesNewlines()
         {
             string yaml = @"
-CurrentDirectory:
+LoggingOptions:
+    LogConfigFile:
 CohortPackagerOptions:
     ReportNewLine: '\r\n'
 ";