capture and report job deserialization errors

common/lib/dependabot/errors.rb

@@ -83,6 +83,11 @@ def self.fetcher_error_details(error)
       # and responsibility for fixing it is on them, not us. As a result we
       # quietly log these as errors
       { "error-type": "server_error" }
+    when BadRequirementError
+      {
+        "error-type": "illformed_requirement",
+        "error-detail": { message: error.message }
+      }
     when *Octokit::RATE_LIMITED_ERRORS
       # If we get a rate-limited error we let dependabot-api handle the
       # retry by re-enqueing the update job after the reset

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Analyze.cs

@@ -345,7 +345,8 @@ private static async Task RunAsync(
                     {
                         if (args[i] == "--job-path")
                         {
-                            experimentsManager = await ExperimentsManager.FromJobFileAsync(args[i + 1], new TestLogger());
+                            var experimentsResult = await ExperimentsManager.FromJobFileAsync(args[i + 1]);
+                            experimentsManager = experimentsResult.ExperimentsManager;
                         }
                     }
 

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Discover.cs

@@ -388,6 +388,83 @@ await RunAsync(path =>
             );
         }
 
+        [Fact]
+        public async Task JobFileParseErrorIsReported_InvalidJson()
+        {
+            using var testDirectory = new TemporaryDirectory();
+            var jobFilePath = Path.Combine(testDirectory.DirectoryPath, "job.json");
+            var resultFilePath = Path.Combine(testDirectory.DirectoryPath, DiscoveryWorker.DiscoveryResultFileName);
+            await File.WriteAllTextAsync(jobFilePath, "not json");
+            await RunAsync(path =>
+                [
+                    "discover",
+                    "--job-path",
+                    jobFilePath,
+                    "--repo-root",
+                    path,
+                    "--workspace",
+                    "/",
+                    "--output",
+                    resultFilePath
+                ],
+                initialFiles: [],
+                expectedResult: new()
+                {
+                    Path = "/",
+                    Projects = [],
+                    ErrorType = ErrorType.Unknown,
+                    ErrorDetailsPattern = "JsonException",
+                }
+            );
+        }
+
+        [Fact]
+        public async Task JobFileParseErrorIsReported_BadRequirement()
+        {
+            using var testDirectory = new TemporaryDirectory();
+            var jobFilePath = Path.Combine(testDirectory.DirectoryPath, "job.json");
+            var resultFilePath = Path.Combine(testDirectory.DirectoryPath, DiscoveryWorker.DiscoveryResultFileName);
+
+            // write a job file with a valid shape, but invalid requirement
+            await File.WriteAllTextAsync(jobFilePath, """
+                {
+                    "job": {
+                        "source": {
+                            "provider": "github",
+                            "repo": "test/repo"
+                        },
+                        "security-advisories": [
+                            {
+                                "dependency-name": "Some.Dependency",
+                                "affected-versions": ["not a valid requirement"]
+                            }
+                        ]
+                    }
+                }
+                """);
+            await RunAsync(path =>
+                [
+                    "discover",
+                    "--job-path",
+                    jobFilePath,
+                    "--repo-root",
+                    path,
+                    "--workspace",
+                    "/",
+                    "--output",
+                    resultFilePath
+                ],
+                initialFiles: [],
+                expectedResult: new()
+                {
+                    Path = "/",
+                    Projects = [],
+                    ErrorType = ErrorType.BadRequirement,
+                    ErrorDetailsPattern = "not a valid requirement",
+                }
+            );
+        }
+
         private static async Task RunAsync(
             Func<string, string[]> getArgs,
             TestFile[] initialFiles,
@@ -406,6 +483,7 @@ private static async Task RunAsync(
                 var originalErr = Console.Error;
                 Console.SetOut(writer);
                 Console.SetError(writer);
+                string? resultPath = null;
 
                 try
                 {
@@ -416,9 +494,15 @@ private static async Task RunAsync(
                     // manually pull out the experiments manager for the validate step below
                     for (int i = 0; i < args.Length - 1; i++)
                     {
-                        if (args[i] == "--job-path")
+                        switch (args[i])
                         {
-                            experimentsManager = await ExperimentsManager.FromJobFileAsync(args[i + 1], new TestLogger());
+                            case "--job-path":
+                                var experimentsResult = await ExperimentsManager.FromJobFileAsync(args[i + 1]);
+                                experimentsManager = experimentsResult.ExperimentsManager;
+                                break;
+                            case "--output":
+                                resultPath = args[i + 1];
+                                break;
                         }
                     }
 
@@ -434,7 +518,7 @@ private static async Task RunAsync(
                     Console.SetError(originalErr);
                 }
 
-                var resultPath = Path.Join(path, DiscoveryWorker.DiscoveryResultFileName);
+                resultPath ??= Path.Join(path, DiscoveryWorker.DiscoveryResultFileName);
                 var resultJson = await File.ReadAllTextAsync(resultPath);
                 var resultObject = JsonSerializer.Deserialize<WorkspaceDiscoveryResult>(resultJson, DiscoveryWorker.SerializerOptions);
                 return resultObject!;

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs

@@ -29,7 +29,7 @@ internal static Command GetCommand(Action<int> setExitCode)
         command.SetHandler(async (jobPath, repoRoot, discoveryPath, dependencyPath, analysisDirectory) =>
         {
             var logger = new ConsoleLogger();
-            var experimentsManager = await ExperimentsManager.FromJobFileAsync(jobPath.FullName, logger);
+            var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
             var worker = new AnalyzeWorker(experimentsManager, logger);
             await worker.RunAsync(repoRoot.FullName, discoveryPath.FullName, dependencyPath.FullName, analysisDirectory.FullName);
         }, JobPathOption, RepoRootOption, DiscoveryFilePathOption, DependencyFilePathOption, AnalysisFolderOption);

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/CloneCommand.cs

@@ -30,7 +30,7 @@ internal static Command GetCommand(Action<int> setExitCode)
             var apiHandler = new HttpApiHandler(apiUrl.ToString(), jobId);
             var logger = new ConsoleLogger();
             var gitCommandHandler = new ShellGitCommandHandler(logger);
-            var worker = new CloneWorker(apiHandler, gitCommandHandler, logger);
+            var worker = new CloneWorker(jobId, apiHandler, gitCommandHandler);
             var exitCode = await worker.RunAsync(jobPath, repoContentsPath);
             setExitCode(exitCode);
         }, JobPathOption, RepoContentsPathOption, ApiUrlOption, JobIdOption);

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs

@@ -26,8 +26,22 @@ internal static Command GetCommand(Action<int> setExitCode)
 
         command.SetHandler(async (jobPath, repoRoot, workspace, outputPath) =>
         {
+            var (experimentsManager, errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
+            if (errorResult is not null)
+            {
+                // to make testing easier, this should be a `WorkspaceDiscoveryResult` object
+                var discoveryErrorResult = new WorkspaceDiscoveryResult
+                {
+                    Path = workspace,
+                    Projects = [],
+                    ErrorType = errorResult.ErrorType,
+                    ErrorDetails = errorResult.ErrorDetails,
+                };
+                await DiscoveryWorker.WriteResultsAsync(repoRoot.FullName, outputPath.FullName, discoveryErrorResult);
+                return;
+            }
+
             var logger = new ConsoleLogger();
-            var experimentsManager = await ExperimentsManager.FromJobFileAsync(jobPath.FullName, logger);
             var worker = new DiscoveryWorker(experimentsManager, logger);
             await worker.RunAsync(repoRoot.FullName, workspace, outputPath.FullName);
         }, JobPathOption, RepoRootOption, WorkspaceOption, OutputOption);

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs

@@ -33,12 +33,12 @@ internal static Command GetCommand(Action<int> setExitCode)
         command.SetHandler(async (jobPath, repoContentsPath, apiUrl, jobId, outputPath, baseCommitSha) =>
         {
             var apiHandler = new HttpApiHandler(apiUrl.ToString(), jobId);
+            var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
             var logger = new ConsoleLogger();
-            var experimentsManager = await ExperimentsManager.FromJobFileAsync(jobPath.FullName, logger);
             var discoverWorker = new DiscoveryWorker(experimentsManager, logger);
             var analyzeWorker = new AnalyzeWorker(experimentsManager, logger);
             var updateWorker = new UpdaterWorker(experimentsManager, logger);
-            var worker = new RunWorker(apiHandler, discoverWorker, analyzeWorker, updateWorker, logger);
+            var worker = new RunWorker(jobId, apiHandler, discoverWorker, analyzeWorker, updateWorker, logger);
             await worker.RunAsync(jobPath, repoContentsPath, baseCommitSha, outputPath);
         }, JobPathOption, RepoContentsPathOption, ApiUrlOption, JobIdOption, OutputPathOption, BaseCommitShaOption);
 

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs

@@ -33,8 +33,8 @@ internal static Command GetCommand(Action<int> setExitCode)
 
         command.SetHandler(async (jobPath, repoRoot, solutionOrProjectFile, dependencyName, newVersion, previousVersion, isTransitive, resultOutputPath) =>
         {
+            var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
             var logger = new ConsoleLogger();
-            var experimentsManager = await ExperimentsManager.FromJobFileAsync(jobPath.FullName, logger);
             var worker = new UpdaterWorker(experimentsManager, logger);
             await worker.RunAsync(repoRoot.FullName, solutionOrProjectFile.FullName, dependencyName, previousVersion, newVersion, isTransitive, resultOutputPath);
             setExitCode(0);

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Clone/CloneWorkerTests.cs

@@ -95,6 +95,65 @@ await TestCloneAsync(
         );
     }
 
+    [Fact]
+    public async Task JobFileParseErrorIsReported_InvalidJson()
+    {
+        // arrange
+        var testApiHandler = new TestApiHandler();
+        var testGitCommandHandler = new TestGitCommandHandler();
+        var cloneWorker = new CloneWorker("JOB-ID", testApiHandler, testGitCommandHandler);
+        using var testDirectory = new TemporaryDirectory();
+        var jobFilePath = Path.Combine(testDirectory.DirectoryPath, "job.json");
+        await File.WriteAllTextAsync(jobFilePath, "not json");
+
+        // act
+        var result = await cloneWorker.RunAsync(new FileInfo(jobFilePath), new DirectoryInfo(testDirectory.DirectoryPath));
+
+        // assert
+        Assert.Equal(1, result);
+        var expectedParseErrorObject = testApiHandler.ReceivedMessages.Single(m => m.Type == typeof(UnknownError));
+        var unknownError = (UnknownError)expectedParseErrorObject.Object;
+        Assert.Equal("JsonException", unknownError.Details["error-class"]);
+    }
+
+    [Fact]
+    public async Task JobFileParseErrorIsReported_BadRequirement()
+    {
+        // arrange
+        var testApiHandler = new TestApiHandler();
+        var testGitCommandHandler = new TestGitCommandHandler();
+        var cloneWorker = new CloneWorker("JOB-ID", testApiHandler, testGitCommandHandler);
+        using var testDirectory = new TemporaryDirectory();
+        var jobFilePath = Path.Combine(testDirectory.DirectoryPath, "job.json");
+
+        // write a job file with a valid shape, but invalid requirement
+        await File.WriteAllTextAsync(jobFilePath, """
+            {
+                "job": {
+                    "source": {
+                        "provider": "github",
+                        "repo": "test/repo"
+                    },
+                    "security-advisories": [
+                        {
+                            "dependency-name": "Some.Dependency",
+                            "affected-versions": ["not a valid requirement"]
+                        }
+                    ]
+                }
+            }
+            """);
+
+        // act
+        var result = await cloneWorker.RunAsync(new FileInfo(jobFilePath), new DirectoryInfo(testDirectory.DirectoryPath));
+
+        // assert
+        Assert.Equal(1, result);
+        var expectedParseErrorObject = testApiHandler.ReceivedMessages.Single(m => m.Type == typeof(BadRequirement));
+        var badRequirement = (BadRequirement)expectedParseErrorObject.Object;
+        Assert.Equal("not a valid requirement", badRequirement.Details["message"]);
+    }
+
     private class TestGitCommandHandlerWithOutputs : TestGitCommandHandler
     {
         private readonly string _stdout;
@@ -134,8 +193,7 @@ private static async Task TestCloneAsync(string provider, string repoMoniker, ob
         // arrange
         var testApiHandler = new TestApiHandler();
         testGitCommandHandler ??= new TestGitCommandHandler();
-        var testLogger = new TestLogger();
-        var worker = new CloneWorker(testApiHandler, testGitCommandHandler, testLogger);
+        var worker = new CloneWorker("TEST-JOB-ID", testApiHandler, testGitCommandHandler);
 
         // act
         var job = new Job()

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs

@@ -45,7 +45,16 @@ protected static void ValidateWorkspaceResult(ExpectedWorkspaceDiscoveryResult e
         ValidateProjectResults(expectedResult.Projects, actualResult.Projects, experimentsManager);
         Assert.Equal(expectedResult.ExpectedProjectCount ?? expectedResult.Projects.Length, actualResult.Projects.Length);
         Assert.Equal(expectedResult.ErrorType, actualResult.ErrorType);
-        Assert.Equal(expectedResult.ErrorDetails, actualResult.ErrorDetails);
+        if (expectedResult.ErrorDetailsPattern is not null)
+        {
+            var errorDetails = actualResult.ErrorDetails?.ToString();
+            Assert.NotNull(errorDetails);
+            Assert.Matches(expectedResult.ErrorDetailsPattern, errorDetails);
+        }
+        else
+        {
+            Assert.Equal(expectedResult.ErrorDetails, actualResult.ErrorDetails);
+        }
 
         return;
 

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/ExpectedDiscoveryResults.cs

@@ -12,6 +12,7 @@ public record ExpectedWorkspaceDiscoveryResult : NativeResult
     public int? ExpectedProjectCount { get; init; }
     public ExpectedDependencyDiscoveryResult? GlobalJson { get; init; }
     public ExpectedDependencyDiscoveryResult? DotNetToolsJson { get; init; }
+    public string? ErrorDetailsPattern { get; init; } = null;
 }
 
 public record ExpectedSdkProjectDiscoveryResult : ExpectedDependencyDiscoveryResult

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/RunWorkerTests.cs

@@ -1733,7 +1733,7 @@ private static async Task RunAsync(Job job, TestFile[] files, IDiscoveryWorker?
         analyzeWorker ??= new AnalyzeWorker(experimentsManager, logger);
         updaterWorker ??= new UpdaterWorker(experimentsManager, logger);
 
-        var worker = new RunWorker(testApiHandler, discoveryWorker, analyzeWorker, updaterWorker, logger);
+        var worker = new RunWorker("TEST-JOB-ID", testApiHandler, discoveryWorker, analyzeWorker, updaterWorker, logger);
         var repoContentsPathDirectoryInfo = new DirectoryInfo(tempDirectory.DirectoryPath);
         var actualResult = await worker.RunAsync(job, repoContentsPathDirectoryInfo, "TEST-COMMIT-SHA");
         var actualApiMessages = testApiHandler.ReceivedMessages.ToArray();