From 9b98d13c064a8a7cfd8374101b3366bd9f3bf75a Mon Sep 17 00:00:00 2001 From: Felipe Cotti Date: Thu, 4 Jun 2026 21:22:37 -0300 Subject: [PATCH] Fix crash when toc references a missing file A `toc` entry pointing at a Markdown file that doesn't exist on disk caused the navigation builder to leave the root node's `Index` as a `null!` sentinel. `BuildNavigationLookups` (and `VisitNavigation` with extensions enabled) then dereferenced that sentinel via the covariant `INodeNavigationItem` branch, throwing an unhandled `NullReferenceException` before the already-computed validation error could surface. Guard the null `Index` sentinel at both dereference sites so the build exits cleanly and reports the error. Also clarify the diagnostic to state whether the referenced file is missing on disk or not a valid Markdown document, anchored to the docset.yml location. Closes #3245 Co-authored-by: Cursor --- .../Node/DocumentationSetNavigation.cs | 12 ++++--- .../NavigationItemExtensions.cs | 9 +++-- src/Elastic.Markdown/IO/DocumentationSet.cs | 9 +++-- .../Isolation/ValidationTests.cs | 33 +++++++++++++++++++ .../TestDocumentationSetContext.cs | 20 +++++++++++ 5 files changed, 75 insertions(+), 8 deletions(-) diff --git a/src/Elastic.Documentation.Navigation/Isolated/Node/DocumentationSetNavigation.cs b/src/Elastic.Documentation.Navigation/Isolated/Node/DocumentationSetNavigation.cs index 16f9af2252..0efc0ba08e 100644 --- a/src/Elastic.Documentation.Navigation/Isolated/Node/DocumentationSetNavigation.cs +++ b/src/Elastic.Documentation.Navigation/Isolated/Node/DocumentationSetNavigation.cs @@ -196,14 +196,18 @@ private static IFileInfo ResolveFileInfo(IDocumentationSetContext context, strin private TModel? CreateDocumentationFile( IFileInfo fileInfo, IFileSystem fileSystem, - IDocumentationSetContext context, - string fullPath + IDocumentationSetContext context ) { var relativePath = Path.GetRelativePath(context.DocumentationSourceDirectory.FullName, fileInfo.FullName); var documentationFile = _factory.TryCreateDocumentationFile(fileInfo, fileSystem); if (documentationFile == null) - context.EmitError(context.ConfigurationPath, $"File navigation '{relativePath}' could not be created. {fullPath}"); + { + var reason = fileInfo.Exists + ? "the file exists but is not a valid Markdown document" + : "the file does not exist on disk"; + context.EmitError(context.ConfigurationPath, $"Table of contents references '{relativePath}' but {reason}."); + } return documentationFile; } @@ -248,7 +252,7 @@ INavigationHomeAccessor homeAccessor DetectionRuleRef ruleRef => ruleRef.FileInfo, _ => ResolveFileInfo(context, fullPath) }; - var documentationFile = CreateDocumentationFile(fileInfo, context.ReadFileSystem, context, fullPath); + var documentationFile = CreateDocumentationFile(fileInfo, context.ReadFileSystem, context); if (documentationFile == null) return null; diff --git a/src/Elastic.Documentation.Navigation/NavigationItemExtensions.cs b/src/Elastic.Documentation.Navigation/NavigationItemExtensions.cs index 898ad5c454..c9ce5ff86b 100644 --- a/src/Elastic.Documentation.Navigation/NavigationItemExtensions.cs +++ b/src/Elastic.Documentation.Navigation/NavigationItemExtensions.cs @@ -120,9 +120,14 @@ private static void BuildNavigationLookupsRecursive( _ = navigationByOrder.TryAdd(leaf.NavigationIndex, leaf); break; case INodeNavigationItem documentationFileNode: - _ = navigationDocumentationFileLookup.TryAdd(documentationFileNode.Index.Model, documentationFileNode); _ = navigationByOrder.TryAdd(documentationFileNode.NavigationIndex, documentationFileNode); - _ = navigationByOrder.TryAdd(documentationFileNode.Index.NavigationIndex, documentationFileNode.Index); + // Index is a null sentinel when this node's table of contents produced no items; the + // validation error is emitted upstream, so skip registering a missing index here. + if (documentationFileNode.Index is { } documentationFileIndex) + { + _ = navigationDocumentationFileLookup.TryAdd(documentationFileIndex.Model, documentationFileNode); + _ = navigationByOrder.TryAdd(documentationFileIndex.NavigationIndex, documentationFileIndex); + } foreach (var child in documentationFileNode.NavigationItems) BuildNavigationLookupsRecursive(child, navigationDocumentationFileLookup, navigationByOrder); break; diff --git a/src/Elastic.Markdown/IO/DocumentationSet.cs b/src/Elastic.Markdown/IO/DocumentationSet.cs index 9068220898..63d88f3661 100644 --- a/src/Elastic.Markdown/IO/DocumentationSet.cs +++ b/src/Elastic.Markdown/IO/DocumentationSet.cs @@ -129,8 +129,13 @@ private void VisitNavigation(INavigationItem item) extension.VisitNavigation(item, markdownLeaf.Model); break; case INodeNavigationItem node: - foreach (var extension in EnabledExtensions) - extension.VisitNavigation(node, node.Index.Model); + // Index is a null sentinel when this node's table of contents produced no items + // (a validation error is emitted upstream); skip visiting a missing index. + if (node.Index is { } nodeIndex) + { + foreach (var extension in EnabledExtensions) + extension.VisitNavigation(node, nodeIndex.Model); + } foreach (var child in node.NavigationItems) VisitNavigation(child); break; diff --git a/tests/Navigation.Tests/Isolation/ValidationTests.cs b/tests/Navigation.Tests/Isolation/ValidationTests.cs index f092ab8103..e617db30bb 100644 --- a/tests/Navigation.Tests/Isolation/ValidationTests.cs +++ b/tests/Navigation.Tests/Isolation/ValidationTests.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information using System.IO.Abstractions.TestingHelpers; +using System.Runtime.CompilerServices; using AwesomeAssertions; using Elastic.Documentation.Configuration.Toc; using Elastic.Documentation.Diagnostics; @@ -259,6 +260,38 @@ public async Task ValidationDoesNotEmitHintForSimpleVirtualFiles() context.Diagnostics.Should().BeEmpty(); } + [Fact] + public async Task BuildNavigationLookupsDoesNotThrowWhenTocReferencesMissingFile() + { + // language=yaml + var yaml = """ + project: 'test-project' + toc: + - file: missing.md + """; + + var fileSystem = new MockFileSystem(); + fileSystem.AddDirectory("/docs"); + // Note: /docs/missing.md is intentionally not created on disk + var context = CreateContext(fileSystem); + var docSet = DocumentationSetFile.LoadAndResolve(context.Collector, yaml, fileSystem.NewDirInfo("docs")); + _ = context.Collector.StartAsync(TestContext.Current.CancellationToken); + + var navigation = new DocumentationSetNavigation(docSet, context, MissingFileDocumentationFileFactory.Instance); + + var lookup = new ConditionalWeakTable(); + var buildLookups = () => navigation.BuildNavigationLookups(lookup); + buildLookups.Should().NotThrow("a missing toc file must surface a validation error, not crash the build"); + + await context.Collector.StopAsync(TestContext.Current.CancellationToken); + + context.Collector.Errors.Should().BeGreaterThan(0); + var diagnostics = context.Diagnostics; + diagnostics.Should().Contain(d => + d.Message.Contains("missing.md") && + d.Message.Contains("does not exist")); + } + [Fact] public async Task ValidationDoesNotEmitHintForFilesWithoutChildren() { diff --git a/tests/Navigation.Tests/TestDocumentationSetContext.cs b/tests/Navigation.Tests/TestDocumentationSetContext.cs index 288dae691f..ffb830ad56 100644 --- a/tests/Navigation.Tests/TestDocumentationSetContext.cs +++ b/tests/Navigation.Tests/TestDocumentationSetContext.cs @@ -171,6 +171,26 @@ public TestDocumentationFile TryCreateDocumentationFile(IFileInfo path, IFileSys } } +// Factory that mirrors production behaviour: returns null when the referenced file is not present on disk. +// Used to reproduce the missing-toc-file scenario where navigation produces a null Index sentinel. +public class MissingFileDocumentationFileFactory : IDocumentationFileFactory +{ + public static MissingFileDocumentationFileFactory Instance { get; } = new(); + + /// + public IDocumentationFile? TryCreateDocumentationFile(IFileInfo path, IFileSystem readFileSystem) + { + if (!path.Exists) + return null; + + var fileName = path.Name; + var title = fileName.EndsWith(".md", StringComparison.OrdinalIgnoreCase) + ? fileName[..^3] + : fileName; + return new TestDocumentationFile(title); + } +} + // Factory that creates base IDocumentationFile instances for tests that don't need concrete types public class GenericDocumentationFileFactory : IDocumentationFileFactory {