diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationRestoreTests.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationRestoreTests.cs index fd76ce4595c..69ed8dceb33 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationRestoreTests.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationRestoreTests.cs @@ -305,7 +305,7 @@ public async Task NonexistentTagFallsBack(string inputJson) ""AssetsRepoId"": """", ""TagPrefix"": ""main"", ""Tag"": ""INVALID_TAG"" - }", "Invocation of \"git fetch origin refs/tags/INVALID_TAG:refs/tags/INVALID_TAG\" had a non-zero exit code -1")] + }", "Invocation of \"git fetch origin refs/tags/INVALID_TAG:refs/tags/INVALID_TAG\" had a non-zero exit code 128")] [Trait("Category", "Integration")] public async Task InvalidTagThrows(string inputJson, string httpException) { diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/Exceptions/GitProcessException.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/Exceptions/GitProcessException.cs index bdece0e4ffc..00d0c74a1d6 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/Exceptions/GitProcessException.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/Exceptions/GitProcessException.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Net; @@ -17,6 +17,8 @@ public GitProcessException(CommandResult result) Result = result; } + public override string Message { get { return this.Result.StdErr; } } + // Override the ToString so it'll give the command exception's toString which // will contain the accurate message and callstack. This is necessary in the // event the exception goes unhandled. diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Startup.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Startup.cs index 6e021b99187..eecdedab1c7 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Startup.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Startup.cs @@ -80,10 +80,12 @@ private static async Task Run(object commandObj) switch (commandObj) { case ConfigLocateOptions configOptions: + DefaultStore.SetStoreExceptionMode(false); assetsJson = RecordingHandler.GetAssetsJsonLocation(configOptions.AssetsJsonPath, TargetLocation); System.Console.WriteLine(await DefaultStore.GetPath(assetsJson)); break; case ConfigShowOptions configOptions: + DefaultStore.SetStoreExceptionMode(false); assetsJson = RecordingHandler.GetAssetsJsonLocation(configOptions.AssetsJsonPath, TargetLocation); using(var f = File.OpenRead(assetsJson)) { @@ -92,6 +94,7 @@ private static async Task Run(object commandObj) } break; case ConfigCreateOptions configOptions: + DefaultStore.SetStoreExceptionMode(false); assetsJson = RecordingHandler.GetAssetsJsonLocation(configOptions.AssetsJsonPath, TargetLocation); throw new NotImplementedException("Interactive creation of assets.json feature is not yet implemented."); case ConfigOptions configOptions: @@ -101,14 +104,17 @@ private static async Task Run(object commandObj) StartServer(startOptions); break; case PushOptions pushOptions: + DefaultStore.SetStoreExceptionMode(false); assetsJson = RecordingHandler.GetAssetsJsonLocation(pushOptions.AssetsJsonPath, TargetLocation); await DefaultStore.Push(assetsJson); break; case ResetOptions resetOptions: + DefaultStore.SetStoreExceptionMode(false); assetsJson = RecordingHandler.GetAssetsJsonLocation(resetOptions.AssetsJsonPath, TargetLocation); await DefaultStore.Reset(assetsJson); break; case RestoreOptions restoreOptions: + DefaultStore.SetStoreExceptionMode(false); assetsJson = RecordingHandler.GetAssetsJsonLocation(restoreOptions.AssetsJsonPath, TargetLocation); await DefaultStore.Restore(assetsJson); break; diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitProcessHandler.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitProcessHandler.cs index 9f226ffc9bc..3c59a42bc74 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitProcessHandler.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitProcessHandler.cs @@ -27,6 +27,8 @@ public class CommandResult public class GitProcessHandler { public const int RETRY_INTERMITTENT_FAILURE_COUNT = 3; + + public bool ThrowOnException = true; /// /// Internal class to hold the minimum supported version of git. If that /// version changes we only need to change it here. @@ -195,16 +197,25 @@ public virtual CommandResult Run(string arguments, string workingDirectory) } } } - // exceptions caught here will be to do with inability to start the git process - // otherwise all "error" states should be handled by the output to stdErr and non-zero exitcode. - catch (Exception e) + // exceptions caught here will be to do with inability to recover from a failed git process + // rather than endlessly retrying and concealing the git error from the user, immediately report and exit the process with an error code + catch (GitProcessException e) { - DebugLogger.LogDebug(e.Message); + // we rethrow here in contexts where we can expect the ASP.NET middleware to handle the thrown exception + if (ThrowOnException) + { + DebugLogger.LogError(e.Result.StdErr); - result.ExitCode = -1; - result.CommandException = e; + throw new GitProcessException(e.Result); + } + // otherwise, we log the error, then early exit from the proces (in CLI contexts) + else + { + DebugLogger.LogError("Git ran into an unrecoverable error. Test-Proxy is exiting. The error output from git is: "); + DebugLogger.LogError(e.Result.StdErr); - throw new GitProcessException(result); + Environment.Exit(1); + } } }); diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index 3c18089d393..aa318381887 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -75,6 +75,18 @@ public GitStore(IConsoleWrapper consoleWrapper) } #region push, reset, restore, and other asset repo implementations + /// + /// Set the GitHandler exception mode. + /// + /// When false: unrecoverable git exceptions will print the error, and early exit + /// When true: unrecoverable git exceptions will log, then be rethrown for the Exception middleware to handle and return as a valid non-successful http response. + /// + /// + public void SetStoreExceptionMode(bool throwOnException) + { + this.GitHandler.ThrowOnException = throwOnException; + } + /// /// Given a config, locate the cloned assets. /// diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/IAssetsStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/IAssetsStore.cs index 4d3eafd20ae..f67671fb6ad 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/IAssetsStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/IAssetsStore.cs @@ -33,5 +33,12 @@ public interface IAssetsStore /// /// public abstract Task GetPath(string pathToAssetsJson); + + /// + /// Set the mode of the store to throw exceptions or to simply early exit for CLI mode. + /// + /// + /// + public abstract void SetStoreExceptionMode(bool throwOnException); } } diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/NullStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/NullStore.cs index 3290ff774c6..a76bfe78ba8 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/NullStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/NullStore.cs @@ -25,5 +25,6 @@ public AssetsConfiguration ParseConfigurationFile(string pathToAssetsJson) } public Task GetPath(string pathToAssetsJson) { return null; } + public void SetStoreExceptionMode(bool throwOnException) { } } } diff --git a/tools/test-proxy/scripts/test-scripts/CLIIntegration.Tests.ps1 b/tools/test-proxy/scripts/test-scripts/CLIIntegration.Tests.ps1 index ed5b936e71f..da680b8614b 100644 --- a/tools/test-proxy/scripts/test-scripts/CLIIntegration.Tests.ps1 +++ b/tools/test-proxy/scripts/test-scripts/CLIIntegration.Tests.ps1 @@ -98,6 +98,26 @@ Describe "AssetsModuleTests" { Test-FileVersion -FilePath $localAssetsFilePath -FileName "file4.txt" -ExpectedVersion 1 Test-FileVersion -FilePath $localAssetsFilePath -FileName "file5.txt" -ExpectedVersion 1 } + It "Should fail to restore an invalid tag" { + $recordingJson = [PSCustomObject]@{ + AssetsRepo = "Azure/azure-sdk-assets-integration" + AssetsRepoPrefixPath = "pull/scenarios" + AssetsRepoId = "" + TagPrefix = "main" + Tag = "python/tables_fc54d0INVALID" + } + $files = @( + "assets.json" + ) + $testFolder = Describe-TestFolder -AssetsJsonContent $recordingJson -Files $files + $assetsFile = Join-Path $testFolder "assets.json" + $assetsJsonRelativePath = [System.IO.Path]::GetRelativePath($testFolder, $assetsFile) + + $CommandArgs = "restore --assets-json-path $assetsJsonRelativePath" + Invoke-ProxyCommand -TestProxyExe $TestProxyExe -CommandArgs $CommandArgs -MountDirectory $testFolder + + $LASTEXITCODE | Should -Be 1 + } AfterEach { Remove-Test-Folder $testFolder } @@ -164,7 +184,7 @@ Describe "AssetsModuleTests" { Test-FileVersion -FilePath $localAssetsFilePath -FileName "file1.txt" -ExpectedVersion 1 Test-FileVersion -FilePath $localAssetsFilePath -FileName "file2.txt" -ExpectedVersion 1 Test-FileVersion -FilePath $localAssetsFilePath -FileName "file3.txt" -ExpectedVersion 1 - + # Create a new file and verify Edit-FileVersion -FilePath $localAssetsFilePath -FileName "file4.txt" -Version 1 Test-FileVersion -FilePath $localAssetsFilePath -FileName "file4.txt" -ExpectedVersion 1 @@ -174,7 +194,7 @@ Describe "AssetsModuleTests" { # Delete a file $fileToRemove = Join-Path -Path $localAssetsFilePath -ChildPath "file2.txt" Remove-Item -Path $fileToRemove - + # Reset answering Y and they should all go back to original restore $CommandArgs = "reset --assets-json-path $assetsJsonRelativePath" Invoke-ProxyCommand -TestProxyExe $TestProxyExe -CommandArgs $CommandArgs -MountDirectory $testFolder -WriteOutput "Y" @@ -211,7 +231,7 @@ Describe "AssetsModuleTests" { Test-FileVersion -FilePath $localAssetsFilePath -FileName "file1.txt" -ExpectedVersion 1 Test-FileVersion -FilePath $localAssetsFilePath -FileName "file2.txt" -ExpectedVersion 1 Test-FileVersion -FilePath $localAssetsFilePath -FileName "file3.txt" -ExpectedVersion 1 - + # Create two new files and verify Edit-FileVersion -FilePath $localAssetsFilePath -FileName "file4.txt" -Version 1 Test-FileVersion -FilePath $localAssetsFilePath -FileName "file4.txt" -ExpectedVersion 1 @@ -223,7 +243,7 @@ Describe "AssetsModuleTests" { # Delete a file $fileToRemove = Join-Path -Path $localAssetsFilePath -ChildPath "file2.txt" Remove-Item -Path $fileToRemove - + # Reset answering N and they should remain changed as per the previous changes $CommandArgs = "reset --assets-json-path $assetsJsonRelativePath" Invoke-ProxyCommand -TestProxyExe $TestProxyExe -CommandArgs $CommandArgs -MountDirectory $testFolder -WriteOutput "N" @@ -446,9 +466,9 @@ Describe "AssetsModuleTests" { TagPrefix = $created_tag_prefix Tag = "" } - + $assetsJsonRelativePath = Join-Path "sdk" "keyvault" "azure-keyvault-keys" "assets.json" - + $files = @( $assetsJsonRelativePath ) @@ -457,16 +477,16 @@ Describe "AssetsModuleTests" { $CommandArgs = "restore --assets-json-path $assetsJsonRelativePath" Invoke-ProxyCommand -TestProxyExe $TestProxyExe -CommandArgs $CommandArgs -MountDirectory $testFolder $LASTEXITCODE | Should -Be 0 - + $localAssetsFilePath = Join-Path $testFolder ".assets" $assetsFolder = $(Get-ChildItem $localAssetsFilePath -Directory)[0].FullName mkdir -p $(Join-Path $assetsFolder $creationPath) - + # Create new files. These are in a predictable location with predicatable content so we can generate the same SHA twice in a row. Edit-FileVersion -FilePath $assetsFolder -FileName $file1 -Version 1 Edit-FileVersion -FilePath $assetsFolder -FileName $file2 -Version 1 Edit-FileVersion -FilePath $assetsFolder -FileName $file3 -Version 1 - + # Push the changes $CommandArgs = "push --assets-json-path $assetsJsonRelativePath" Invoke-ProxyCommand -TestProxyExe $TestProxyExe -CommandArgs $CommandArgs -MountDirectory $testFolder @@ -489,13 +509,13 @@ Describe "AssetsModuleTests" { Edit-FileVersion -FilePath $newAssetsFolder -FileName $file1 -Version 1 Edit-FileVersion -FilePath $newAssetsFolder -FileName $file2 -Version 1 Edit-FileVersion -FilePath $newAssetsFolder -FileName $file3 -Version 1 - + $CommandArgs = "push --assets-json-path $assetsJsonRelativePath" Invoke-ProxyCommand -TestProxyExe $TestProxyExe -CommandArgs $CommandArgs -MountDirectory $newTestFolder $LASTEXITCODE | Should -Be 0 - + $updatedAssets = Update-AssetsFromFile -AssetsJsonContent $newAssetsFile - + $exists = Test-TagExists -AssetsJsonContent $updatedAssets -WorkingDirectory $localAssetsFilePath $exists | Should -Be $true } @@ -544,7 +564,7 @@ Describe "AssetsModuleTests" { Edit-FileVersion -FilePath $assetsFolder -FileName $file1 -Version 3 Edit-FileVersion -FilePath $assetsFolder -FileName $file2 -Version 3 Edit-FileVersion -FilePath $assetsFolder -FileName $file3 -Version 3 - + # now lets restore again $CommandArgs = "restore --assets-json-path $assetsJsonRelativePath" Invoke-ProxyCommand -TestProxyExe $TestProxyExe -CommandArgs $CommandArgs -MountDirectory $testFolder @@ -592,7 +612,7 @@ Describe "AssetsModuleTests" { Edit-FileVersion -FilePath $assetsFolder -FileName $file1 -Version 3 Edit-FileVersion -FilePath $assetsFolder -FileName $file2 -Version 3 Edit-FileVersion -FilePath $assetsFolder -FileName $file3 -Version 3 - + # now lets modify the targeted tag. this simulates a user checking out a different branch or commit in their language repo $assetsJsonLocation = Join-Path $testFolder $assetsJsonRelativePath $recordingJson.Tag = "python/tables_fc54d0"