Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why do handled exceptions show in ErrorVariable? #3768

Open
Jaykul opened this issue May 12, 2017 · 20 comments
Open

Why do handled exceptions show in ErrorVariable? #3768

Jaykul opened this issue May 12, 2017 · 20 comments
Labels
Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Enhancement the issue is more of a feature request than a bug Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a KeepOpen The bot will ignore these and not auto-close Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors WG-Engine core PowerShell engine, interpreter, and runtime

Comments

@Jaykul
Copy link
Contributor

Jaykul commented May 12, 2017

When you use try/catch and handle an exception, it still shows up in the global $Error, and more importantly, in the ErrorVariable.

Is there any way to actually prevent handled exceptions from leaking into the ErrorVariable? Is there any way for a caller to determine that they are not, in fact, errors, but expected results that were dealt with by the function? That is, how does a caller distinguish the errors that matter?

Consider a simple example:

function Count-Parameters {
    [CmdletBinding()]
    param([Parameter(ValueFromRemainingArguments)]$Parameters)
    try {
        $Parameters.FasterMethod()
    } catch {
        $Parameters.Count
    }
}

$Count = Count-Parameters # Everything is fine

$Count = Count-Parameters This One Has Four # Still fine

# But then my paranoid "best practice" fanatic does something like this:
$Count = Count-Parameters -ErrorVariable noCount
if(!$Count -and $NoCount) { 
   Write-Warning $noCount
}

How can I stop the error variable from containing these things that I explicitly handled?

@iSazonov iSazonov added WG-Engine core PowerShell engine, interpreter, and runtime Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a labels May 12, 2017
@mklement0
Copy link
Contributor

mklement0 commented May 12, 2017

Good catch (accidental pun).

Note that another quirk comes into play here: -ErrorVariable always returns a [System.Collections.ArrayList] instance, so that you'd have to use Write-Warning $noCount[0], whereas Write-Warning $noCount would actually break, even though only 1 error is returned:

