diff --git a/common/lib/dependabot/errors.rb b/common/lib/dependabot/errors.rb index 69393d6e4b..1db1a2873d 100644 --- a/common/lib/dependabot/errors.rb +++ b/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 diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Analyze.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Analyze.cs index b5d08165d0..ef0799a4e4 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Analyze.cs +++ b/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; } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Discover.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Discover.cs index ad43a2292d..2622c56e85 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Discover.cs +++ b/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 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(resultJson, DiscoveryWorker.SerializerOptions); return resultObject!; diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs index 347c8adce4..0133b62b87 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs @@ -29,7 +29,7 @@ internal static Command GetCommand(Action 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); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/CloneCommand.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/CloneCommand.cs index 581d95de0f..3a203b2895 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/CloneCommand.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/CloneCommand.cs @@ -30,7 +30,7 @@ internal static Command GetCommand(Action 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); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs index 1a11834cc5..d3d62b1a1e 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs @@ -26,8 +26,22 @@ internal static Command GetCommand(Action 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); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs index 53d9ff4d2d..439c2b07d6 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs @@ -33,12 +33,12 @@ internal static Command GetCommand(Action 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); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs index 0b79b6da45..bd04c652cc 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs @@ -33,8 +33,8 @@ internal static Command GetCommand(Action 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); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Clone/CloneWorkerTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Clone/CloneWorkerTests.cs index 71e6387bff..b658dc6f3f 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Clone/CloneWorkerTests.cs +++ b/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() diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs index 018729bd6c..4da255d04f 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs +++ b/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; diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/ExpectedDiscoveryResults.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/ExpectedDiscoveryResults.cs index 7e3638103f..d9d434aad6 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/ExpectedDiscoveryResults.cs +++ b/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 diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/RunWorkerTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/RunWorkerTests.cs index f02300e866..c58a29fcd7 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/RunWorkerTests.cs +++ b/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(); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/SerializationTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/SerializationTests.cs index 4d14c6c70f..5cbbedd511 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/SerializationTests.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/SerializationTests.cs @@ -1,6 +1,5 @@ using NuGet.Versioning; -using NuGetUpdater.Core.Analyze; using NuGetUpdater.Core.Run; using NuGetUpdater.Core.Run.ApiModel; using NuGetUpdater.Core.Test.Utilities; @@ -228,37 +227,33 @@ public void DeserializeExperimentsManager_NoExperiments() Assert.False(experimentsManager.UseDirectDiscovery); } - [Fact] - public async Task DeserializeExperimentsManager_UnsupportedJobFileShape_InfoIsReportedAndEmptyExperimentSetIsReturned() + [Theory] + [MemberData(nameof(DeserializeErrorTypesData))] + public void SerializeError(JobErrorBase error, string expectedSerialization) { - // arrange - using var tempDir = new TemporaryDirectory(); - var jobFilePath = Path.Combine(tempDir.DirectoryPath, "job.json"); - var jobContent = """ - { - "this-is-not-a-job-and-parsing-will-fail-but-an-empty-experiment-set-should-sill-be-returned": { - } - } - """; - await File.WriteAllTextAsync(jobFilePath, jobContent); - var capturingTestLogger = new CapturingTestLogger(); - - // act - this is the entrypoint the update command uses to parse the job file - var experimentsManager = await ExperimentsManager.FromJobFileAsync(jobFilePath, capturingTestLogger); + if (error is UnknownError unknown) + { + // special case the exception's call stack to make it testable + unknown.Details["error-backtrace"] = "TEST-BACKTRACE"; + } - // assert - Assert.False(experimentsManager.UseLegacyDependencySolver); - Assert.False(experimentsManager.UseDirectDiscovery); - Assert.Single(capturingTestLogger.Messages, m => m.Contains("Error deserializing job file")); + var actual = HttpApiHandler.Serialize(error); + Assert.Equal(expectedSerialization, actual); } [Fact] - public void SerializeError() + public void SerializeError_AllErrorTypesHaveSerializationTests() { - var error = new JobRepoNotFound("some message"); - var actual = HttpApiHandler.Serialize(error); - var expected = """{"data":{"error-type":"job_repo_not_found","error-details":{"message":"some message"}}}"""; - Assert.Equal(expected, actual); + var untestedTypes = typeof(JobErrorBase).Assembly.GetTypes() + .Where(t => t.IsSubclassOf(typeof(JobErrorBase))) + .ToHashSet(); + foreach (object?[] data in DeserializeErrorTypesData()) + { + var testedErrorType = data[0]!.GetType(); + untestedTypes.Remove(testedErrorType); + } + + Assert.Empty(untestedTypes.Select(t => t.Name)); } [Fact] @@ -514,6 +509,57 @@ public void DeserializeCommitOptions() Assert.Null(jobWrapper.Job.CommitMessageOptions!.IncludeScope); } + public static IEnumerable DeserializeErrorTypesData() + { + yield return + [ + new BadRequirement("some message"), + """ + {"data":{"error-type":"illformed_requirement","error-details":{"message":"some message"}}} + """ + ]; + + yield return + [ + new DependencyFileNotFound("some message", "/some/file"), + """ + {"data":{"error-type":"dependency_file_not_found","error-details":{"message":"some message","file-path":"/some/file"}}} + """ + ]; + + yield return + [ + new JobRepoNotFound("some message"), + """ + {"data":{"error-type":"job_repo_not_found","error-details":{"message":"some message"}}} + """ + ]; + + yield return + [ + new PrivateSourceAuthenticationFailure(["url1", "url2"]), + """ + {"data":{"error-type":"private_source_authentication_failure","error-details":{"source":"(url1|url2)"}}} + """ + ]; + + yield return + [ + new UnknownError(new Exception("some message"), "JOB-ID"), + """ + {"data":{"error-type":"unknown_error","error-details":{"error-class":"Exception","error-message":"some message","error-backtrace":"TEST-BACKTRACE","package-manager":"nuget","job-id":"JOB-ID"}}} + """ + ]; + + yield return + [ + new UpdateNotPossible(["dep1", "dep2"]), + """ + {"data":{"error-type":"update_not_possible","error-details":{"dependencies":["dep1","dep2"]}}} + """ + ]; + } + public static IEnumerable DeserializeAllowedUpdatesData() { // common default value - most job files look like this @@ -596,16 +642,4 @@ public void DeserializeCommitOptions() } ]; } - - private class CapturingTestLogger : ILogger - { - private readonly List _messages = new(); - - public IReadOnlyList Messages => _messages; - - public void LogRaw(string message) - { - _messages.Add(message); - } - } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/RequirementConverter.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/RequirementConverter.cs index 361643f67f..1da74d4326 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/RequirementConverter.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/RequirementConverter.cs @@ -7,7 +7,20 @@ public class RequirementConverter : JsonConverter { public override Requirement? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - return Requirement.Parse(reader.GetString()!); + var text = reader.GetString(); + if (text is null) + { + throw new ArgumentNullException(nameof(text)); + } + + try + { + return Requirement.Parse(text); + } + catch + { + throw new BadRequirementException(text); + } } public override void Write(Utf8JsonWriter writer, Requirement value, JsonSerializerOptions options) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/BadRequirementException.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/BadRequirementException.cs new file mode 100644 index 0000000000..1d9ffa78c2 --- /dev/null +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/BadRequirementException.cs @@ -0,0 +1,9 @@ +namespace NuGetUpdater.Core; + +internal class BadRequirementException : Exception +{ + public BadRequirementException(string details) + : base(details) + { + } +} diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Clone/CloneWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Clone/CloneWorker.cs index 761a364f77..4f48a8d477 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Clone/CloneWorker.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Clone/CloneWorker.cs @@ -9,23 +9,45 @@ namespace NuGetUpdater.Core.Clone; public class CloneWorker { + private readonly string _jobId; private readonly IApiHandler _apiHandler; private readonly IGitCommandHandler _gitCommandHandler; - private readonly ILogger _logger; - public CloneWorker(IApiHandler apiHandler, IGitCommandHandler gitCommandHandler, ILogger logger) + public CloneWorker(string jobId, IApiHandler apiHandler, IGitCommandHandler gitCommandHandler) { + _jobId = jobId; _apiHandler = apiHandler; _gitCommandHandler = gitCommandHandler; - _logger = logger; } // entrypoint for cli public async Task RunAsync(FileInfo jobFilePath, DirectoryInfo repoContentsPath) { var jobFileContent = await File.ReadAllTextAsync(jobFilePath.FullName); - var jobWrapper = RunWorker.Deserialize(jobFileContent); - var result = await RunAsync(jobWrapper.Job, repoContentsPath.FullName); + + // only a limited set of errors can occur here + JobFile? jobFile = null; + JobErrorBase? parseError = null; + try + { + jobFile = RunWorker.Deserialize(jobFileContent); + } + catch (BadRequirementException ex) + { + parseError = new BadRequirement(ex.Message); + } + catch (Exception ex) + { + parseError = new UnknownError(ex, _jobId); + } + + if (parseError is not null) + { + await ReportError(parseError); + return 1; + } + + var result = await RunAsync(jobFile!.Job, repoContentsPath.FullName); return result; } @@ -48,19 +70,24 @@ public async Task RunAsync(Job job, string repoContentsPath) } catch (Exception ex) { - error = new UnknownError(ex.ToString()); + error = new UnknownError(ex, _jobId); } if (error is not null) { - await _apiHandler.RecordUpdateJobError(error); - await _apiHandler.MarkAsProcessed(new("unknown")); + await ReportError(error); return 1; } return 0; } + private async Task ReportError(JobErrorBase error) + { + await _apiHandler.RecordUpdateJobError(error); + await _apiHandler.MarkAsProcessed(new("unknown")); + } + internal static CommandArguments[] GetAllCommandArgs(Job job, string repoContentsPath) { var commandArgs = new List() diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs index 398f04e54f..fdeaf75733 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs @@ -57,6 +57,16 @@ internal async Task RunWithErrorHandlingAsync(string r Projects = [], }; } + catch (Exception ex) + { + result = new WorkspaceDiscoveryResult + { + ErrorType = ErrorType.Unknown, + ErrorDetails = ex.ToString(), + Path = workspacePath, + Projects = [], + }; + } return result; } @@ -397,6 +407,6 @@ internal static async Task WriteResultsAsync(string repoRootPath, string outputP } var resultJson = JsonSerializer.Serialize(result, SerializerOptions); - await File.WriteAllTextAsync(path: resultPath, resultJson); + await File.WriteAllTextAsync(resultPath, resultJson); } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ErrorType.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ErrorType.cs index 36154ad039..23dab4bb8d 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ErrorType.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ErrorType.cs @@ -4,6 +4,7 @@ public enum ErrorType { None, AuthenticationFailure, + BadRequirement, MissingFile, UpdateNotPossible, DependencyFileNotParseable, diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs index 8e3cc01221..96a00680b7 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs @@ -30,19 +30,42 @@ public static ExperimentsManager GetExperimentsManager(Dictionary FromJobFileAsync(string jobFilePath, ILogger logger) + public static async Task<(ExperimentsManager ExperimentsManager, NativeResult? ErrorResult)> FromJobFileAsync(string jobFilePath) { - var jobFileContent = await File.ReadAllTextAsync(jobFilePath); + var experimentsManager = new ExperimentsManager(); + NativeResult? errorResult = null; try { + var jobFileContent = await File.ReadAllTextAsync(jobFilePath); var jobWrapper = RunWorker.Deserialize(jobFileContent); - return GetExperimentsManager(jobWrapper.Job.Experiments); + experimentsManager = GetExperimentsManager(jobWrapper.Job.Experiments); + } + catch (BadRequirementException ex) + { + errorResult = new NativeResult + { + ErrorType = ErrorType.BadRequirement, + ErrorDetails = ex.Message, + }; } catch (JsonException ex) { - logger.Info($"Error deserializing job file: {ex.ToString()}: {jobFileContent}"); - return new ExperimentsManager(); + errorResult = new NativeResult + { + ErrorType = ErrorType.Unknown, + ErrorDetails = $"Error deserializing job file: {ex}: {File.ReadAllText(jobFilePath)}", + }; + } + catch (Exception ex) + { + errorResult = new NativeResult + { + ErrorType = ErrorType.Unknown, + ErrorDetails = ex.ToString(), + }; } + + return (experimentsManager, errorResult); } private static bool IsEnabled(Dictionary? experiments, string experimentName) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NuGetUpdater.Core.csproj b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NuGetUpdater.Core.csproj index d45ed65905..7bb1102c92 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NuGetUpdater.Core.csproj +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NuGetUpdater.Core.csproj @@ -26,6 +26,7 @@ + diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/BadRequirement.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/BadRequirement.cs new file mode 100644 index 0000000000..40cff22835 --- /dev/null +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/BadRequirement.cs @@ -0,0 +1,10 @@ +namespace NuGetUpdater.Core.Run.ApiModel; + +public record BadRequirement : JobErrorBase +{ + public BadRequirement(string details) + : base("illformed_requirement") + { + Details["message"] = details; + } +} diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/DependencyFileNotFound.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/DependencyFileNotFound.cs index 94c1b71f08..91d385da47 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/DependencyFileNotFound.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/DependencyFileNotFound.cs @@ -2,9 +2,10 @@ namespace NuGetUpdater.Core.Run.ApiModel; public record DependencyFileNotFound : JobErrorBase { - public DependencyFileNotFound(string filePath) + public DependencyFileNotFound(string message, string filePath) : base("dependency_file_not_found") { - Details = filePath; + Details["message"] = message; + Details["file-path"] = filePath; } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobErrorBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobErrorBase.cs index 7507ba583d..3b38b43d3c 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobErrorBase.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobErrorBase.cs @@ -13,6 +13,5 @@ public JobErrorBase(string type) public string Type { get; } [JsonPropertyName("error-details")] - [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] - public object? Details { get; init; } = null; + public Dictionary Details { get; init; } = new(); } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobRepoNotFound.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobRepoNotFound.cs index 12d9f229d7..3fd013426b 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobRepoNotFound.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobRepoNotFound.cs @@ -5,9 +5,6 @@ public record JobRepoNotFound : JobErrorBase public JobRepoNotFound(string message) : base("job_repo_not_found") { - Details = new Dictionary() - { - ["message"] = message - }; + Details["message"] = message; } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/PrivateSourceAuthenticationFailure.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/PrivateSourceAuthenticationFailure.cs index 35a39894fd..d8e5aeb49f 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/PrivateSourceAuthenticationFailure.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/PrivateSourceAuthenticationFailure.cs @@ -5,6 +5,6 @@ public record PrivateSourceAuthenticationFailure : JobErrorBase public PrivateSourceAuthenticationFailure(string[] urls) : base("private_source_authentication_failure") { - Details = $"({string.Join("|", urls)})"; + Details["source"] = $"({string.Join("|", urls)})"; } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/UnknownError.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/UnknownError.cs index 90517a8f1e..8773cc0817 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/UnknownError.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/UnknownError.cs @@ -2,9 +2,13 @@ namespace NuGetUpdater.Core.Run.ApiModel; public record UnknownError : JobErrorBase { - public UnknownError(string details) + public UnknownError(Exception ex, string jobId) : base("unknown_error") { - Details = details; + Details["error-class"] = ex.GetType().Name; + Details["error-message"] = ex.Message; + Details["error-backtrace"] = ex.StackTrace ?? ""; + Details["package-manager"] = "nuget"; + Details["job-id"] = jobId; } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/UpdateNotPossible.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/UpdateNotPossible.cs index cdf7854063..6d0808240a 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/UpdateNotPossible.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/UpdateNotPossible.cs @@ -5,6 +5,6 @@ public record UpdateNotPossible : JobErrorBase public UpdateNotPossible(string[] dependencies) : base("update_not_possible") { - Details = dependencies; + Details["dependencies"] = dependencies; } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs index eb7a39cb65..43c82ce7e3 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs @@ -12,6 +12,7 @@ namespace NuGetUpdater.Core.Run; public class RunWorker { + private readonly string _jobId; private readonly IApiHandler _apiHandler; private readonly ILogger _logger; private readonly IDiscoveryWorker _discoveryWorker; @@ -25,8 +26,9 @@ public class RunWorker Converters = { new JsonStringEnumConverter(), new RequirementConverter(), new VersionConverter() }, }; - public RunWorker(IApiHandler apiHandler, IDiscoveryWorker discoverWorker, IAnalyzeWorker analyzeWorker, IUpdaterWorker updateWorker, ILogger logger) + public RunWorker(string jobId, IApiHandler apiHandler, IDiscoveryWorker discoverWorker, IAnalyzeWorker analyzeWorker, IUpdaterWorker updateWorker, ILogger logger) { + _jobId = jobId; _apiHandler = apiHandler; _logger = logger; _discoveryWorker = discoverWorker; @@ -87,9 +89,13 @@ private async Task RunWithErrorHandlingAsync(Job job, DirectoryInfo r { error = new PrivateSourceAuthenticationFailure(lastUsedPackageSourceUrls); } + catch (BadRequirementException ex) + { + error = new BadRequirement(ex.Message); + } catch (MissingFileException ex) { - error = new DependencyFileNotFound(ex.FilePath); + error = new DependencyFileNotFound("file not found", ex.FilePath); } catch (UpdateNotPossibleException ex) { @@ -97,7 +103,7 @@ private async Task RunWithErrorHandlingAsync(Job job, DirectoryInfo r } catch (Exception ex) { - error = new UnknownError(ex.ToString()); + error = new UnknownError(ex, _jobId); } if (error is not null) diff --git a/nuget/lib/dependabot/nuget/native_helpers.rb b/nuget/lib/dependabot/nuget/native_helpers.rb index b40a353e04..2de499c279 100644 --- a/nuget/lib/dependabot/nuget/native_helpers.rb +++ b/nuget/lib/dependabot/nuget/native_helpers.rb @@ -301,6 +301,8 @@ def self.ensure_no_errors(json) raise DependencyFileNotParseable, T.must(T.let(error_details, T.nilable(String))) when "AuthenticationFailure" raise PrivateSourceAuthenticationFailure, T.let(error_details, T.nilable(String)) + when "BadRequirement" + raise BadRequirementError, T.let(error_details, T.nilable(String)) when "MissingFile" raise DependencyFileNotFound, T.let(error_details, T.nilable(String)) when "UpdateNotPossible"