Improve error handling in sdp (dotnet#2466)

* Improve error handling in sdp * fix ut
turrisxyz · Feb 28, 2018 · b3c09ca · b3c09ca
1 parent 8e884f7
commit b3c09ca
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 37 deletions.
diff --git a/src/Microsoft.DocAsCode.Build.Engine/HostServiceCreator.cs b/src/Microsoft.DocAsCode.Build.Engine/HostServiceCreator.cs
@@ -4,6 +4,7 @@
 namespace Microsoft.DocAsCode.Build.Engine
 {
     using System;
+    using System.Collections.Concurrent;
     using System.Collections.Generic;
     using System.Collections.Immutable;
     using System.IO;
@@ -41,12 +42,7 @@ public virtual HostService CreateHostService(
         {
             var hostService = new HostService(
                 parameters.Files.DefaultBaseDir,
-                files == null
-                    ? Enumerable.Empty<FileModel>()
-                    : from file in files
-                      select Load(processor, parameters.Metadata, parameters.FileMetadata, file) into model
-                      where model != null
-                      select model,
+                LoadModels(files, parameters, processor),
                 parameters.VersionName,
                 parameters.VersionDir,
                 parameters.LruSize,
@@ -74,14 +70,34 @@ public virtual FileModel Load(IDocumentProcessor processor, ImmutableDictionary<
                 {
                     return processor.Load(file, metadata);
                 }
-                catch (Exception)
+                catch (Exception e)
                 {
-                    Logger.LogError($"Unable to load file: {file.File} via processor: {processor.Name}.");
+                    Logger.LogError($"Unable to load file: {file.File} via processor: {processor.Name}: {e.Message}");
                     throw;
                 }
             }
         }
 
+        private IEnumerable<FileModel> LoadModels(IEnumerable<FileAndType> files, DocumentBuildParameters parameters, IDocumentProcessor processor)
+        {
+            var models = new ConcurrentBag<FileModel>();
+            if (files == null)
+            {
+                return models;
+            }
+
+            files.RunAll(file =>
+            {
+                var model = Load(processor, parameters.Metadata, parameters.FileMetadata, file);
+                if (model != null)
+                {
+                    models.Add(model);
+                }
+            }, _context.MaxParallelism);
+
+            return models;
+        }
+
         private static ImmutableDictionary<string, object> ApplyFileMetadata(
             string file,
             ImmutableDictionary<string, object> metadata,

diff --git a/src/Microsoft.DocAsCode.Build.SchemaDriven/ApplyOverwriteDocument.cs b/src/Microsoft.DocAsCode.Build.SchemaDriven/ApplyOverwriteDocument.cs
@@ -3,6 +3,7 @@
 
 namespace Microsoft.DocAsCode.Build.SchemaDriven
 {
+    using System;
     using System.Collections.Generic;
     using System.Collections.Immutable;
     using System.Composition;
@@ -23,31 +24,36 @@ public class ApplyOverwriteDocument : BaseDocumentBuildStep, ISupportIncremental
         public override void Postbuild(ImmutableList<FileModel> models, IHostService host)
         {
             var overwriteApplier = new OverwriteApplier(host, OverwriteModelType.Classic);
-            foreach (var uid in host.GetAllUids())
+            host.GetAllUids().RunAll(uid => ApplyOverwriteToModel(overwriteApplier, uid, host));
+        }
+
+        private void ApplyOverwriteToModel(OverwriteApplier overwriteApplier, string uid, IHostService host)
+        {
+            var ms = host.LookupByUid(uid);
+            var ods = ms.Where(m => m.Type == DocumentType.Overwrite).ToList();
+            var articles = ms.Except(ods).ToList();
+            if (articles.Count == 0 || ods.Count == 0)
             {
-                var ms = host.LookupByUid(uid);
-                var ods = ms.Where(m => m.Type == DocumentType.Overwrite).ToList();
-                var articles = ms.Except(ods).ToList();
-                if (articles.Count == 0 || ods.Count == 0)
-                {
-                    continue;
-                }
+                return;
+            }
+
+            if (articles.Count > 1)
+            {
+                throw new DocumentException($"{uid} is defined in multiple articles {articles.Select(s => s.LocalPathFromRoot).ToDelimitedString()}");
+            }
 
-                if (articles.Count > 1)
+            var model = articles[0];
+            var schema = model.Properties.Schema as DocumentSchema;
+            using (new LoggerFileScope(model.LocalPathFromRoot))
+            {
+                var uidDefiniton = model.Uids.Where(s => s.Name == uid).ToList();
+                if (uidDefiniton.Count == 0)
                 {
-                    throw new DocumentException($"{uid} is defined in multiple articles {articles.Select(s => s.LocalPathFromRoot).ToDelimitedString()}");
+                    throw new DocfxException($"Unable to find UidDefinition for Uid {uid}");
                 }
 
-                var model = articles[0];
-                var schema = model.Properties.Schema as DocumentSchema;
-                using (new LoggerFileScope(model.LocalPathFromRoot))
+                try
                 {
-                    var uidDefiniton = model.Uids.Where(s => s.Name == uid).ToList();
-                    if (uidDefiniton.Count == 0)
-                    {
-                        throw new DocfxException($"Unable to find UidDefinition for Uid {uid}");
-                    }
-
                     foreach (var ud in uidDefiniton)
                     {
                         var jsonPointer = new JsonPointer(ud.Path).GetParentPointer();
@@ -74,10 +80,18 @@ public override void Postbuild(ImmutableList<FileModel> models, IHostService hos
                     }
 
                     // 1. Validate schema after the merge
+                    // TODO: Issue exists - however unable to identify which overwrite document the issue is from
                     ((SchemaDrivenDocumentProcessor)host.Processor).SchemaValidator.Validate(model.Content);
 
                     // 2. Re-export xrefspec after the merge
                     overwriteApplier.UpdateXrefSpec(model, schema);
+
+                }
+                catch (DocumentException e)
+                {
+                    // Log error here to preserve file info
+                    Logger.LogError(e.Message);
+                    throw;
                 }
             }
         }

diff --git a/src/Microsoft.DocAsCode.Build.SchemaDriven/ApplyOverwriteFragments.cs b/src/Microsoft.DocAsCode.Build.SchemaDriven/ApplyOverwriteFragments.cs
@@ -71,6 +71,11 @@ public override void Build(FileModel model, IHostService host)
                         code: WarningCodes.Overwrite.InvalidMarkdownFragments);
                     return;
                 }
+                catch (DocumentException de)
+                {
+                    Logger.LogError(de.Message);
+                    throw;
+                }
             }
         }
 

diff --git a/src/Microsoft.DocAsCode.Build.SchemaDriven/Exceptions/InvalidJsonPointerException.cs b/src/Microsoft.DocAsCode.Build.SchemaDriven/Exceptions/InvalidJsonPointerException.cs
@@ -6,8 +6,10 @@ namespace Microsoft.DocAsCode.Exceptions
     using System;
     using System.Runtime.Serialization;
 
+    using Microsoft.DocAsCode.Plugins;
+
     [Serializable]
-    public class InvalidJsonPointerException : DocfxException
+    public class InvalidJsonPointerException : DocumentException
     {
         public InvalidJsonPointerException() : this("The value of json pointer is not valid")
         {

diff --git a/src/Microsoft.DocAsCode.Build.SchemaDriven/Exceptions/InvalidSchemaException.cs b/src/Microsoft.DocAsCode.Build.SchemaDriven/Exceptions/InvalidSchemaException.cs
@@ -6,8 +6,10 @@ namespace Microsoft.DocAsCode.Exceptions
     using System;
     using System.Runtime.Serialization;
 
+    using Microsoft.DocAsCode.Plugins;
+
     [Serializable]
-    public class InvalidSchemaException : DocfxException
+    public class InvalidSchemaException : DocumentException
     {
         public InvalidSchemaException() : this("Document schema is not valid")
         {

diff --git a/src/Microsoft.DocAsCode.Build.SchemaDriven/Models/DocumentSchema.cs b/src/Microsoft.DocAsCode.Build.SchemaDriven/Models/DocumentSchema.cs
@@ -39,8 +39,17 @@ public static DocumentSchema Load(TextReader reader, string title)
             DocumentSchema schema;
             using (var jtr = new JsonTextReader(reader))
             {
-                var jObject = JObject.Load(jtr);
-                var jSchema = JSchema.Load(jObject.CreateReader());
+                JSchema jSchema;
+                JObject jObject;
+                try
+                {
+                    jObject = JObject.Load(jtr);
+                    jSchema = JSchema.Load(jObject.CreateReader());
+                }
+                catch (Exception e) when (e is JSchemaException || e is JsonException)
+                {
+                    throw new InvalidSchemaException($"{title} is not a valid schema: {e.Message}", e);
+                }
 
                 var validator = new SchemaValidator(jObject, jSchema);
 

diff --git a/src/Microsoft.DocAsCode.Build.SchemaDriven/Validators/SchemaValidateService.cs b/src/Microsoft.DocAsCode.Build.SchemaDriven/Validators/SchemaValidateService.cs
@@ -72,7 +72,7 @@ public void Validate(object obj, JSchema schema)
 
             if (errors.Count > 0)
             {
-                throw new InvalidSchemaException($"Validation against {schema.SchemaVersion.OriginalString} failed: \n{errors.ToDelimitedString("\n")}");
+                throw new InvalidSchemaException($"Validation against \"{schema.SchemaVersion.OriginalString}\" failed: \n{errors.ToDelimitedString("\n")}");
             }
         }
 

diff --git a/src/Microsoft.DocAsCode.Plugins/DocumentExceptionExtensions.cs b/src/Microsoft.DocAsCode.Plugins/DocumentExceptionExtensions.cs
@@ -40,7 +40,7 @@ public static TResult[] RunAll<TElement, TResult>(this IReadOnlyList<TElement> e
             return results;
         }
 
-        public static void RunAll<TElement>(this IReadOnlyList<TElement> elements, Action<TElement> action)
+        public static void RunAll<TElement>(this IEnumerable<TElement> elements, Action<TElement> action)
         {
             if (elements == null)
             {
@@ -51,11 +51,11 @@ public static void RunAll<TElement>(this IReadOnlyList<TElement> elements, Actio
                 throw new ArgumentNullException(nameof(action));
             }
             DocumentException firstException = null;
-            for (int i = 0; i < elements.Count; i++)
+            foreach (var element in elements)
             {
                 try
                 {
-                    action(elements[i]);
+                    action(element);
                 }
                 catch (DocumentException ex)
                 {
@@ -71,7 +71,7 @@ public static void RunAll<TElement>(this IReadOnlyList<TElement> elements, Actio
             }
         }
 
-        public static void RunAll<TElement>(this IReadOnlyList<TElement> elements, Action<TElement> action, int parallelism)
+        public static void RunAll<TElement>(this IEnumerable<TElement> elements, Action<TElement> action, int parallelism)
         {
             if (elements == null)
             {

diff --git a/test/Microsoft.DocAsCode.Build.SchemaDriven.Tests/SchemaDrivenProcessorTest.cs b/test/Microsoft.DocAsCode.Build.SchemaDriven.Tests/SchemaDrivenProcessorTest.cs
@@ -433,7 +433,10 @@ public void TestInvalidObjectAgainstSchema()
 
                 FileCollection files = new FileCollection(_defaultFiles);
                 files.Add(DocumentType.Article, new[] { inputFile }, _inputFolder);
-                Assert.Throws<InvalidSchemaException>(() => BuildDocument(files));
+                Assert.Throws<DocumentException>(() => BuildDocument(files));
+                var errors = listener.Items.Where(s => s.LogLevel == LogLevel.Error).ToList();
+                Assert.Equal(1, errors.Count);
+                Assert.Equal($"Unable to load file: {inputFile} via processor: MetadataReferenceTest: Validation against \"http://dotnet.github.io/docfx/schemas/v1.0/schema.json#\" failed: \nInvalid type. Expected Object but got String. Path 'metadata'.", errors[0].Message);
             }
         }