From d389600a1c38b503bbf6ab3498591f960ac9bd6a Mon Sep 17 00:00:00 2001 From: Matt Warren Date: Fri, 1 May 2026 16:05:09 -0700 Subject: [PATCH 1/3] Catch exceptions and log when parsing graph models and snapshots Co-authored-by: Copilot --- src/Server/Schema/ServerSchemaSource.cs | 60 ++-- src/Server/Utilities/GraphModel.cs | 24 +- .../Features/ServerSchemaSourceTests.cs | 284 ++++++++++++++++++ 3 files changed, 339 insertions(+), 29 deletions(-) create mode 100644 src/ServerTests/Features/ServerSchemaSourceTests.cs diff --git a/src/Server/Schema/ServerSchemaSource.cs b/src/Server/Schema/ServerSchemaSource.cs index 4ef7ced..a742a65 100644 --- a/src/Server/Schema/ServerSchemaSource.cs +++ b/src/Server/Schema/ServerSchemaSource.cs @@ -1,11 +1,11 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using Kusto.Data; -using Kusto.Data.Common; -using Kusto.Language; -using Kusto.Language.Symbols; -using Newtonsoft.Json; +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using Kusto.Data; +using Kusto.Data.Common; +using Kusto.Language; +using Kusto.Language.Symbols; +using Newtonsoft.Json; using System.Collections.Immutable; namespace Kusto.Vscode; @@ -399,21 +399,39 @@ private async Task> CreateGraphModelInfosAsync( return graphEntities .Select(e => { - snapshotMap.TryGetValue(e.EntityName, out var snapshots); - var snapshotNames = snapshots != null - ? JsonConvert.DeserializeObject>(snapshots) ?? ImmutableList.Empty - : ImmutableList.Empty; + var snapshotNames = ImmutableList.Empty; + if (snapshotMap.TryGetValue(e.EntityName, out var snapshots) + && snapshots != null) + { + try + { + snapshotNames = JsonConvert.DeserializeObject>(snapshots) ?? ImmutableList.Empty; + } + catch + { + _logger?.Log($"ServerSchemaSource: Failed to parse snapshots for graph model: {e.EntityName}"); + } + } - GraphModel.TryParse(e.Content, out var model); - return new GraphModelInfo + if (GraphModel.TryParse(e.Content, out var model)) { - Name = e.EntityName, - Model = e.Content, - Snapshots = snapshotNames, - Description = e.DocString, - Folder = e.Folder - }; - }).ToImmutableList(); + return new GraphModelInfo + { + Name = e.EntityName, + Model = e.Content, + Snapshots = snapshotNames, + Description = e.DocString, + Folder = e.Folder + }; + } + else + { + _logger?.Log($"ServerSchemaSource: Failed to parse graph model for model: {e.EntityName}"); + return null; + } + }) + .OfType() + .ToImmutableList(); } public async Task> GetStoredQueryResultInfosAsync( diff --git a/src/Server/Utilities/GraphModel.cs b/src/Server/Utilities/GraphModel.cs index ef86bf4..805b9dc 100644 --- a/src/Server/Utilities/GraphModel.cs +++ b/src/Server/Utilities/GraphModel.cs @@ -1,9 +1,9 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; - +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; + namespace Kusto.Vscode; // disable nullability to allow construction of target instances before deserializing into them @@ -15,8 +15,16 @@ public class GraphModel public static bool TryParse(string text, out GraphModel model) { - model = JsonConvert.DeserializeObject(text); - return model != null; + try + { + model = JsonConvert.DeserializeObject(text); + return model != null; + } + catch + { + model = null!; + return false; + } } public override string ToString() diff --git a/src/ServerTests/Features/ServerSchemaSourceTests.cs b/src/ServerTests/Features/ServerSchemaSourceTests.cs new file mode 100644 index 0000000..2d08450 --- /dev/null +++ b/src/ServerTests/Features/ServerSchemaSourceTests.cs @@ -0,0 +1,284 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Collections.Immutable; +using System.Data; +using System.Diagnostics.CodeAnalysis; +using Kusto.Data; +using Kusto.Data.Common; +using Kusto.Data.Data; +using Kusto.Language; +using Kusto.Language.Editor; +using Kusto.Vscode; +using Newtonsoft.Json; + +namespace Tests.Features; + +[TestClass] +public class ServerSchemaSourceTests +{ + private const string TestCluster = "testcluster.kusto.windows.net"; + private const string TestDatabase = "testdb"; + + // A minimal but valid GraphModel JSON. The parser uses Newtonsoft.Json to + // deserialize directly into the GraphModel type; an empty object suffices. + private const string ValidGraphModelContent = "{}"; + + [TestMethod] + public async Task GetGraphModelInfosAsync_NoSnapshotData_LoadsGraphModel() + { + // Graph model exists, but the snapshots query returns no rows for it. + // Schema loading must not break, and the graph model should still be + // returned (with an empty snapshots list). + var entities = new List + { + CreateGraphEntity("MyGraph", content: ValidGraphModelContent), + }; + var snapshots = new List(); + + var source = CreateSource(entities, snapshots); + + var result = await source.GetGraphModelInfosAsync(TestCluster, TestDatabase, null, CancellationToken.None); + + Assert.IsNotNull(result); + var graph = result.FirstOrDefault(g => g.Name == "MyGraph"); + Assert.IsNotNull(graph, "Graph model should load even with no snapshot data."); + Assert.IsEmpty(graph.Snapshots); + } + + [TestMethod] + public async Task GetGraphModelInfosAsync_NullSnapshotsField_LoadsGraphModel() + { + var entities = new List + { + CreateGraphEntity("MyGraph", content: ValidGraphModelContent), + }; + var snapshots = new List + { + new() { ModelName = "MyGraph", Snapshots = null! }, + }; + + var source = CreateSource(entities, snapshots); + + var result = await source.GetGraphModelInfosAsync(TestCluster, TestDatabase, null, CancellationToken.None); + + Assert.IsNotNull(result); + var graph = result.FirstOrDefault(g => g.Name == "MyGraph"); + Assert.IsNotNull(graph, "Graph model should load even when its snapshots field is null."); + Assert.IsEmpty(graph.Snapshots); + } + + [TestMethod] + public async Task GetGraphModelInfosAsync_BadSnapshotJson_LoadsGraphModelWithEmptySnapshots() + { + var entities = new List + { + CreateGraphEntity("MyGraph", content: ValidGraphModelContent), + }; + var snapshots = new List + { + new() { ModelName = "MyGraph", Snapshots = "this is not json" }, + }; + + var source = CreateSource(entities, snapshots); + + // The bug-fix wraps deserialization in a try/catch so bad JSON should + // not propagate and break schema loading. + var result = await source.GetGraphModelInfosAsync(TestCluster, TestDatabase, null, CancellationToken.None); + + Assert.IsNotNull(result); + var graph = result.FirstOrDefault(g => g.Name == "MyGraph"); + Assert.IsNotNull(graph, "Graph model should load even when snapshot JSON is malformed."); + Assert.IsEmpty(graph.Snapshots); + } + + [TestMethod] + public async Task GetGraphModelInfosAsync_BadSnapshotJsonOnOneModel_OtherModelsLoad() + { + var entities = new List + { + CreateGraphEntity("BadGraph", content: ValidGraphModelContent), + CreateGraphEntity("GoodGraph", content: ValidGraphModelContent), + }; + var snapshots = new List + { + new() { ModelName = "BadGraph", Snapshots = "}{not json" }, + new() { ModelName = "GoodGraph", Snapshots = JsonConvert.SerializeObject(new[] { "snap1" }) }, + }; + + var source = CreateSource(entities, snapshots); + + var result = await source.GetGraphModelInfosAsync(TestCluster, TestDatabase, null, CancellationToken.None); + + Assert.IsNotNull(result); + var good = result.FirstOrDefault(g => g.Name == "GoodGraph"); + Assert.IsNotNull(good, "GoodGraph should still be loaded despite a sibling having bad JSON."); + Assert.HasCount(1, good.Snapshots); + Assert.AreEqual("snap1", good.Snapshots[0]); + } + + [TestMethod] + public async Task GetDatabaseInfoAsync_GraphModelWithBadSnapshotJson_DoesNotBreakSchemaLoading() + { + // Verify the broader schema-load path also tolerates the bad data. + var entities = new List + { + CreateGraphEntity("MyGraph", content: ValidGraphModelContent), + }; + var snapshots = new List + { + new() { ModelName = "MyGraph", Snapshots = "garbage" }, + }; + + var source = CreateSource(entities, snapshots); + + var info = await source.GetDatabaseInfoAsync(TestCluster, TestDatabase, null, CancellationToken.None); + + Assert.IsNotNull(info); + Assert.AreEqual(TestDatabase, info.Name); + Assert.HasCount(1, info.GraphModels); + } + + [TestMethod] + public async Task GetDatabaseInfoAsync_GraphModelWithoutSnapshotData_DoesNotBreakSchemaLoading() + { + var entities = new List + { + CreateGraphEntity("MyGraph", content: ValidGraphModelContent), + }; + var snapshots = new List(); + + var source = CreateSource(entities, snapshots); + + var info = await source.GetDatabaseInfoAsync(TestCluster, TestDatabase, null, CancellationToken.None); + + Assert.IsNotNull(info); + Assert.AreEqual(TestDatabase, info.Name); + Assert.HasCount(1, info.GraphModels); + } + + #region Helpers + + private static ServerSchemaSource CreateSource( + IEnumerable entities, + IEnumerable snapshots) + { + var connection = new TestConnection(TestCluster, TestDatabase) + { + EntitiesResult = entities.ToImmutableList(), + GraphSnapshotsResult = snapshots.ToImmutableList(), + DatabaseIdentityResult = ImmutableList.Create( + new ServerSchemaSource.DatabaseNamesResult + { + DatabaseName = TestDatabase, + PrettyName = TestDatabase, + }), + }; + + var manager = new TestConnectionManager(connection); + return new ServerSchemaSource(manager, logger: null); + } + + private static DatabasesEntitiesShowCommandResult CreateGraphEntity(string name, string content) + { + return new DatabasesEntitiesShowCommandResult + { + DatabaseName = TestDatabase, + EntityType = "Graph", + EntityName = name, + Content = content, + DocString = null, + Folder = null, + CslInputSchema = null, + CslOutputSchema = null, + }; + } + + #endregion + + #region Test Doubles + + private sealed class TestConnectionManager : IConnectionManager + { + private readonly IConnection _connection; + + public TestConnectionManager(IConnection connection) + { + _connection = connection; + } + + public IConnection GetOrAddConnection(string connectionStrings) => _connection; + + public bool TryGetConnection(string clusterName, [NotNullWhen(true)] out IConnection? connection) + { + connection = _connection; + return true; + } + } + + private sealed class TestConnection : IConnection + { + public TestConnection(string cluster, string? database) + { + Cluster = cluster; + Database = database; + } + + public string Cluster { get; } + public string? Database { get; } + + public ImmutableList EntitiesResult { get; set; } + = ImmutableList.Empty; + + public ImmutableList GraphSnapshotsResult { get; set; } + = ImmutableList.Empty; + + public ImmutableList DatabaseIdentityResult { get; set; } + = ImmutableList.Empty; + + public IConnection WithCluster(string clusterName) => this; + public IConnection WithDatabase(string databaseName) => this; + + public Task ExecuteAsync( + EditString query, + ImmutableDictionary? options = null, + ImmutableDictionary? parameters = null, + CancellationToken cancellationToken = default) + { + return Task.FromResult(new ExecuteResult()); + } + + public Task> ExecuteAsync( + EditString query, + ImmutableDictionary? options = null, + ImmutableDictionary? parameters = null, + CancellationToken cancellationToken = default) + { + object? values = null; + + if (typeof(T) == typeof(DatabasesEntitiesShowCommandResult)) + { + values = EntitiesResult; + } + else if (typeof(T) == typeof(ServerSchemaSource.LoadGraphModelSnapshotsResult)) + { + values = GraphSnapshotsResult; + } + else if (typeof(T) == typeof(ServerSchemaSource.DatabaseNamesResult)) + { + values = DatabaseIdentityResult; + } + + var result = new ExecuteResult + { + Values = (ImmutableList?)values, + }; + return Task.FromResult(result); + } + + public Task GetServerKindAsync(CancellationToken cancellationToken) + => Task.FromResult("Engine"); + } + + #endregion +} From b5671576c41e168787a4fb4a6bdd21931e2af515 Mon Sep 17 00:00:00 2001 From: Matt Warren Date: Fri, 1 May 2026 16:13:12 -0700 Subject: [PATCH 2/3] No need to parse model if we just pass it along as text. --- src/Server/Schema/ServerSchemaSource.cs | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/Server/Schema/ServerSchemaSource.cs b/src/Server/Schema/ServerSchemaSource.cs index a742a65..e9b65c8 100644 --- a/src/Server/Schema/ServerSchemaSource.cs +++ b/src/Server/Schema/ServerSchemaSource.cs @@ -413,24 +413,15 @@ private async Task> CreateGraphModelInfosAsync( } } - if (GraphModel.TryParse(e.Content, out var model)) + return new GraphModelInfo { - return new GraphModelInfo - { - Name = e.EntityName, - Model = e.Content, - Snapshots = snapshotNames, - Description = e.DocString, - Folder = e.Folder - }; - } - else - { - _logger?.Log($"ServerSchemaSource: Failed to parse graph model for model: {e.EntityName}"); - return null; - } + Name = e.EntityName, + Model = e.Content, + Snapshots = snapshotNames, + Description = e.DocString, + Folder = e.Folder + }; }) - .OfType() .ToImmutableList(); } From 8be16ce891cb0e84829a9af866346615ac2fa740 Mon Sep 17 00:00:00 2001 From: Matt Warren Date: Fri, 1 May 2026 16:15:25 -0700 Subject: [PATCH 3/3] Add exception message to log --- src/Server/Schema/ServerSchemaSource.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Server/Schema/ServerSchemaSource.cs b/src/Server/Schema/ServerSchemaSource.cs index e9b65c8..674620e 100644 --- a/src/Server/Schema/ServerSchemaSource.cs +++ b/src/Server/Schema/ServerSchemaSource.cs @@ -401,15 +401,15 @@ private async Task> CreateGraphModelInfosAsync( { var snapshotNames = ImmutableList.Empty; if (snapshotMap.TryGetValue(e.EntityName, out var snapshots) - && snapshots != null) + && !string.IsNullOrWhiteSpace(snapshots)) { try { snapshotNames = JsonConvert.DeserializeObject>(snapshots) ?? ImmutableList.Empty; } - catch + catch (JsonException je) { - _logger?.Log($"ServerSchemaSource: Failed to parse snapshots for graph model: {e.EntityName}"); + _logger?.Log($"ServerSchemaSource: Failed to parse snapshots for graph model '{e.EntityName}': {je.Message}"); } }