Write-Warning : Cannot convert 'System.Collections.ArrayList' to the type 'System.String' required by parameter 'Message'. Specified method is not supported.`

-OutVariable, -InformationVariable, and -WarningVariable exhibit the same behavior - see #3773.

@BrucePay
Copy link
Collaborator

$Error was originally conceived as a running log of all errors that happened in your script, including exceptions that were caught or when ErrorAction was set to SilentlyContinue The goal was primarily for forensics analysis after your script had been run. (Note: this goal of recording all errors was muted somewhat by the introduction of -ErrorAction ignore where the error isn't written into the error variable.)

@SteveL-MSFT
Copy link
Member

Perhaps a non-breaking change is we could add a IsHandled property to ErrorRecord to help filter?

@Jaykul
Copy link
Contributor Author

Jaykul commented May 16, 2017

@BrucePay and the -ErrorVariable must mirror that? I mean, I'm ok with everything being in $Error ...

@SteveL-MSFT I guess that would at least give us a way ...

Right now I do not think there is a way to hide errors, and also tell whether or not a command was successful.

@SteveL-MSFT SteveL-MSFT added Issue-Enhancement the issue is more of a feature request than a bug Review - Committee The PR/Issue needs a review from the PowerShell Committee labels May 18, 2017
@joeyaiello
Copy link
Contributor

joeyaiello commented May 24, 2017

The @PowerShell/powershell-committee had a very interesting conversation around this, it's difficult to know which way we should go with this.

Could you explain in more detail why this is important to you? Is it simply that you're trying to drill the error variable down to zero as you test your code and try to catch all exceptions?

@mklement0
Copy link
Contributor

mklement0 commented May 25, 2017

I obviously can't speak for @Jaykul, but here's my 0.0178236 EUR (using today's exchange rate):

There are two conceivable scenarios (not mutually exclusive):

  • (a) I want to know if any non-terminating error occurred when the cmdlet / advanced function ran - in other words: if the cmdlet / advanced function fully succeeded.

  • (b) I want to examine the specific non-terminating errors emitted, if any, when the cmdlet / advanced function ran.

(a), even though it should be covered by automatic variable $?, is currently not a reliable option (pet peeve alert), because:

(b) is currently not a reliable option, because examining the -ErrorVariable-assigned variable or the added-since-before-invocation $Error entries may contain false positives in the form of intentionally caught exceptions that should be considered an implementation detail of the cmdlet / advanced function invoked - that's this issue's complaint.

@SteveL-MSFT SteveL-MSFT added Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels May 31, 2017
@SteveL-MSFT
Copy link
Member

@PowerShell/powershell-committee reviewed this (@BrucePay was absent from discussion) and agreed this change where -errorvariable only contains unhandled errors makes sense

@KirkMunro
Copy link
Contributor

KirkMunro commented Jun 27, 2017

There are some related data points here that are worth highlighting as part of this discussion before breaking them out into their own issues.

It seems some other use cases surrounding error handling are resulting in confusion as well. For example, if you invoke PowerShell using the System.Management.Automation.PowerShell class, and if you handle the errors in the command you invoke by indicating that they should be ignored, S.M.A.PowerShell will still set HadErrors to true. Here is a code sample illustrating the problem:

$ps = [PowerShell]::Create()
$ps.AddCommand('Get-Service').AddParameter('Name','Invalid').AddParameter('ErrorAction',[System.Management.Automation.ActionPreference]::Ignore) > $null
$ps.Invoke()
$ps.HadErrors # returns $true; expectation is that this would return false since the command was configured to ignore errors

You can see the same behaviour via direct command invocation in PowerShell:

Get-Service -Name Invalid -ErrorAction Ignore
$? # returns $false; shouldn't this return $true?

In both of these cases, -ErrorAction Ignore is being used to tell PowerShell that the error can be completely ignored. Why then, do both $? and $ps.HadErrors indicate the error? Technically there was an error, but it is being handled/treated like it is not an error, so I believe in both of these cases $? and $ps.HadErrors should have values indicating that there wasn't an error. Thoughts?

@mklement0
Copy link
Contributor

@SteveL-MSFT:

Just to clarify: does that mean that caught errors will be absent from -ErrorVariable and $Error (which I think makes sense), or only the former?

Note that if such errors are still recorded in $Error, the implication is that the for-non-terminating-errors-only -ErrorAction Ignore construct will have no counterpart for terminating errors.

@mklement0
Copy link
Contributor

@KirkMunro:

I'm also inclined to think that $? and .HadErrors should reflect $True with -ErrorAction Ignore, given that someone who really needs to know whether a error occurred has the option of using -ErrorAction SilentlyContinue -ErrorVariable ev and then inspecting $ev.

Given that terminating errors (those that can be caught with try / catch) aren't affected by -ErrorAction, however, can I suggest you open a new issue?

On a side note: try { 1 / 0 } catch {}; $? also yields $False, due to the exception triggered by 1 / 0 setting $? to $False and the catch block being empty.

@Jaykul
Copy link
Contributor Author

Jaykul commented Aug 18, 2017

@mklement0 I think you're confusing the use case.

Are you suggesting that it's ok that an application that uses the PowerShell class to invoke scripts for automation would need to inject script wrappers around scripts provided by end-users just to be able to tell whether or not they succeeded?

@mklement0
Copy link
Contributor

@Jaykul:

We are talking about a different use case - at least based on how error handling currently works, where -ErrorAction truly only applies to non-terminating errors, as opposed to terminating errors (both script- and statement-terminating), which are the only ones that try / catch can catch - which is why I suggested creating a new issue.

Apart from that, I don't understand what you're asking.

@KirkMunro
Copy link
Contributor

@mklement0 That's really thin. If someone really wants to know if an error occurred when -ErrorAction Ignore is used, they should write their own script that doesn't use -ErrorAction Ignore. Give me a real world use case where someone would want to know if a benign error occurred in a command or script they invoked. You shouldn't care about that when you invoke a command, because at the moment you are invoking that command you are placing your trust that the author of that command will properly let you know if there was an error that you should care about. If you don't trust that, then you shouldn't be invoking that specific command.

The whole point of -ErrorAction Ignore is to suppress errors. Suppression is of little use for scripts invoked via S.M.A.PowerShell if the invoker has to ignore .HadErrors and check the Errors collection every time (which will be empty). In fact, I'd wager that there are probably many C# binaries out there where PowerShell is invoked from C# with S.M.A.PowerShell that are buggy right now because they rely on .HadErrors identifying whether or not there were actually errors.

I will log a separate issue for the .HadErrors/$? issue.

@mklement0
Copy link
Contributor

I will log a separate issue for the .HadErrors/$? issue.

Great, thanks.

Other than that, I don't see a disagreement, unless I misunderstood your original comment. You do not want $? and .HadErrors to reflect $False if -ErrorAction Ignore is used, correct? I agree.

Note that I was talking about SilentlyContinue, not Ignore, in the - definitely exotic - alternative scenario I described.

@KirkMunro
Copy link
Contributor

@mklement0 Whoops, you're right, I misread your previous reply. We're on the same page then.

@mklement0
Copy link
Contributor

mklement0 commented Aug 18, 2017

@Jaykul: While I'm still unclear on what you were asking, perhaps I can clarify on my end:

Re logging in $Error / -ErrorVariable:

My concern fits into the larger issue of the existing, longstanding dichotomy with respect to how non-terminating vs. terminating errors must be handled - see Our Error Handling, Ourselves - time to fully understand and properly document PowerShell's error handling.

As @BrucePay has pointed out, -ErrorAction Ignore already is a departure from the unconditionally-log-all-exceptions-in-$Error behavior, but is currently only available for non-terminating errors (e.g., Get-Item /NoSuchPath -ErrorAction -Ignore).

As stated, -ErrorAction Ignore has no effect on a (statement)-terminating error such as Get-Item -NoSuchParam -ErrorAction Ignore (a distinction that strikes me as problematic, especially that using the $ErrorActionPreference preference variable does apply to terminating errors, but it's probably way too late to change that).

Here's what -ErrorAction Ignore combined with -ErrorVariable currently does:

> $Error.Clear(); Get-Item /NoSuchPath -ErrorAction Ignore -ErrorVariable ev; $Error.Count; "[$ev]"
0  # nothing was written to $Error
[] # nothing was captured in $ev

Your proposal at least in part asks for the analogous behavior with respect to handling terminating errors, which must be done with Try/ Catch.

It is the in part aspect that I find problematic:

For symmetry, conceiving of Try/ Catch as the -ErrorAction Ignore counterpart - or, more accurately, a superset of it - makes sense to me.

If we only suppress filling the -ErrorVariable variable while continuing to log to $Error, we don't have that symmetry:

> $Error.Clear(); Invoke-Command { try {1 / 0} catch{} } -ErrorVariable ev; $Error.Count; "[$ev]"
1  # $Error was still written to, unlike with -ErrorAction Ignore
[] # in the future: $ev was not assigned to

Hence my suggestion to not only suppress assigning to the -ErrorVariable variable, but to also suppress logging to $Error.


Re $? / .HadErrors:

@KirkMunro then took a step back to bring up a conceptually related issue, arguing that -ErrorAction Ignore - again: applicable to non-terminating errors only - should indicate success, given the explicitly signaled intent to ignore errors - similar to how Try / Catch can be conceived of.

My aside re try { 1 / 0 } catch {}; $? was meant to show that in the terminating-error realm checking $? after try / catch is virtually pointless, because what $? is set to depends on whether the catch block happens to be empty ($False) or not (whatever statement happens to execute last in the catch block determines the value of $?).

Taking another step back: $? is problematic in other contexts too:

@Jaykul
Copy link
Contributor Author

Jaykul commented Aug 20, 2017

I think we're in agreement.

My request is focused on ErrorVariable only because I was focused on error detection in a script.

That is, I wasn't concerned with PowerShell.Invoke or any of the rest of this ;-)

In general, I think it's useless to check $Error in scripts because it's persistent through a whole session, and you don't normally care about errors that happened before your code. Of course, there are lots of ways (not least -ErrorAction SilentlyContinue) that errors show up in $Error and the user doesn't care, but since you're not normally using it, it doesn't matter the same way ErrorVariable does.

For what it's worth, I would agree that exceptions that are handled in a catch could be left out of $Error as well as ErrorVariable...

@microsoft-github-policy-service microsoft-github-policy-service bot added Resolution-No Activity Issue has had no activity for 6 months or more labels Jan 4, 2024
Copy link
Contributor

This issue has not had any activity in 6 months, if there is no further activity in 7 days, the issue will be closed automatically.

Activity in this case refers only to comments on the issue. If the issue is closed and you are the author, you can re-open the issue using the button below. Please add more information to be considered during retriage. If you are not the author but the issue is impacting you after it has been closed, please submit a new issue with updated details and a link to this issue and the original.

1 similar comment
Copy link
Contributor

This issue has not had any activity in 6 months, if there is no further activity in 7 days, the issue will be closed automatically.

Activity in this case refers only to comments on the issue. If the issue is closed and you are the author, you can re-open the issue using the button below. Please add more information to be considered during retriage. If you are not the author but the issue is impacting you after it has been closed, please submit a new issue with updated details and a link to this issue and the original.

@MatejKafka
Copy link

@kilasuit Please reopen, the issue is unresolved, but triaged and up-for-grabs.

@kilasuit kilasuit added KeepOpen The bot will ignore these and not auto-close and removed Resolution-No Activity Issue has had no activity for 6 months or more labels May 20, 2024
@kilasuit kilasuit reopened this May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Enhancement the issue is more of a feature request than a bug Issue-Question ideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a KeepOpen The bot will ignore these and not auto-close Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors WG-Engine core PowerShell engine, interpreter, and runtime
Projects
Developer Experience/SDK
  
Awaiting triage
Development

No branches or pull requests

9 participants