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 ef0799a4e4..0f2429f006 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Analyze.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Analyze.cs @@ -26,6 +26,8 @@ public async Task FindsUpdatedPackageAndReturnsTheCorrectData() await RunAsync(path => [ "analyze", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -146,6 +148,8 @@ public async Task DotNetToolsJsonCanBeAnalyzed() await RunAsync(path => [ "analyze", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -235,6 +239,8 @@ public async Task GlobalJsonCanBeAnalyzed() await RunAsync(path => [ "analyze", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -345,7 +351,7 @@ private static async Task RunAsync( { if (args[i] == "--job-path") { - var experimentsResult = await ExperimentsManager.FromJobFileAsync(args[i + 1]); + var experimentsResult = await ExperimentsManager.FromJobFileAsync("TEST-JOB-ID", 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 2622c56e85..be65bdc4f8 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Discover.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Discover.cs @@ -1,5 +1,6 @@ using System.Text; using System.Text.Json; +using System.Text.Json.Serialization; using NuGetUpdater.Core; using NuGetUpdater.Core.Discover; @@ -25,6 +26,8 @@ public async Task PathWithSpaces(bool useDirectDiscovery) await RunAsync(path => [ "discover", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -83,6 +86,8 @@ public async Task WithSolution(bool useDirectDiscovery) await RunAsync(path => [ "discover", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -178,6 +183,8 @@ public async Task WithProject(bool useDirectDiscovery) await RunAsync(path => [ "discover", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -251,6 +258,8 @@ public async Task WithDirectory(bool useDirectDiscovery) await RunAsync(path => [ "discover", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -321,6 +330,8 @@ public async Task WithDuplicateDependenciesOfDifferentTypes() await RunAsync(path => [ "discover", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -398,6 +409,8 @@ public async Task JobFileParseErrorIsReported_InvalidJson() await RunAsync(path => [ "discover", + "--job-id", + "TEST-JOB-ID", "--job-path", jobFilePath, "--repo-root", @@ -412,8 +425,7 @@ await RunAsync(path => { Path = "/", Projects = [], - ErrorType = ErrorType.Unknown, - ErrorDetailsPattern = "JsonException", + ErrorRegex = "Error deserializing job file contents", } ); } @@ -445,6 +457,8 @@ await File.WriteAllTextAsync(jobFilePath, """ await RunAsync(path => [ "discover", + "--job-id", + "TEST-JOB-ID", "--job-path", jobFilePath, "--repo-root", @@ -459,8 +473,7 @@ await RunAsync(path => { Path = "/", Projects = [], - ErrorType = ErrorType.BadRequirement, - ErrorDetailsPattern = "not a valid requirement", + Error = new Core.Run.ApiModel.BadRequirement("not a valid requirement"), } ); } @@ -497,7 +510,7 @@ private static async Task RunAsync( switch (args[i]) { case "--job-path": - var experimentsResult = await ExperimentsManager.FromJobFileAsync(args[i + 1]); + var experimentsResult = await ExperimentsManager.FromJobFileAsync("TEST-JOB-ID", args[i + 1]); experimentsManager = experimentsResult.ExperimentsManager; break; case "--output": @@ -520,11 +533,38 @@ private static async Task RunAsync( resultPath ??= Path.Join(path, DiscoveryWorker.DiscoveryResultFileName); var resultJson = await File.ReadAllTextAsync(resultPath); - var resultObject = JsonSerializer.Deserialize(resultJson, DiscoveryWorker.SerializerOptions); + var serializerOptions = new JsonSerializerOptions() + { + Converters = { new TestJobErrorBaseConverter() } + }; + foreach (var converter in DiscoveryWorker.SerializerOptions.Converters) + { + serializerOptions.Converters.Add(converter); + } + var resultObject = JsonSerializer.Deserialize(resultJson, serializerOptions); return resultObject!; }); ValidateWorkspaceResult(expectedResult, actualResult, experimentsManager); } + + private class TestJobErrorBaseConverter : JsonConverter + { + public override Core.Run.ApiModel.JobErrorBase? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + var dict = JsonSerializer.Deserialize>(ref reader, options)!; + return dict["error-type"].GetString() switch + { + "illformed_requirement" => new Core.Run.ApiModel.BadRequirement(dict["error-details"].GetProperty("message").GetString()!), + "unknown_error" => new Core.Run.ApiModel.UnknownError(new Exception("Error deserializing job file contents"), "TEST-JOB-ID"), + _ => throw new NotImplementedException($"Unknown error type: {dict["error-type"]}"), + }; + } + + public override void Write(Utf8JsonWriter writer, Core.Run.ApiModel.JobErrorBase value, JsonSerializerOptions options) + { + throw new NotImplementedException(); + } + } } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs index 6981b2d369..a0d9b64fab 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs @@ -19,6 +19,8 @@ public async Task WithSolution() await Run(path => [ "update", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -122,6 +124,8 @@ public async Task WithProject() await Run(path => [ "update", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -202,6 +206,8 @@ public async Task WithDirsProjAndDirectoryBuildPropsThatIsOutOfDirectoryButStill await Run(path => [ "update", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(path, "job.json"), "--repo-root", @@ -361,6 +367,8 @@ await File.WriteAllTextAsync(projectPath, """ IEnumerable executableArgs = [ executableName, "update", + "--job-id", + "TEST-JOB-ID", "--job-path", Path.Combine(tempDir.DirectoryPath, "job.json"), "--repo-root", diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs index 0133b62b87..7c3d6363a6 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs @@ -7,6 +7,7 @@ namespace NuGetUpdater.Cli.Commands; internal static class AnalyzeCommand { + internal static readonly Option JobIdOption = new("--job-id") { IsRequired = true }; internal static readonly Option JobPathOption = new("--job-path") { IsRequired = true }; internal static readonly Option RepoRootOption = new("--repo-root") { IsRequired = true }; internal static readonly Option DependencyFilePathOption = new("--dependency-file-path") { IsRequired = true }; @@ -17,6 +18,7 @@ internal static Command GetCommand(Action setExitCode) { Command command = new("analyze", "Determines how to update a dependency based on the workspace discovery information.") { + JobIdOption, JobPathOption, RepoRootOption, DependencyFilePathOption, @@ -26,13 +28,13 @@ internal static Command GetCommand(Action setExitCode) command.TreatUnmatchedTokensAsErrors = true; - command.SetHandler(async (jobPath, repoRoot, discoveryPath, dependencyPath, analysisDirectory) => + command.SetHandler(async (jobId, jobPath, repoRoot, discoveryPath, dependencyPath, analysisDirectory) => { var logger = new ConsoleLogger(); - var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName); - var worker = new AnalyzeWorker(experimentsManager, logger); + var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName); + var worker = new AnalyzeWorker(jobId, experimentsManager, logger); await worker.RunAsync(repoRoot.FullName, discoveryPath.FullName, dependencyPath.FullName, analysisDirectory.FullName); - }, JobPathOption, RepoRootOption, DiscoveryFilePathOption, DependencyFilePathOption, AnalysisFolderOption); + }, JobIdOption, JobPathOption, RepoRootOption, DiscoveryFilePathOption, DependencyFilePathOption, AnalysisFolderOption); return command; } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs index d3d62b1a1e..d346d90659 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs @@ -7,6 +7,7 @@ namespace NuGetUpdater.Cli.Commands; internal static class DiscoverCommand { + internal static readonly Option JobIdOption = new("--job-id") { IsRequired = true }; internal static readonly Option JobPathOption = new("--job-path") { IsRequired = true }; internal static readonly Option RepoRootOption = new("--repo-root") { IsRequired = true }; internal static readonly Option WorkspaceOption = new("--workspace") { IsRequired = true }; @@ -16,6 +17,7 @@ internal static Command GetCommand(Action setExitCode) { Command command = new("discover", "Generates a report of the workspace dependencies and where they are located.") { + JobIdOption, JobPathOption, RepoRootOption, WorkspaceOption, @@ -24,27 +26,26 @@ internal static Command GetCommand(Action setExitCode) command.TreatUnmatchedTokensAsErrors = true; - command.SetHandler(async (jobPath, repoRoot, workspace, outputPath) => + command.SetHandler(async (jobId, jobPath, repoRoot, workspace, outputPath) => { - var (experimentsManager, errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName); - if (errorResult is not null) + var (experimentsManager, error) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName); + if (error is not null) { // to make testing easier, this should be a `WorkspaceDiscoveryResult` object var discoveryErrorResult = new WorkspaceDiscoveryResult { + Error = error, Path = workspace, Projects = [], - ErrorType = errorResult.ErrorType, - ErrorDetails = errorResult.ErrorDetails, }; await DiscoveryWorker.WriteResultsAsync(repoRoot.FullName, outputPath.FullName, discoveryErrorResult); return; } var logger = new ConsoleLogger(); - var worker = new DiscoveryWorker(experimentsManager, logger); + var worker = new DiscoveryWorker(jobId, experimentsManager, logger); await worker.RunAsync(repoRoot.FullName, workspace, outputPath.FullName); - }, JobPathOption, RepoRootOption, WorkspaceOption, OutputOption); + }, JobIdOption, JobPathOption, RepoRootOption, WorkspaceOption, OutputOption); return command; } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs index 439c2b07d6..fe6a064d9a 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs @@ -33,11 +33,11 @@ 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 (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName); var logger = new ConsoleLogger(); - var discoverWorker = new DiscoveryWorker(experimentsManager, logger); - var analyzeWorker = new AnalyzeWorker(experimentsManager, logger); - var updateWorker = new UpdaterWorker(experimentsManager, logger); + var discoverWorker = new DiscoveryWorker(jobId, experimentsManager, logger); + var analyzeWorker = new AnalyzeWorker(jobId, experimentsManager, logger); + var updateWorker = new UpdaterWorker(jobId, experimentsManager, 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 bd04c652cc..91b920be99 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs @@ -6,6 +6,7 @@ namespace NuGetUpdater.Cli.Commands; internal static class UpdateCommand { + internal static readonly Option JobIdOption = new("--job-id") { IsRequired = true }; internal static readonly Option JobPathOption = new("--job-path") { IsRequired = true }; internal static readonly Option RepoRootOption = new("--repo-root", () => new DirectoryInfo(Environment.CurrentDirectory)) { IsRequired = false }; internal static readonly Option SolutionOrProjectFileOption = new("--solution-or-project") { IsRequired = true }; @@ -19,6 +20,7 @@ internal static Command GetCommand(Action setExitCode) { Command command = new("update", "Applies the changes from an analysis report to update a dependency.") { + JobIdOption, JobPathOption, RepoRootOption, SolutionOrProjectFileOption, @@ -30,15 +32,25 @@ internal static Command GetCommand(Action setExitCode) }; command.TreatUnmatchedTokensAsErrors = true; - - command.SetHandler(async (jobPath, repoRoot, solutionOrProjectFile, dependencyName, newVersion, previousVersion, isTransitive, resultOutputPath) => + command.SetHandler(async (context) => { - var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName); + // since we have more than 8 arguments, we have to pull them out manually + var jobId = context.ParseResult.GetValueForOption(JobIdOption)!; + var jobPath = context.ParseResult.GetValueForOption(JobPathOption)!; + var repoRoot = context.ParseResult.GetValueForOption(RepoRootOption)!; + var solutionOrProjectFile = context.ParseResult.GetValueForOption(SolutionOrProjectFileOption)!; + var dependencyName = context.ParseResult.GetValueForOption(DependencyNameOption)!; + var newVersion = context.ParseResult.GetValueForOption(NewVersionOption)!; + var previousVersion = context.ParseResult.GetValueForOption(PreviousVersionOption)!; + var isTransitive = context.ParseResult.GetValueForOption(IsTransitiveOption); + var resultOutputPath = context.ParseResult.GetValueForOption(ResultOutputPathOption); + + var (experimentsManager, _error) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName); var logger = new ConsoleLogger(); - var worker = new UpdaterWorker(experimentsManager, logger); + var worker = new UpdaterWorker(jobId, experimentsManager, logger); await worker.RunAsync(repoRoot.FullName, solutionOrProjectFile.FullName, dependencyName, previousVersion, newVersion, isTransitive, resultOutputPath); setExitCode(0); - }, JobPathOption, RepoRootOption, SolutionOrProjectFileOption, DependencyNameOption, NewVersionOption, PreviousVersionOption, IsTransitiveOption, ResultOutputPathOption); + }); return command; } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTestBase.cs index ce60bf9eeb..104ac9abf1 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTestBase.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTestBase.cs @@ -12,7 +12,7 @@ namespace NuGetUpdater.Core.Test.Analyze; using TestFile = (string Path, string Content); -public class AnalyzeWorkerTestBase +public class AnalyzeWorkerTestBase : TestBase { protected static async Task TestAnalyzeAsync( WorkspaceDiscoveryResult discovery, @@ -39,7 +39,7 @@ protected static async Task TestAnalyzeAsync( var discoveryPath = Path.GetFullPath(DiscoveryWorker.DiscoveryResultFileName, directoryPath); var dependencyPath = Path.GetFullPath(relativeDependencyPath, directoryPath); - var worker = new AnalyzeWorker(experimentsManager, new TestLogger()); + var worker = new AnalyzeWorker("TEST-JOB-ID", experimentsManager, new TestLogger()); var result = await worker.RunWithErrorHandlingAsync(directoryPath, discoveryPath, dependencyPath); return result; }); @@ -55,8 +55,7 @@ protected static void ValidateAnalysisResult(ExpectedAnalysisResult expectedResu Assert.Equal(expectedResult.VersionComesFromMultiDependencyProperty, actualResult.VersionComesFromMultiDependencyProperty); ValidateDependencies(expectedResult.UpdatedDependencies, actualResult.UpdatedDependencies); Assert.Equal(expectedResult.ExpectedUpdatedDependenciesCount ?? expectedResult.UpdatedDependencies.Length, actualResult.UpdatedDependencies.Length); - Assert.Equal(expectedResult.ErrorType, actualResult.ErrorType); - Assert.Equal(expectedResult.ErrorDetails, actualResult.ErrorDetails); + ValidateResult(expectedResult, actualResult); return; @@ -81,6 +80,18 @@ void ValidateDependencies(ImmutableArray expectedDependencies, Immut } } + protected static void ValidateResult(ExpectedAnalysisResult? expectedResult, AnalysisResult actualResult) + { + if (expectedResult?.Error is not null) + { + ValidateError(expectedResult.Error, actualResult.Error); + } + else + { + Assert.Null(actualResult.Error); + } + } + protected static async Task RunAnalyzerAsync(string dependencyName, TestFile[] files, Func> action) { // write initial files diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTests.cs index 4e93815025..0b546c97c3 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTests.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTests.cs @@ -4,6 +4,7 @@ using NuGet; using NuGetUpdater.Core.Analyze; +using NuGetUpdater.Core.Run.ApiModel; using Xunit; @@ -1015,8 +1016,7 @@ public async Task ResultFileHasCorrectShapeForAuthenticationFailure() using var temporaryDirectory = await TemporaryDirectory.CreateWithContentsAsync([]); await AnalyzeWorker.WriteResultsAsync(temporaryDirectory.DirectoryPath, "Some.Dependency", new() { - ErrorType = ErrorType.AuthenticationFailure, - ErrorDetails = "", + Error = new PrivateSourceAuthenticationFailure([""]), UpdatedVersion = "", UpdatedDependencies = [], }, new TestLogger()); @@ -1025,16 +1025,23 @@ public async Task ResultFileHasCorrectShapeForAuthenticationFailure() // raw result file should look like this: // { // ... - // "ErrorType": "AuthenticationFailure", + // "Error": { + // "error-type": "private_source_authentication_failure", + // "error-details": { + // "source": "()" + // } + // } // "ErrorDetails": "", // ... // } var jsonDocument = JsonDocument.Parse(discoveryContents); - var errorType = jsonDocument.RootElement.GetProperty("ErrorType"); - var errorDetails = jsonDocument.RootElement.GetProperty("ErrorDetails"); + var error = jsonDocument.RootElement.GetProperty("Error"); + var errorType = error.GetProperty("error-type"); + var errorDetails = error.GetProperty("error-details"); + var errorSource = errorDetails.GetProperty("source"); - Assert.Equal("AuthenticationFailure", errorType.GetString()); - Assert.Equal("", errorDetails.GetString()); + Assert.Equal("private_source_authentication_failure", errorType.GetString()); + Assert.Equal("()", errorSource.GetString()); } [Fact] @@ -1110,8 +1117,7 @@ await TestAnalyzeAsync( }, expectedResult: new() { - ErrorType = ErrorType.AuthenticationFailure, - ErrorDetails = $"({http.BaseUrl.TrimEnd('/')}/index.json)", + Error = new PrivateSourceAuthenticationFailure([$"{http.BaseUrl.TrimEnd('/')}/index.json"]), UpdatedVersion = string.Empty, CanUpdate = false, UpdatedDependencies = [], 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 4da255d04f..f9bf1b0b09 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs @@ -5,7 +5,9 @@ using NuGetUpdater.Core.Discover; using NuGetUpdater.Core.Test.Update; +using NuGetUpdater.Core.Test.Updater; using NuGetUpdater.Core.Test.Utilities; +using NuGetUpdater.Core.Updater; using NuGetUpdater.Core.Utilities; using Xunit; @@ -28,7 +30,7 @@ protected static async Task TestDiscoveryAsync( { await UpdateWorkerTestBase.MockNuGetPackagesInDirectory(packages, directoryPath); - var worker = new DiscoveryWorker(experimentsManager, new TestLogger()); + var worker = new DiscoveryWorker("TEST-JOB-ID", experimentsManager, new TestLogger()); var result = await worker.RunWithErrorHandlingAsync(directoryPath, workspacePath); return result; }); @@ -44,17 +46,7 @@ protected static void ValidateWorkspaceResult(ExpectedWorkspaceDiscoveryResult e ValidateResultWithDependencies(expectedResult.DotNetToolsJson, actualResult.DotNetToolsJson); ValidateProjectResults(expectedResult.Projects, actualResult.Projects, experimentsManager); Assert.Equal(expectedResult.ExpectedProjectCount ?? expectedResult.Projects.Length, actualResult.Projects.Length); - Assert.Equal(expectedResult.ErrorType, actualResult.ErrorType); - 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); - } + ValidateDiscoveryOperationResult(expectedResult, actualResult); return; @@ -76,6 +68,22 @@ static void ValidateResultWithDependencies(ExpectedDependencyDiscoveryResult? ex } } + protected static void ValidateDiscoveryOperationResult(ExpectedWorkspaceDiscoveryResult? expectedResult, WorkspaceDiscoveryResult actualResult) + { + if (expectedResult?.Error is not null) + { + ValidateError(expectedResult.Error, actualResult.Error); + } + else if (expectedResult?.ErrorRegex is not null) + { + ValidateErrorRegex(expectedResult.ErrorRegex, actualResult.Error); + } + else + { + Assert.Null(actualResult.Error); + } + } + internal static void ValidateProjectResults(ImmutableArray expectedProjects, ImmutableArray actualProjects, ExperimentsManager experimentsManager) { if (expectedProjects.IsDefaultOrEmpty) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.cs index 066595385c..9f12965692 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.cs @@ -3,6 +3,7 @@ using System.Text.Json; using NuGetUpdater.Core.Discover; +using NuGetUpdater.Core.Run.ApiModel; using Xunit; @@ -1130,8 +1131,7 @@ await TestDiscoveryAsync( { Path = "", Projects = [], - ErrorType = ErrorType.DependencyFileNotParseable, - ErrorDetails = "project2.csproj", + Error = new DependencyFileNotParseable("project2.csproj"), }); } @@ -1142,8 +1142,7 @@ public async Task ResultFileHasCorrectShapeForAuthenticationFailure() var discoveryResultPath = Path.Combine(temporaryDirectory.DirectoryPath, DiscoveryWorker.DiscoveryResultFileName); await DiscoveryWorker.WriteResultsAsync(temporaryDirectory.DirectoryPath, discoveryResultPath, new() { - ErrorType = ErrorType.AuthenticationFailure, - ErrorDetails = "", + Error = new PrivateSourceAuthenticationFailure([""]), Path = "/", Projects = [], }); @@ -1152,16 +1151,22 @@ public async Task ResultFileHasCorrectShapeForAuthenticationFailure() // raw result file should look like this: // { // ... - // "ErrorType": "AuthenticationFailure", - // "ErrorDetails": "", + // "Error": { + // "error-type": "private_source_authentication_failure", + // "error-detail": { + // "source": "()" + // } + // } // ... // } var jsonDocument = JsonDocument.Parse(discoveryContents); - var errorType = jsonDocument.RootElement.GetProperty("ErrorType"); - var errorDetails = jsonDocument.RootElement.GetProperty("ErrorDetails"); + var error = jsonDocument.RootElement.GetProperty("Error"); + var errorType = error.GetProperty("error-type"); + var errorDetail = error.GetProperty("error-details"); + var errorSource = errorDetail.GetProperty("source"); - Assert.Equal("AuthenticationFailure", errorType.GetString()); - Assert.Equal("", errorDetails.GetString()); + Assert.Equal("private_source_authentication_failure", errorType.GetString()); + Assert.Equal("()", errorSource.GetString()); } [Theory] @@ -1236,8 +1241,7 @@ await TestDiscoveryAsync( ], expectedResult: new() { - ErrorType = ErrorType.AuthenticationFailure, - ErrorDetails = $"({http.BaseUrl.TrimEnd('/')}/index.json)", + Error = new PrivateSourceAuthenticationFailure([$"{http.BaseUrl.TrimEnd('/')}/index.json"]), Path = "", Projects = [], } 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 d9d434aad6..3d6da437bc 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/ExpectedDiscoveryResults.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/ExpectedDiscoveryResults.cs @@ -12,7 +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 string? ErrorRegex { 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 c58a29fcd7..cef94b63b6 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/RunWorkerTests.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/RunWorkerTests.cs @@ -1729,11 +1729,12 @@ private static async Task RunAsync(Job job, TestFile[] files, IDiscoveryWorker? experimentsManager ??= new ExperimentsManager(); var testApiHandler = new TestApiHandler(); var logger = new TestLogger(); - discoveryWorker ??= new DiscoveryWorker(experimentsManager, logger); - analyzeWorker ??= new AnalyzeWorker(experimentsManager, logger); - updaterWorker ??= new UpdaterWorker(experimentsManager, logger); + var jobId = "TEST-JOB-ID"; + discoveryWorker ??= new DiscoveryWorker(jobId, experimentsManager, logger); + analyzeWorker ??= new AnalyzeWorker(jobId, experimentsManager, logger); + updaterWorker ??= new UpdaterWorker(jobId, experimentsManager, logger); - var worker = new RunWorker("TEST-JOB-ID", testApiHandler, discoveryWorker, analyzeWorker, updaterWorker, logger); + var worker = new RunWorker(jobId, 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 68264c1a2f..f9541cd6b7 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/SerializationTests.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/SerializationTests.cs @@ -523,12 +523,20 @@ public void DeserializeCommitOptions() yield return [ - new DependencyFileNotFound("some message", "/some/file"), + new DependencyFileNotFound("/some/file", "some message"), """ {"data":{"error-type":"dependency_file_not_found","error-details":{"message":"some message","file-path":"/some/file"}}} """ ]; + yield return + [ + new DependencyFileNotParseable("/some/file", "some message"), + """ + {"data":{"error-type":"dependency_file_not_parseable","error-details":{"message":"some message","file-path":"/some/file"}}} + """ + ]; + yield return [ new JobRepoNotFound("some message"), diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/TestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/TestBase.cs index f427981ec2..711c6943c3 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/TestBase.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/TestBase.cs @@ -1,3 +1,10 @@ +using System.Text.Json; + +using NuGetUpdater.Core.Run.ApiModel; +using NuGetUpdater.Core.Run; + +using Xunit; + namespace NuGetUpdater.Core.Test { public abstract class TestBase @@ -6,5 +13,22 @@ protected TestBase() { MSBuildHelper.RegisterMSBuild(Environment.CurrentDirectory, Environment.CurrentDirectory); } + + protected static void ValidateError(JobErrorBase expected, JobErrorBase? actual) + { + var expectedErrorString = JsonSerializer.Serialize(expected, RunWorker.SerializerOptions); + var actualErrorString = actual is null + ? null + : JsonSerializer.Serialize(actual, RunWorker.SerializerOptions); + Assert.Equal(expectedErrorString, actualErrorString); + } + + protected static void ValidateErrorRegex(string expectedErrorRegex, JobErrorBase? actual) + { + var actualErrorString = actual is null + ? null + : JsonSerializer.Serialize(actual, RunWorker.SerializerOptions); + Assert.Matches(expectedErrorRegex, actualErrorString); + } } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/ExpectedUpdateOperationResult.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/ExpectedUpdateOperationResult.cs index 6f4b9d4089..dba5d4436a 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/ExpectedUpdateOperationResult.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/ExpectedUpdateOperationResult.cs @@ -4,5 +4,5 @@ namespace NuGetUpdater.Core.Test.Updater; public record ExpectedUpdateOperationResult : UpdateOperationResult { - public string? ErrorDetailsRegex { get; init; } = null; + public string? ErrorRegex { get; init; } = null; } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs index 2b6d15ac18..e23cbf8bef 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs @@ -152,17 +152,10 @@ protected static async Task TestUpdateForProject( // run update experimentsManager ??= new ExperimentsManager(); - var worker = new UpdaterWorker(experimentsManager, new TestLogger()); + var worker = new UpdaterWorker("TEST-JOB-ID", experimentsManager, new TestLogger()); var projectPath = placeFilesInSrc ? $"src/{projectFilePath}" : projectFilePath; var actualResult = await worker.RunWithErrorHandlingAsync(temporaryDirectory, projectPath, dependencyName, oldVersion, newVersion, isTransitive); - if (expectedResult is { }) - { - ValidateUpdateOperationResult(expectedResult, actualResult!); - } - else if ((actualResult.ErrorType ?? ErrorType.None) != ErrorType.None) - { - throw new Exception($"Result indicates failure: ErrorType={actualResult.ErrorType}, ErrorDetails={actualResult.ErrorDetails}"); - } + ValidateUpdateOperationResult(expectedResult, actualResult!); if (additionalChecks is not null) { @@ -185,16 +178,19 @@ protected static async Task TestUpdateForProject( AssertContainsFiles(expectedResultFiles, actualResult); } - protected static void ValidateUpdateOperationResult(ExpectedUpdateOperationResult expectedResult, UpdateOperationResult actualResult) + protected static void ValidateUpdateOperationResult(ExpectedUpdateOperationResult? expectedResult, UpdateOperationResult actualResult) { - Assert.Equal(expectedResult.ErrorType, actualResult.ErrorType); - if (expectedResult.ErrorDetailsRegex is not null && actualResult.ErrorDetails is string errorDetails) + if (expectedResult?.Error is not null) + { + ValidateError(expectedResult.Error, actualResult.Error); + } + else if (expectedResult?.ErrorRegex is not null) { - Assert.Matches(expectedResult.ErrorDetailsRegex, errorDetails); + ValidateErrorRegex(expectedResult.ErrorRegex, actualResult.Error); } else { - Assert.Equivalent(expectedResult.ErrorDetails, actualResult.ErrorDetails); + Assert.Null(actualResult.Error); } } @@ -274,7 +270,7 @@ protected static async Task TestUpdateForSolution( experimentsManager ??= new ExperimentsManager(); var slnPath = Path.Combine(temporaryDirectory, slnName); - var worker = new UpdaterWorker(experimentsManager, new TestLogger()); + var worker = new UpdaterWorker("TEST-JOB-ID", experimentsManager, new TestLogger()); await worker.RunAsync(temporaryDirectory, slnPath, dependencyName, oldVersion, newVersion, isTransitive); }); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DirsProj.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DirsProj.cs index 82cd79478c..4c1bf4d7b8 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DirsProj.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DirsProj.cs @@ -366,7 +366,7 @@ static async Task TestUpdateForDirsProj( experimentsManager ??= new ExperimentsManager(); var projectPath = Path.Combine(temporaryDirectory, projectFileName); - var worker = new UpdaterWorker(experimentsManager, new TestLogger()); + var worker = new UpdaterWorker("TEST-JOB-ID", experimentsManager, new TestLogger()); await worker.RunAsync(temporaryDirectory, projectPath, dependencyName, oldVersion, newVersion, isTransitive); }); diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.Mixed.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.Mixed.cs index 5f3dc797f1..b7145c355e 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.Mixed.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.Mixed.cs @@ -1,5 +1,6 @@ using System.Text.Json; +using NuGetUpdater.Core.Run.ApiModel; using NuGetUpdater.Core.Updater; using Xunit; @@ -16,8 +17,7 @@ public async Task ResultFileHasCorrectShapeForAuthenticationFailure() using var temporaryDirectory = await TemporaryDirectory.CreateWithContentsAsync([]); var result = new UpdateOperationResult() { - ErrorType = ErrorType.AuthenticationFailure, - ErrorDetails = "", + Error = new PrivateSourceAuthenticationFailure([""]), }; var resultFilePath = Path.Combine(temporaryDirectory.DirectoryPath, "update-result.json"); await UpdaterWorker.WriteResultFile(result, resultFilePath, new TestLogger()); @@ -26,16 +26,22 @@ public async Task ResultFileHasCorrectShapeForAuthenticationFailure() // raw result file should look like this: // { // ... - // "ErrorType": "AuthenticationFailure", - // "ErrorDetails": "", + // "Error": { + // "error-type": "private_source_authentication_failure", + // "error-details": { + // "source": "" + // } + // } // ... // } var jsonDocument = JsonDocument.Parse(resultContent); - var errorType = jsonDocument.RootElement.GetProperty("ErrorType"); - var errorDetails = jsonDocument.RootElement.GetProperty("ErrorDetails"); + var error = jsonDocument.RootElement.GetProperty("Error"); + var errorType = error.GetProperty("error-type"); + var errorDetails = error.GetProperty("error-details"); + var source = errorDetails.GetProperty("source"); - Assert.Equal("AuthenticationFailure", errorType.GetString()); - Assert.Equal("", errorDetails.GetString()); + Assert.Equal("private_source_authentication_failure", errorType.GetString()); + Assert.Equal("()", source.GetString()); } [Fact] diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackageReference.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackageReference.cs index f4b17749d3..5269ae61fb 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackageReference.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackageReference.cs @@ -2,6 +2,7 @@ using System.Text; using System.Text.Json; +using NuGetUpdater.Core.Run.ApiModel; using NuGetUpdater.Core.Updater; using Xunit; @@ -571,7 +572,7 @@ await File.WriteAllTextAsync(Path.Combine(srcDirectory, "NuGet.Config"), $""" // // do the update // - UpdaterWorker worker = new(new ExperimentsManager(), new TestLogger()); + UpdaterWorker worker = new("TEST-JOB-ID", new ExperimentsManager(), new TestLogger()); await worker.RunAsync(tempDirectory.DirectoryPath, projectPath, "Some.Package", "1.0.0", "1.1.0", isTransitive: false); // @@ -3482,8 +3483,7 @@ await TestUpdateForProject("Some.Package", "1.0.0", "1.1.0", """, expectedResult: new() { - ErrorType = ErrorType.AuthenticationFailure, - ErrorDetails = $"({http.BaseUrl.TrimEnd('/')}/index.json)", + Error = new PrivateSourceAuthenticationFailure([$"{http.BaseUrl.TrimEnd('/')}/index.json"]), } ); } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs index fa04a35238..a220963014 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs @@ -4,6 +4,7 @@ using NuGet; +using NuGetUpdater.Core.Run.ApiModel; using NuGetUpdater.Core.Test.Updater; using NuGetUpdater.Core.Updater; @@ -2282,13 +2283,13 @@ public async Task MissingTargetsAreReported() await MockNuGetPackagesInDirectory(packages, Path.Combine(temporaryDirectory.DirectoryPath, "packages")); var resultOutputPath = Path.Combine(temporaryDirectory.DirectoryPath, "result.json"); - var worker = new UpdaterWorker(new ExperimentsManager(), new TestLogger()); + var worker = new UpdaterWorker("TEST-JOB-ID", new ExperimentsManager(), new TestLogger()); await worker.RunAsync(temporaryDirectory.DirectoryPath, "project.csproj", "Some.Package", "1.0.0", "1.1.0", isTransitive: false, resultOutputPath: resultOutputPath); var resultContents = await File.ReadAllTextAsync(resultOutputPath); - var result = JsonSerializer.Deserialize(resultContents, UpdaterWorker.SerializerOptions)!; - Assert.Equal(ErrorType.MissingFile, result.ErrorType); - Assert.Equal(Path.Combine(temporaryDirectory.DirectoryPath, "this.file.does.not.exist.targets"), result.ErrorDetails!.ToString()); + var rawResult = JsonDocument.Parse(resultContents); + Assert.Equal("dependency_file_not_found", rawResult.RootElement.GetProperty("Error").GetProperty("error-type").GetString()); + Assert.Equal(Path.Combine(temporaryDirectory.DirectoryPath, "this.file.does.not.exist.targets").NormalizePathToUnix(), rawResult.RootElement.GetProperty("Error").GetProperty("error-details").GetProperty("file-path").GetString()); } [Fact] @@ -2338,13 +2339,13 @@ public async Task MissingVisualStudioComponentTargetsAreReportedAsMissingFiles() await MockNuGetPackagesInDirectory(packages, Path.Combine(temporaryDirectory.DirectoryPath, "packages")); var resultOutputPath = Path.Combine(temporaryDirectory.DirectoryPath, "result.json"); - var worker = new UpdaterWorker(new ExperimentsManager(), new TestLogger()); + var worker = new UpdaterWorker("TEST-JOB-ID", new ExperimentsManager(), new TestLogger()); await worker.RunAsync(temporaryDirectory.DirectoryPath, "project.csproj", "Some.Package", "1.0.0", "1.1.0", isTransitive: false, resultOutputPath: resultOutputPath); var resultContents = await File.ReadAllTextAsync(resultOutputPath); - var result = JsonSerializer.Deserialize(resultContents, UpdaterWorker.SerializerOptions)!; - Assert.Equal(ErrorType.MissingFile, result.ErrorType); - Assert.Equal("$(MSBuildExtensionsPath32)/Microsoft/VisualStudio/v$(VisualStudioVersion)/Some.Visual.Studio.Component.props", result.ErrorDetails!.ToString().NormalizePathToUnix()); + var rawResult = JsonDocument.Parse(resultContents); + Assert.Equal("dependency_file_not_found", rawResult.RootElement.GetProperty("Error").GetProperty("error-type").GetString()); + Assert.Equal("$(MSBuildExtensionsPath32)/Microsoft/VisualStudio/v$(VisualStudioVersion)/Some.Visual.Studio.Component.props", rawResult.RootElement.GetProperty("Error").GetProperty("error-details").GetProperty("file-path").GetString()); } [Theory] @@ -2424,8 +2425,7 @@ await TestUpdateForProject("Some.Package", "1.0.0", "1.1.0", """, expectedResult: new() { - ErrorType = ErrorType.AuthenticationFailure, - ErrorDetails = $"({http.BaseUrl.TrimEnd('/')}/index.json)", + Error = new PrivateSourceAuthenticationFailure([$"{http.BaseUrl.TrimEnd('/')}/index.json"]), } ); } @@ -2555,8 +2555,7 @@ await TestUpdateForProject("Some.Package", "1.0.0", "1.1.0", """, expectedResult: new() { - ErrorType = ErrorType.Unknown, - ErrorDetailsRegex = "Response status code does not indicate success", + ErrorRegex = "Response status code does not indicate success", } ); } @@ -2633,8 +2632,7 @@ await TestUpdateForProject("Some.Package", "1.0.1", "1.0.2", """, expectedResult: new() { - ErrorType = ErrorType.UpdateNotPossible, - ErrorDetails = new[] { "Unrelated.Package.1.0.0" }, + Error = new UpdateNotPossible(["Unrelated.Package.1.0.0"]), } ); } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/AnalyzeWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/AnalyzeWorker.cs index eb0b8fcd86..c4c9163dd8 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/AnalyzeWorker.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/AnalyzeWorker.cs @@ -7,6 +7,7 @@ using NuGet.Versioning; using NuGetUpdater.Core.Discover; +using NuGetUpdater.Core.Run.ApiModel; namespace NuGetUpdater.Core.Analyze; @@ -16,6 +17,7 @@ public partial class AnalyzeWorker : IAnalyzeWorker { public const string AnalysisDirectoryName = "./.dependabot/analysis"; + private readonly string _jobId; private readonly ExperimentsManager _experimentsManager; private readonly ILogger _logger; @@ -25,8 +27,9 @@ public partial class AnalyzeWorker : IAnalyzeWorker Converters = { new JsonStringEnumConverter(), new RequirementArrayConverter() }, }; - public AnalyzeWorker(ExperimentsManager experimentsManager, ILogger logger) + public AnalyzeWorker(string jobId, ExperimentsManager experimentsManager, ILogger logger) { + _jobId = jobId; _experimentsManager = experimentsManager; _logger = logger; } @@ -48,26 +51,11 @@ internal async Task RunWithErrorHandlingAsync(string repoRoot, s { analysisResult = await RunAsync(repoRoot, discovery, dependencyInfo); } - catch (HttpRequestException ex) - when (ex.StatusCode == HttpStatusCode.Unauthorized || ex.StatusCode == HttpStatusCode.Forbidden) - { - var localPath = PathHelper.JoinPath(repoRoot, discovery.Path); - using var nugetContext = new NuGetContext(localPath); - analysisResult = new AnalysisResult - { - ErrorType = ErrorType.AuthenticationFailure, - ErrorDetails = "(" + string.Join("|", nugetContext.PackageSources.Select(s => s.Source)) + ")", - UpdatedVersion = string.Empty, - CanUpdate = false, - UpdatedDependencies = [], - }; - } catch (Exception ex) { analysisResult = new AnalysisResult { - ErrorType = ErrorType.Unknown, - ErrorDetails = ex.ToString(), + Error = JobErrorBase.ErrorFromException(ex, _jobId, PathHelper.JoinPath(repoRoot, discovery.Path)), UpdatedVersion = string.Empty, CanUpdate = false, UpdatedDependencies = [], diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Clone/CloneWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Clone/CloneWorker.cs index 2e4d379195..f9dfc6b6e8 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Clone/CloneWorker.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Clone/CloneWorker.cs @@ -70,11 +70,12 @@ public async Task RunAsync(Job job, string repoContentsPath) catch (HttpRequestException ex) when (ex.StatusCode == HttpStatusCode.Unauthorized || ex.StatusCode == HttpStatusCode.Forbidden) { + // this is a _very_ specific case we want to handle before the common error handling kicks in error = new JobRepoNotFound(ex.Message); } catch (Exception ex) { - error = new UnknownError(ex, _jobId); + error = JobErrorBase.ErrorFromException(ex, _jobId, repoContentsPath); } if (error is not null) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs index 3253517c9c..951179f724 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs @@ -1,5 +1,4 @@ using System.Collections.Immutable; -using System.Net; using System.Text.Json; using System.Text.Json.Serialization; @@ -10,7 +9,7 @@ using NuGet.Frameworks; -using NuGetUpdater.Core.Analyze; +using NuGetUpdater.Core.Run.ApiModel; using NuGetUpdater.Core.Utilities; namespace NuGetUpdater.Core.Discover; @@ -19,6 +18,7 @@ public partial class DiscoveryWorker : IDiscoveryWorker { public const string DiscoveryResultFileName = "./.dependabot/discovery.json"; + private readonly string _jobId; private readonly ExperimentsManager _experimentsManager; private readonly ILogger _logger; private readonly HashSet _processedProjectPaths = new(StringComparer.Ordinal); private readonly HashSet _restoredMSBuildSdks = new(StringComparer.OrdinalIgnoreCase); @@ -29,8 +29,9 @@ public partial class DiscoveryWorker : IDiscoveryWorker Converters = { new JsonStringEnumConverter() }, }; - public DiscoveryWorker(ExperimentsManager experimentsManager, ILogger logger) + public DiscoveryWorker(string jobId, ExperimentsManager experimentsManager, ILogger logger) { + _jobId = jobId; _experimentsManager = experimentsManager; _logger = logger; } @@ -48,23 +49,11 @@ internal async Task RunWithErrorHandlingAsync(string r { result = await RunAsync(repoRootPath, workspacePath); } - catch (HttpRequestException ex) - when (ex.StatusCode == HttpStatusCode.Unauthorized || ex.StatusCode == HttpStatusCode.Forbidden) - { - result = new WorkspaceDiscoveryResult - { - ErrorType = ErrorType.AuthenticationFailure, - ErrorDetails = "(" + string.Join("|", NuGetContext.GetPackageSourceUrls(PathHelper.JoinPath(repoRootPath, workspacePath))) + ")", - Path = workspacePath, - Projects = [], - }; - } catch (Exception ex) { result = new WorkspaceDiscoveryResult { - ErrorType = ErrorType.Unknown, - ErrorDetails = ex.ToString(), + Error = JobErrorBase.ErrorFromException(ex, _jobId, PathHelper.JoinPath(repoRootPath, workspacePath)), Path = workspacePath, Projects = [], }; @@ -123,8 +112,7 @@ public async Task RunAsync(string repoRootPath, string DotNetToolsJson = null, GlobalJson = null, Projects = projectResults.Where(p => p.IsSuccess).OrderBy(p => p.FilePath).ToImmutableArray(), - ErrorType = failedProjectResult.ErrorType, - ErrorDetails = failedProjectResult.FilePath, + Error = failedProjectResult.Error, IsSuccess = false, }; @@ -192,8 +180,7 @@ private async Task> RunForDirectoryAsync( ImportedFiles = ImmutableArray.Empty, AdditionalFiles = ImmutableArray.Empty, IsSuccess = false, - ErrorType = ErrorType.DependencyFileNotParseable, - ErrorDetails = "Failed to parse project file found at " + invalidProjectFile, + Error = new DependencyFileNotParseable(invalidProjectFile), }]; } if (projects.IsEmpty) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/ProjectDiscoveryResult.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/ProjectDiscoveryResult.cs index 2781239e9f..1011f9b122 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/ProjectDiscoveryResult.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/ProjectDiscoveryResult.cs @@ -1,5 +1,6 @@ using System.Collections.Immutable; -using System.Text.Json.Serialization; + +using NuGetUpdater.Core.Run.ApiModel; namespace NuGetUpdater.Core.Discover; @@ -8,8 +9,7 @@ public record ProjectDiscoveryResult : IDiscoveryResultWithDependencies public required string FilePath { get; init; } public required ImmutableArray Dependencies { get; init; } public bool IsSuccess { get; init; } = true; - public string? ErrorDetails { get; init; } - public ErrorType? ErrorType { get; init; } + public JobErrorBase? Error { get; init; } = null; public ImmutableArray Properties { get; init; } = []; public ImmutableArray TargetFrameworks { get; init; } = []; public ImmutableArray ReferencedProjectPaths { get; init; } = []; diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ErrorType.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ErrorType.cs deleted file mode 100644 index 23dab4bb8d..0000000000 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ErrorType.cs +++ /dev/null @@ -1,12 +0,0 @@ -namespace NuGetUpdater.Core; - -public enum ErrorType -{ - None, - AuthenticationFailure, - BadRequirement, - MissingFile, - UpdateNotPossible, - DependencyFileNotParseable, - Unknown, -} diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs index 96a00680b7..e517b4dbe7 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs @@ -1,6 +1,7 @@ using System.Text.Json; using NuGetUpdater.Core.Run; +using NuGetUpdater.Core.Run.ApiModel; namespace NuGetUpdater.Core; @@ -30,42 +31,28 @@ public static ExperimentsManager GetExperimentsManager(Dictionary FromJobFileAsync(string jobFilePath) + public static async Task<(ExperimentsManager ExperimentsManager, JobErrorBase? Error)> FromJobFileAsync(string jobId, string jobFilePath) { var experimentsManager = new ExperimentsManager(); - NativeResult? errorResult = null; + JobErrorBase? error = null; + var jobFileContent = string.Empty; try { - var jobFileContent = await File.ReadAllTextAsync(jobFilePath); + jobFileContent = await File.ReadAllTextAsync(jobFilePath); var jobWrapper = RunWorker.Deserialize(jobFileContent); experimentsManager = GetExperimentsManager(jobWrapper.Job.Experiments); } - catch (BadRequirementException ex) - { - errorResult = new NativeResult - { - ErrorType = ErrorType.BadRequirement, - ErrorDetails = ex.Message, - }; - } catch (JsonException ex) { - errorResult = new NativeResult - { - ErrorType = ErrorType.Unknown, - ErrorDetails = $"Error deserializing job file: {ex}: {File.ReadAllText(jobFilePath)}", - }; + // this is a very specific case where we want to log the JSON contents for easier debugging + error = JobErrorBase.ErrorFromException(new NotSupportedException($"Error deserializing job file contents: {jobFileContent}", ex), jobId, Environment.CurrentDirectory); // TODO } catch (Exception ex) { - errorResult = new NativeResult - { - ErrorType = ErrorType.Unknown, - ErrorDetails = ex.ToString(), - }; + error = JobErrorBase.ErrorFromException(ex, jobId, Environment.CurrentDirectory); // TODO } - return (experimentsManager, errorResult); + return (experimentsManager, error); } private static bool IsEnabled(Dictionary? experiments, string experimentName) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Files/PackagesConfigBuildFile.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Files/PackagesConfigBuildFile.cs index dadca2d58b..1432687b39 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Files/PackagesConfigBuildFile.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Files/PackagesConfigBuildFile.cs @@ -13,12 +13,16 @@ public static PackagesConfigBuildFile Parse(string basePath, string path, string public PackagesConfigBuildFile(string basePath, string path, XmlDocumentSyntax contents) : base(basePath, path, contents) { + var invalidPackageElements = Packages.Where(p => p.GetAttribute("id") is null || p.GetAttribute("version") is null).ToList(); + if (invalidPackageElements.Any()) + { + throw new UnparseableFileException("`package` element missing `id` or `version` attributes", path); + } } public IEnumerable Packages => Contents.RootSyntax.GetElements("package", StringComparison.OrdinalIgnoreCase); public IEnumerable GetDependencies() => Packages - .Where(p => p.GetAttribute("id") is not null && p.GetAttribute("version") is not null) .Select(p => new Dependency( p.GetAttributeValue("id", StringComparison.OrdinalIgnoreCase), p.GetAttributeValue("version", StringComparison.OrdinalIgnoreCase), diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/MissingFileException.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/MissingFileException.cs index 6d92ebed60..fad1ede35f 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/MissingFileException.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/MissingFileException.cs @@ -4,7 +4,8 @@ internal class MissingFileException : Exception { public string FilePath { get; } - public MissingFileException(string filePath) + public MissingFileException(string filePath, string? message = null) + : base(message) { FilePath = filePath; } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NativeResult.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NativeResult.cs index 1ebc767ca0..be87915be2 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NativeResult.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NativeResult.cs @@ -1,8 +1,8 @@ +using NuGetUpdater.Core.Run.ApiModel; + namespace NuGetUpdater.Core; public record NativeResult { - // TODO: nullable not required, `ErrorType.None` is the default anyway - public ErrorType? ErrorType { get; init; } - public object? ErrorDetails { get; init; } + public JobErrorBase? Error { get; init; } = null; } 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 91d385da47..da3d722002 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/DependencyFileNotFound.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/DependencyFileNotFound.cs @@ -2,10 +2,14 @@ namespace NuGetUpdater.Core.Run.ApiModel; public record DependencyFileNotFound : JobErrorBase { - public DependencyFileNotFound(string message, string filePath) + public DependencyFileNotFound(string filePath, string? message = null) : base("dependency_file_not_found") { - Details["message"] = message; - Details["file-path"] = filePath; + if (message is not null) + { + Details["message"] = message; + } + + Details["file-path"] = filePath.NormalizePathToUnix(); } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/DependencyFileNotParseable.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/DependencyFileNotParseable.cs new file mode 100644 index 0000000000..0efc03b0b1 --- /dev/null +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/DependencyFileNotParseable.cs @@ -0,0 +1,15 @@ +namespace NuGetUpdater.Core.Run.ApiModel; + +public record DependencyFileNotParseable : JobErrorBase +{ + public DependencyFileNotParseable(string filePath, string? message = null) + : base("dependency_file_not_parseable") + { + if (message is not null) + { + Details["message"] = message; + } + + Details["file-path"] = filePath.NormalizePathToUnix(); + } +} 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 3b38b43d3c..1522f2d42d 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobErrorBase.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobErrorBase.cs @@ -1,5 +1,10 @@ +using System.Net; using System.Text.Json.Serialization; +using Microsoft.Build.Exceptions; + +using NuGetUpdater.Core.Analyze; + namespace NuGetUpdater.Core.Run.ApiModel; public abstract record JobErrorBase @@ -14,4 +19,23 @@ public JobErrorBase(string type) [JsonPropertyName("error-details")] public Dictionary Details { get; init; } = new(); + + public static JobErrorBase ErrorFromException(Exception ex, string jobId, string currentDirectory) + { + return ex switch + { + BadRequirementException badRequirement => new BadRequirement(badRequirement.Message), + HttpRequestException httpRequest => httpRequest.StatusCode switch + { + HttpStatusCode.Unauthorized or + HttpStatusCode.Forbidden => new PrivateSourceAuthenticationFailure(NuGetContext.GetPackageSourceUrls(currentDirectory)), + _ => new UnknownError(ex, jobId), + }, + InvalidProjectFileException invalidProjectFile => new DependencyFileNotParseable(invalidProjectFile.ProjectFile), + MissingFileException missingFile => new DependencyFileNotFound(missingFile.FilePath, missingFile.Message), + UnparseableFileException unparseableFile => new DependencyFileNotParseable(unparseableFile.FilePath, unparseableFile.Message), + UpdateNotPossibleException updateNotPossible => new UpdateNotPossible(updateNotPossible.Dependencies), + _ => new UnknownError(ex, jobId), + }; + } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs index 43c82ce7e3..7bce84a7e6 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs @@ -53,7 +53,7 @@ public Task RunAsync(Job job, DirectoryInfo repoContentsPath, string private async Task RunWithErrorHandlingAsync(Job job, DirectoryInfo repoContentsPath, string baseCommitSha) { JobErrorBase? error = null; - string[] lastUsedPackageSourceUrls = []; // used for error reporting below + var currentDirectory = repoContentsPath.FullName; // used for error reporting below var runResult = new RunResult() { Base64DependencyFiles = [], @@ -69,7 +69,7 @@ private async Task RunWithErrorHandlingAsync(Job job, DirectoryInfo r foreach (var directory in job.GetAllDirectories()) { var localPath = PathHelper.JoinPath(repoContentsPath.FullName, directory); - lastUsedPackageSourceUrls = NuGetContext.GetPackageSourceUrls(localPath); + currentDirectory = localPath; var result = await RunForDirectory(job, repoContentsPath, directory, baseCommitSha, experimentsManager); foreach (var dependencyFile in result.Base64DependencyFiles) { @@ -84,26 +84,9 @@ private async Task RunWithErrorHandlingAsync(Job job, DirectoryInfo r BaseCommitSha = baseCommitSha, }; } - catch (HttpRequestException ex) - when (ex.StatusCode == HttpStatusCode.Unauthorized || ex.StatusCode == HttpStatusCode.Forbidden) - { - error = new PrivateSourceAuthenticationFailure(lastUsedPackageSourceUrls); - } - catch (BadRequirementException ex) - { - error = new BadRequirement(ex.Message); - } - catch (MissingFileException ex) - { - error = new DependencyFileNotFound("file not found", ex.FilePath); - } - catch (UpdateNotPossibleException ex) - { - error = new UpdateNotPossible(ex.Dependencies); - } catch (Exception ex) { - error = new UnknownError(ex, _jobId); + error = JobErrorBase.ErrorFromException(ex, _jobId, currentDirectory); } if (error is not null) @@ -123,6 +106,8 @@ private async Task RunForDirectory(Job job, DirectoryInfo repoContent _logger.Info("Discovery JSON content:"); _logger.Info(JsonSerializer.Serialize(discoveryResult, DiscoveryWorker.SerializerOptions)); + // TODO: report errors + // report dependencies var discoveredUpdatedDependencies = GetUpdatedDependencyListFromDiscovery(discoveryResult, repoContentsPath.FullName); await _apiHandler.UpdateDependencyList(discoveredUpdatedDependencies); @@ -221,7 +206,7 @@ async Task TrackOriginalContentsAsync(string directory, string fileName) var dependencyFilePath = Path.Join(discoveryResult.Path, project.FilePath).FullyNormalizedRootedPath(); var updateResult = await _updaterWorker.RunAsync(repoContentsPath.FullName, dependencyFilePath, dependency.Name, dependency.Version!, analysisResult.UpdatedVersion, isTransitive: false); // TODO: need to report if anything was actually updated - if (updateResult.ErrorType is null || updateResult.ErrorType == ErrorType.None) + if (updateResult.Error is null) { if (dependencyLocation != dependencyFilePath) { diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/UnparseableFileException.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/UnparseableFileException.cs new file mode 100644 index 0000000000..823cd321af --- /dev/null +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/UnparseableFileException.cs @@ -0,0 +1,12 @@ +namespace NuGetUpdater.Core; + +internal class UnparseableFileException : Exception +{ + public string FilePath { get; } + + public UnparseableFileException(string message, string filePath) + : base(message) + { + FilePath = filePath; + } +} diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs index f6aaae756c..efb2fc31cf 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs @@ -3,6 +3,7 @@ using System.Text.Json.Serialization; using NuGetUpdater.Core.Analyze; +using NuGetUpdater.Core.Run.ApiModel; using NuGetUpdater.Core.Updater; using NuGetUpdater.Core.Utilities; @@ -10,6 +11,7 @@ namespace NuGetUpdater.Core; public class UpdaterWorker : IUpdaterWorker { + private readonly string _jobId; private readonly ExperimentsManager _experimentsManager; private readonly ILogger _logger; private readonly HashSet _processedProjectPaths = new(StringComparer.OrdinalIgnoreCase); @@ -20,8 +22,9 @@ public class UpdaterWorker : IUpdaterWorker Converters = { new JsonStringEnumConverter() }, }; - public UpdaterWorker(ExperimentsManager experimentsManager, ILogger logger) + public UpdaterWorker(string jobId, ExperimentsManager experimentsManager, ILogger logger) { + _jobId = jobId; _experimentsManager = experimentsManager; _logger = logger; } @@ -43,42 +46,15 @@ internal async Task RunWithErrorHandlingAsync(string repo { result = await RunAsync(repoRootPath, workspacePath, dependencyName, previousDependencyVersion, newDependencyVersion, isTransitive); } - catch (HttpRequestException ex) - when (ex.StatusCode == HttpStatusCode.Unauthorized || ex.StatusCode == HttpStatusCode.Forbidden) + catch (Exception ex) { if (!Path.IsPathRooted(workspacePath) || !File.Exists(workspacePath)) { workspacePath = Path.GetFullPath(Path.Join(repoRootPath, workspacePath)); } - - result = new() - { - ErrorType = ErrorType.AuthenticationFailure, - ErrorDetails = "(" + string.Join("|", NuGetContext.GetPackageSourceUrls(workspacePath)) + ")", - }; - } - catch (MissingFileException ex) - { - result = new() - { - ErrorType = ErrorType.MissingFile, - ErrorDetails = ex.FilePath, - }; - } - catch (UpdateNotPossibleException ex) - { - result = new() - { - ErrorType = ErrorType.UpdateNotPossible, - ErrorDetails = ex.Dependencies, - }; - } - catch (Exception ex) - { result = new() { - ErrorType = ErrorType.Unknown, - ErrorDetails = ex.ToString(), + Error = JobErrorBase.ErrorFromException(ex, _jobId, workspacePath), }; } diff --git a/nuget/lib/dependabot/nuget/native_helpers.rb b/nuget/lib/dependabot/nuget/native_helpers.rb index 2de499c279..963b85d572 100644 --- a/nuget/lib/dependabot/nuget/native_helpers.rb +++ b/nuget/lib/dependabot/nuget/native_helpers.rb @@ -11,6 +11,11 @@ module Nuget module NativeHelpers extend T::Sig + sig { returns(String) } + def self.job_id + ENV.fetch("DEPENDABOT_JOB_ID") + end + sig { returns(String) } def self.native_helpers_root helpers_root = ENV.fetch("DEPENDABOT_NATIVE_HELPERS_PATH", nil) @@ -66,6 +71,8 @@ def self.get_nuget_discover_tool_command(job_path:, repo_root:, workspace_path:, command_parts = [ exe_path, "discover", + "--job-id", + job_id, "--job-path", job_path, "--repo-root", @@ -81,6 +88,8 @@ def self.get_nuget_discover_tool_command(job_path:, repo_root:, workspace_path:, fingerprint = [ exe_path, "discover", + "--job-id", + "", "--job-path", "", "--repo-root", @@ -127,6 +136,8 @@ def self.get_nuget_analyze_tool_command(job_path:, repo_root:, discovery_file_pa command_parts = [ exe_path, "analyze", + "--job-id", + job_id, "--job-path", job_path, "--repo-root", @@ -144,6 +155,8 @@ def self.get_nuget_analyze_tool_command(job_path:, repo_root:, discovery_file_pa fingerprint = [ exe_path, "analyze", + "--job-id", + "", "--job-path", "", "--discovery-file-path", @@ -190,6 +203,8 @@ def self.get_nuget_updater_tool_command(job_path:, repo_root:, proj_path:, depen command_parts = [ exe_path, "update", + "--job-id", + job_id, "--job-path", job_path, "--repo-root", @@ -212,6 +227,8 @@ def self.get_nuget_updater_tool_command(job_path:, repo_root:, proj_path:, depen fingerprint = [ exe_path, "update", + "--job-id", + "", "--job-path", "", "--repo-root", @@ -290,29 +307,37 @@ def self.install_dotnet_sdks puts output end + # rubocop:disable Metrics/AbcSize sig { params(json: T::Hash[String, T.untyped]).void } def self.ensure_no_errors(json) - error_type = T.let(json.fetch("ErrorType", nil), T.nilable(String)) - error_details = json.fetch("ErrorDetails", nil) + error = T.let(json.fetch("Error", nil), T.nilable(T::Hash[String, T.untyped])) + return if error.nil? + + error_type = T.let(error.fetch("error-type"), String) + error_details = T.let(error.fetch("error-details", {}), T::Hash[String, T.untyped]) + case error_type - when "None", nil - # no issue - when "DependencyFileNotParseable" - 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" - raise UpdateNotPossible, T.let(error_details, T::Array[String]) - when "Unknown" - raise DependabotError, T.let(error_details, String) + when "dependency_file_not_found" + file_path = T.let(error_details.fetch("file-path"), String) + message = T.let(error_details.fetch("message", nil), T.nilable(String)) + raise DependencyFileNotFound.new(file_path, message) + when "dependency_file_not_parseable" + file_path = T.let(error_details.fetch("file-path"), String) + message = T.let(error_details.fetch("message", nil), T.nilable(String)) + raise DependencyFileNotParseable.new(file_path, message) + when "illformed_requirement" + raise BadRequirementError, T.let(error_details.fetch("message"), String) + when "private_source_authentication_failure" + raise PrivateSourceAuthenticationFailure, T.let(error_details.fetch("source"), String) + when "update_not_possible" + raise UpdateNotPossible, T.let(error_details.fetch("dependencies"), T::Array[String]) + when "unknown_error" + raise DependabotError, error_details.to_json else raise "Unexpected error type from native tool: #{error_type}: #{error_details}" end end + # rubocop:enable Metrics/AbcSize end end end diff --git a/nuget/spec/dependabot/nuget/file_fetcher_spec.rb b/nuget/spec/dependabot/nuget/file_fetcher_spec.rb index b2eb1be9a1..e56c04e589 100644 --- a/nuget/spec/dependabot/nuget/file_fetcher_spec.rb +++ b/nuget/spec/dependabot/nuget/file_fetcher_spec.rb @@ -260,8 +260,12 @@ def run_fetch_test(files_on_disk:, discovery_content_hash:, &_block) Projects: [], GlobalJson: nil, DotNetToolsJson: nil, - ErrorType: "AuthenticationFailure", - ErrorDetails: "the-error-details" + Error: { + "error-type": "private_source_authentication_failure", + "error-details": { + source: "some-package-source" + } + } } ) do expect { fetched_file_paths }.to raise_error(Dependabot::PrivateSourceAuthenticationFailure) diff --git a/nuget/spec/dependabot/nuget/file_parser_spec.rb b/nuget/spec/dependabot/nuget/file_parser_spec.rb index 7b9fecdcd0..6bb943f574 100644 --- a/nuget/spec/dependabot/nuget/file_parser_spec.rb +++ b/nuget/spec/dependabot/nuget/file_parser_spec.rb @@ -69,6 +69,7 @@ def clean_common_files def run_parser_test(&_block) ENV["DEPENDABOT_NUGET_CACHE_DISABLED"] = "true" + ENV["DEPENDABOT_JOB_ID"] = "TEST-JOB-ID" clean_common_files Dependabot::Nuget::DiscoveryJsonReader.testonly_clear_caches @@ -89,6 +90,7 @@ def run_parser_test(&_block) ensure Dependabot::Nuget::DiscoveryJsonReader.testonly_clear_caches ENV.delete("DEPENDABOT_NUGET_CACHE_DISABLED") + ENV.delete("DEPENDABOT_JOB_ID") clean_common_files end diff --git a/nuget/spec/dependabot/nuget/file_updater_spec.rb b/nuget/spec/dependabot/nuget/file_updater_spec.rb index becb6418ce..0cd3983541 100644 --- a/nuget/spec/dependabot/nuget/file_updater_spec.rb +++ b/nuget/spec/dependabot/nuget/file_updater_spec.rb @@ -92,6 +92,7 @@ def clean_common_files def run_update_test(&_block) # caching is explicitly required for these tests ENV["DEPENDABOT_NUGET_CACHE_DISABLED"] = "false" + ENV["DEPENDABOT_JOB_ID"] = "TEST-JOB-ID" Dependabot::Nuget::DiscoveryJsonReader.testonly_clear_caches clean_common_files @@ -123,6 +124,7 @@ def run_update_test(&_block) ensure Dependabot::Nuget::DiscoveryJsonReader.testonly_clear_caches ENV["DEPENDABOT_NUGET_CACHE_DISABLED"] = "true" + ENV.delete("DEPENDABOT_JOB_ID") clean_common_files end diff --git a/nuget/spec/dependabot/nuget/native_helpers_spec.rb b/nuget/spec/dependabot/nuget/native_helpers_spec.rb index 122aa8a656..c93b8d02ef 100644 --- a/nuget/spec/dependabot/nuget/native_helpers_spec.rb +++ b/nuget/spec/dependabot/nuget/native_helpers_spec.rb @@ -36,8 +36,17 @@ let(:is_transitive) { false } let(:result_output_path) { "/path/to/result.json" } + before do + ENV["DEPENDABOT_JOB_ID"] = "TEST-JOB-ID" + end + + after do + ENV.delete("DEPENDABOT_JOB_ID") + end + it "returns a properly formatted command with spaces on the path" do - expect(command).to eq("/path/to/NuGetUpdater.Cli update --job-path /path/to/job.json --repo-root /path/to/repo " \ + expect(command).to eq("/path/to/NuGetUpdater.Cli update --job-id TEST-JOB-ID --job-path /path/to/job.json " \ + "--repo-root /path/to/repo " \ '--solution-or-project /path/to/repo/src/some\ project/some_project.csproj ' \ "--dependency Some.Package --new-version 1.2.3 --previous-version 1.2.2 " \ "--result-output-path /path/to/result.json") @@ -92,8 +101,12 @@ .to receive(:run_shell_command) .and_wrap_original do |_original_method, *_args, &_block| result = { - ErrorType: "AuthenticationFailure", - ErrorDetails: "the-error-details" + Error: { + "error-type": "private_source_authentication_failure", + "error-details": { + source: "some-url" + } + } } File.write(described_class.update_result_file_path, result.to_json) end @@ -118,8 +131,13 @@ .to receive(:run_shell_command) .and_wrap_original do |_original_method, *_args, &_block| result = { - ErrorType: "MissingFile", - ErrorDetails: "the-error-details" + Error: { + "error-type": "dependency_file_not_found", + "error-details": { + message: "some message", + "file-path": "/some/file" + } + } } File.write(described_class.update_result_file_path, result.to_json) end @@ -145,7 +163,7 @@ # defaults to no error return nil rescue StandardError => e - return e.to_s + return e end context "when nothing is reported" do @@ -154,61 +172,121 @@ it { is_expected.to be_nil } end - context "when the error is expclicitly none" do + context "when the error is expclicitly null" do let(:json) do { - ErrorType: "None" + Error: nil }.to_json end it { is_expected.to be_nil } end - context "when an authentication failure is encountered" do + context "when a dependency file was not found" do + let(:json) do + { + Error: { + "error-type": "dependency_file_not_found", + "error-details": { + message: "some message", + "file-path": "/some/file" + } + } + }.to_json + end + + it { is_expected.to be_a Dependabot::DependencyFileNotFound } + end + + context "when a file is not parseable" do let(:json) do { - ErrorType: "AuthenticationFailure", - ErrorDetails: "(some-source)" + Error: { + "error-type": "dependency_file_not_parseable", + "error-details": { + message: "some message", + "file-path": "/some/file" + } + } }.to_json end - it { is_expected.to include(": (some-source)") } + it { is_expected.to be_a Dependabot::DependencyFileNotParseable } end - context "when a file is missing" do + context "when a requirement cannot be parsed" do let(:json) do { - ErrorType: "MissingFile", - ErrorDetails: "some.file" + Error: { + "error-type": "illformed_requirement", + "error-details": { + message: "some message" + } + } }.to_json end - it { is_expected.to include("some.file not found") } + it { is_expected.to be_a Dependabot::BadRequirementError } + end + + context "when an authenticated feed was rejected" do + let(:json) do + { + Error: { + "error-type": "private_source_authentication_failure", + "error-details": { + source: "some-url" + } + } + }.to_json + end + + it { is_expected.to be_a Dependabot::PrivateSourceAuthenticationFailure } end context "when an update is not possible" do let(:json) do { - ErrorType: "UpdateNotPossible", - ErrorDetails: [ - "dependency 1", - "dependency 2" - ] + Error: { + "error-type": "update_not_possible", + "error-details": { + dependencies: %w(dep1 dep2) + } + } }.to_json end - it { is_expected.to include("dependency 1, dependency 2") } + it { is_expected.to be_a Dependabot::UpdateNotPossible } end - context "when any other error is returned" do + context "when an unknown error is reported" do let(:json) do { - ErrorType: "Unknown", - ErrorDetails: "some error details" + Error: { + "error-type": "unknown_error", + "error-details": { + message: "some message" + } + } + }.to_json + end + + it { is_expected.to be_a Dependabot::DependabotError } + end + + context "when any other type of error is returned" do + let(:json) do + { + Error: { + "error-type": "some_error_type_that_is_not_handled", + "error-details": { + message: "some message" + } + } }.to_json end - it { is_expected.to include("some error details") } + it { is_expected.to be_a StandardError } end end end diff --git a/nuget/spec/dependabot/nuget/update_checker_spec.rb b/nuget/spec/dependabot/nuget/update_checker_spec.rb index 8904a31ff4..c4b9caa6e7 100644 --- a/nuget/spec/dependabot/nuget/update_checker_spec.rb +++ b/nuget/spec/dependabot/nuget/update_checker_spec.rb @@ -287,8 +287,12 @@ def intercept_native_tools(discovery_content_hash:, dependency_name:, analysis_c CanUpdate: false, VersionComesFromMultiDependencyProperty: false, UpdatedDependencies: [], - ErrorType: "AuthenticationFailure", - ErrorDetails: "the-error-details" + Error: { + "error-type": "private_source_authentication_failure", + "error-details": { + source: "some-package-source" + } + } } ) end