From ef0af95845928c1dbab8be7de1e7390c53ce2ac6 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 8 Feb 2023 10:18:32 -0800 Subject: [PATCH] Make the exception error in PowerShell able to associate with the right history entry (#19095) --- .../engine/AutomationEngine.cs | 16 ++- .../engine/lang/parserutils.cs | 16 +++ .../engine/runtime/Operations/MiscOps.cs | 5 +- .../utils/ParserException.cs | 4 +- .../Scripting/ErrorHistoryId.Tests.ps1 | 110 ++++++++++++++++++ 5 files changed, 144 insertions(+), 7 deletions(-) create mode 100644 test/powershell/Language/Scripting/ErrorHistoryId.Tests.ps1 diff --git a/src/System.Management.Automation/engine/AutomationEngine.cs b/src/System.Management.Automation/engine/AutomationEngine.cs index 61952f7739..cc1ca2f5ad 100644 --- a/src/System.Management.Automation/engine/AutomationEngine.cs +++ b/src/System.Management.Automation/engine/AutomationEngine.cs @@ -105,12 +105,22 @@ namespace System.Management.Automation if (errors.Length > 0) { - if (errors[0].IncompleteInput) + ParseException ex = errors[0].IncompleteInput + ? new IncompleteParseException(errors[0].Message, errors[0].ErrorId) + : new ParseException(errors); + + if (addToHistory) { - throw new IncompleteParseException(errors[0].Message, errors[0].ErrorId); + // Try associating the parsing error with the history item if we can. + InvocationInfo invInfo = ex.ErrorRecord.InvocationInfo; + LocalRunspace localRunspace = Context.CurrentRunspace as LocalRunspace; + if (invInfo is not null && localRunspace?.History is not null) + { + invInfo.HistoryId = localRunspace.History.GetNextHistoryId(); + } } - throw new ParseException(errors); + throw ex; } return new ScriptBlock(ast, isFilter: false); diff --git a/src/System.Management.Automation/engine/lang/parserutils.cs b/src/System.Management.Automation/engine/lang/parserutils.cs index f159e72be4..e997068481 100644 --- a/src/System.Management.Automation/engine/lang/parserutils.cs +++ b/src/System.Management.Automation/engine/lang/parserutils.cs @@ -1973,6 +1973,22 @@ namespace System.Management.Automation } } } + + internal static void UpdateExceptionErrorRecordHistoryId(RuntimeException exception, ExecutionContext context) + { + InvocationInfo invInfo = exception.ErrorRecord.InvocationInfo; + if (invInfo is not { HistoryId: -1 }) + { + return; + } + + if (context?.CurrentCommandProcessor is null) + { + return; + } + + invInfo.HistoryId = context.CurrentCommandProcessor.Command.MyInvocation.HistoryId; + } } #endregion InterpreterError diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index e6efdace4c..6e67925ddb 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -1266,7 +1266,7 @@ namespace System.Management.Automation } catch (Exception exception) { - if (!(exception is RuntimeException rte)) + if (exception is not RuntimeException rte) { throw ExceptionHandlingOps.ConvertToRuntimeException(exception, functionDefinitionAst.Extent); } @@ -1623,6 +1623,9 @@ namespace System.Management.Automation InterpreterError.UpdateExceptionErrorRecordPosition(rte, funcContext.CurrentPosition); } + // Update the history id if needed to associate the exception with the right history item. + InterpreterError.UpdateExceptionErrorRecordHistoryId(rte, funcContext._executionContext); + var context = funcContext._executionContext; var outputPipe = funcContext._outputPipe; diff --git a/src/System.Management.Automation/utils/ParserException.cs b/src/System.Management.Automation/utils/ParserException.cs index b705610821..9087c374b6 100644 --- a/src/System.Management.Automation/utils/ParserException.cs +++ b/src/System.Management.Automation/utils/ParserException.cs @@ -127,9 +127,7 @@ namespace System.Management.Automation /// Initializes a new instance of the ParseException class with a collection of error messages. /// /// The collection of error messages. - [SuppressMessage("Microsoft.Usage", "CA2214:DoNotCallOverridableMethodsInConstructors", - Justification = "ErrorRecord is not overridden in classes deriving from ParseException")] - public ParseException(Language.ParseError[] errors) + public ParseException(ParseError[] errors) { ArgumentNullException.ThrowIfNull(errors); diff --git a/test/powershell/Language/Scripting/ErrorHistoryId.Tests.ps1 b/test/powershell/Language/Scripting/ErrorHistoryId.Tests.ps1 new file mode 100644 index 0000000000..3d467b66c1 --- /dev/null +++ b/test/powershell/Language/Scripting/ErrorHistoryId.Tests.ps1 @@ -0,0 +1,110 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe "Error HistoryId Tests" -Tags "CI" { + + BeforeAll { + $setting = [System.Management.Automation.PSInvocationSettings]::New() + $setting.AddToHistory = $true + + function RunCommand($ps, $command) { + $ps.Commands.Clear() + $ps.AddScript($command).Invoke($null, $setting) + } + + $funcBarUseWriteErrorApi = @' +function bar { + [CmdletBinding()] + param() + + $er = [System.Management.Automation.ErrorRecord]::new( + [System.ArgumentException]::new(), + 'PSCmdlet.WriteError', + [System.Management.Automation.ErrorCategory]::InvalidOperation, + $null) + $PSCmdlet.WriteError($er) +} +'@ + $funcBarThrowTermError = @' +function bar { + [CmdletBinding()] + param() + + $er = [System.Management.Automation.ErrorRecord]::new( + [System.ArgumentException]::new(), + 'PSCmdlet.ThrowTerminatingError', + [System.Management.Automation.ErrorCategory]::InvalidOperation, + $null) + $PSCmdlet.ThrowTerminatingError($er) +} +'@ + } + + BeforeEach { + $ps = [PowerShell]::Create("NewRunspace") + } + + AfterEach { + $ps.Dispose() + } + + It "Error from has the right history Id" -TestCases @( + @{ caption = "'throw' in global scope"; cmd1 = "1+1"; cmd2 = "throw 'abc'" } + @{ caption = "'throw' in function"; cmd1 = "function bar { throw 'abc' }"; cmd2 = "bar" } + ) { + param($cmd1, $cmd2) + + try { + ## 1st in history + $null = RunCommand $ps $cmd1 + ## 2nd in history + $null = RunCommand $ps $cmd2 + } catch { + ## ignore the exception + } + + $ps.HadErrors | Should -BeTrue + $err = RunCommand $ps '$Error[0]' + $err.FullyQualifiedErrorId | Should -BeExactly 'abc' + $err.InvocationInfo.HistoryId | Should -Be 2 + } + + It "Error from has the right history Id" -TestCases @( + @{ caption = "parameter binding"; cmd1 = "1+1"; cmd2 = "Get-Verb -abc"; errId = "NamedParameterNotFound,Microsoft.PowerShell.Commands.GetVerbCommand" } + @{ caption = "'1/0' in global scope"; cmd1 = "1+1"; cmd2 = "1/0"; errId = "RuntimeException" } + @{ caption = "'1/0' in function"; cmd1 = "function bar { 1/0 }"; cmd2 = "bar"; errId = "RuntimeException" } + @{ caption = "method exception in global scope"; cmd1 = "1+1"; cmd2 = "[System.IO.Path]::GetExtension()"; errId = "MethodCountCouldNotFindBest" } + @{ caption = "method exception in function"; cmd1 = "function bar { [System.IO.Path]::GetExtension() }"; cmd2 = "bar"; errId = "MethodCountCouldNotFindBest" } + @{ caption = "'Write-Error'"; cmd1 = "function bar { Write-Error 'abc' }"; cmd2 = "bar"; errId = "Microsoft.PowerShell.Commands.WriteErrorException,bar" } + @{ caption = "'PSCmdlet.WriteError'"; cmd1 = $funcBarUseWriteErrorApi; cmd2 = "bar"; errId = "PSCmdlet.WriteError,bar" } + @{ caption = "'PSCmdlet.ThrowTerminatingError'"; cmd1 = $funcBarThrowTermError; cmd2 = "bar"; errId = "PSCmdlet.ThrowTerminatingError,bar" } + ) { + param($cmd1, $cmd2, $errId) + + ## 1st in history + $null = RunCommand $ps $cmd1 + ## 2nd in history + $null = RunCommand $ps $cmd2 + + $ps.HadErrors | Should -BeTrue + $err = RunCommand $ps '$Error[0]' + $err.FullyQualifiedErrorId | Should -BeExactly $errId + $err.InvocationInfo.HistoryId | Should -Be 2 + } + + It "ParseError has the right history Id" { + try { + ## 1st in history + $null = RunCommand $ps '1+1' + ## 2nd in history + $null = RunCommand $ps 'for (int i = 2; i < 3; i++) { foreach {} }' + } catch { + ## ignore the exception + } + + $ps.HadErrors | Should -BeTrue + $err = RunCommand $ps '$Error[0]' + $err | Should -BeOfType 'System.Management.Automation.ParseException' + $err.ErrorRecord.InvocationInfo.HistoryId | Should -Be 2 + } +}