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

Respect ErrorActionPreference universally #7774

Closed
chriskuech opened this issue Sep 13, 2018 · 19 comments
Closed

Respect ErrorActionPreference universally #7774

chriskuech opened this issue Sep 13, 2018 · 19 comments
Labels
Resolution-Duplicate The issue is a duplicate.

Comments

@chriskuech
Copy link

PowerShell has two types of errors: terminating and non-terminating. Non-terminating errors are handled consistently with common parameters and scoped $ErrorActionPreference. Terminating errors do not support this interface and instead rely on the programmer to determine that the error is non-terminating (usually after a period of confusion), and handle with separate but equivalent logic. Terminating errors are therefore only an inconvenience to advanced users and a source of significant confusion for new users.

All cmdlets should respect the ErrorActionPreference

@mklement0
Copy link
Contributor

For a complete overview of PowerShell's current error handling, see MicrosoftDocs/PowerShell-Docs#1583

@chriskuech
Copy link
Author

Arguably, -ErrorAction too should apply to all error types (as appropriate), though that would certainly be a breaking change.

This is precisely the subject of my feature request. Everyone I have seen encounter this behavior have thought they found a bug and all used strong language when I explained the underlying behavior. This behavior is extremely counter-intuitive and worth changing in the next major release, even if it's a breaking change.

@mklement0
Copy link
Contributor

mklement0 commented Sep 13, 2018

I hear you. I pointed you to the docs issue to get the full picture.

It's unclear if there will ever be a release that changes such fundamental behaviors, because the commitment to backward compatibility has been ironclad so far.

For a meta-discussion, see #6745

@iSazonov iSazonov added the Resolution-Duplicate The issue is a duplicate. label Sep 13, 2018
@BrucePay
Copy link
Collaborator

@chriskuech

All cmdlets should respect the ErrorActionPreference

Given that all cmdlets already respect ErrorActionPreference, can you elaborate on what you mean by this? Thanks.

@mklement0
Copy link
Contributor

@BrucePay:

You're right, the $ErrorActionPreference preference variable applies to all errors, against documented behavior:

Neither $ErrorActionPreference nor the ErrorAction common parameter affect how PowerShell responds to terminating errors (those that stop cmdlet processing).

PS> & { $ErrorActionPreference = 'SilentlyContinue'; Get-Item -NoSuchParam; 'Done' }
Done  # Statement-terminating error was silently ignored.

By contrast, the - ostensibly equivalent - command-scoped mechanism, the -ErrorAction common parameter, applies only to non-terminating errors, in line with the documentation:

# The error is still reported, because it is (statement-)terminating.
PS> & { WriteOutput -ErrorAction SilentlyContinue (Get-Item -NoSuchParam); 'Done' }
Get-Item : A parameter cannot be found that matches parameter name 'NoSuchParam'.
....
Done

If I understand @chriskuech correctly, his preference is for -ErrorAction to apply to terminating errors as well (whether statement- or -script-terminating), just as $ErrorActionPreference already does.

@BrucePay
Copy link
Collaborator

@mklement0

You're right, the $ErrorActionPreference preference variable applies to all errors,

I didn't say all errors, I said the preference variables applies to all cmdlets and @chriskuech was asserting that it did not. I was wondering what he meant by that. Still am.

I'm not sure I understand the example you included. Is this yours or is it from the documentation?

PS> & { $ErrorActionPreference = 'SilentlyContinue'; Get-Item -NoSuchParam; 'Done' }
Done  # Statement-terminating error was silently ignored.

There are three statement inside the scriptblock (';' separates statements). Only the statement with the statement-terminating error is terminated so of course 'Done' should run. Do you think something is wrong here?

the -ErrorAction common parameter, applies only to non-terminating errors, in line with the documentation:

Very specifically -ErrorAction applies to things that are written to the error pipeline. As I've explained previously, an error is neither terminating nor non-terminating. It's what you do with it that matters. If you want it to be nonterminating, write it to the error pipeline. If you want it to be terminating, throw it as an exception. It has been suggested that we add a property to the ErrorRecord to track the disposition though I'm uncertain how useful that will be since a single error record may be thrown, caught then written. As an aside, we do track if an exception was thrown from the throw statement:

PSCore (1:105) >  try { throw 'Yikes' } catch { $e = $_ }
PSCore (1:106) >  $e.Exception.WasThrownFromThrowStatement
True

but that's used internally by the engine for exception processing.

-ErrorAction to apply to terminating errors as well (whether statement- or -script-terminating), just as $ErrorActionPreference already does.

$ErrorActionPreference doesn't apply to terminating errors (exceptions). As for applying the existing preference variable/parameter to terminating errors, that actually makes no sense. The preference variables control disposition of the error. Applying the same variable setting to two distinct error "channels" with inherently different behaviours makes no sense.

@mklement0
Copy link
Contributor

mklement0 commented Sep 14, 2018

@BrucePay:

Let me just say that the gist of what's in the OP and his follow-up comment suggest that his concerns are about:

  • wishing that the -ErrorAction common parameter take effect for terminating errors too
  • and, more broadly, that the distinction between terminating and non-terminating may be unhelpful, given that it requires distinct ways of handling the error, and there's often confusion over what error type is issued when.

But enough speculation - I'll let @chriskuech respond.


Let's talk about terminology first:

an error is neither terminating nor non-terminating.

  • Non-terminating vs. terminating is the official nomenclature, and it makes sense, as it describes the default behavior when these errors occur.

  • Non-terminating errors by default do not affect the flow of execution; in a pipeline, they continue processing of further objects, if applicable.

  • As noted in the linked docs issue, a regrettable omission from the docs is the important subdivision of terminating errors into:

    • statement-terminating errors (which happen to be pipeline-terminating errors if the statement is (only) a pipeline); by default, these terminate the current statement only (albeit instantly) and execution continues, with the next statement.

    • script-terminating errors - a terminology we previously agreed upon (technically: a current-thread terminating error, as you've previously explained).

    • In other words: these two subtypes differ by what they terminate (the scope of the termination)

I'll be using these terms in the rest of this post.


I didn't say all errors

It is what I said, however, because it is true (though I should have be more precise: all types of runtime errors (as opposed to parse-time errors)), and it also encompasses your claim.
(As an aside: As previously discussed, preference variables such as $ErrorActionPreference are not honored by advanced functions from other modules - see #4568).


I'm not sure I understand the example you included. Is this yours or is it from the documentation?
Do you think something is wrong here?

The example, which I've created, demonstrates that $ErrorActionPreference = 'SilentlyContinue' is able to silence the terminating error that Get-Item -NoSuchParam generates - against documented behavior.

(The only reason I used & { ... } was to localize the effect of changing $ErrorActionPreference.)

If you want it to be terminating, throw it as an exception.

Get-Item -NoSuchParam is terminating - it is statement-terminating - and for that reason -ErrorAction SilentlyContinue can not be used to silence it, as the 2nd command demonstrates.

A statement-terminating error issued by a cmdlet is not the same as a script-terminating error generated with throw. As stated, this vital - potentially originally unintended, but de-facto extant - distinction is missing from the docs.


$ErrorActionPreference doesn't apply to terminating errors (exceptions)

It already does:

PS> & { $ErrorActionPreference = 'SilentlyContinue'; Throw 'a hissy fit'; 'Done' }
Done

As you can see, the Throw statement was happily ignored.


As for applying the existing preference variable/parameter to terminating errors, that actually makes no sense.

Again:

  • The preference variable is already being applied to terminating errors (of both subtypes).

  • The common parameter is not being applied to terminating errors (of both subtypes; with script-terminating errors only occurring in advanced functions using Throw).

This inconsistency, along with the lack of distinction between statement- and script-terminating errors in the docs have caused much, much confusion over the years.

@chriskuech
Copy link
Author

I assumed $ErrorActionPreference and -ErrorAction would have the same behavior, and in my opinion, the fact that they don't is even more confusing than the behavior I assumed.

The exact case that keeps coming up:

Invoke-WebRequest "bad-uri" -ErrorAction SilentlyContinue

Further, the word "terminating" is not present anywhere in the docs for this cmdlet and I assume the same is true of other cmdlets that throw terminating errors.

@GeeLaw
Copy link

GeeLaw commented Mar 15, 2019

In reply to @mklement0

You're right, the $ErrorActionPreference preference variable applies to all errors, against documented behavior:

Neither $ErrorActionPreference nor the ErrorAction common parameter affect how PowerShell responds to terminating errors (those that stop cmdlet processing).

PS> & { $ErrorActionPreference = 'SilentlyContinue'; Get-Item -NoSuchParam; 'Done' }
Done  # Statement-terminating error was silently ignored.

Get-Item -NoSuch produces a non-terminating error. You can imagine the engine working like this:

Function Invoke-Cmdlet
{
  If (NoSuchParam)
  {
    Write-Error 'No such param.'
  }
  Else
  {
    # actual stuff
  }
}

In other words, it is that the statement never gets executed because it's unclear how to do so, and it is not the case that the statement gets terminated.


Throwing a terminating error signals a severe error condition for which the continuation of the cmdlet isn't meaningful. For example, if you try to download and parse a JSON, it makes no sense to start parsing if the downloading didn't succeed, thus the terminating error. In other words, it is used by cmdlets to control their flow in addition to reporting to the upstream commands why they didn't complete normally. It's like writing the error and calling Break. (I'll return to this later) If some cmdlet stopped because of a terminating error, the error is passed to the caller and is not going to terminate the caller.

Let me restate the OP's question in a clearer way.

function x
{
[cmdletbinding()]param()
begin{
$PSCmdlet.ThrowTerminatingError(
[System.Management.Automation.ErrorRecord]::new(
[System.NotImplementedException]::new(),
'Not implemented.', 'NotSpecified', $null));
}
}

x -ea silentlycontinue; 'Done 1';
x; 'Done 2';
& { $ErrorActionPreference = 'silentlycontinue'; x -ea silentlycontinue; 'Done 3'; }
& { $ErrorActionPreference = 'silentlycontinue'; x; 'Done 4'; }

It makes no sense to continue the execution of that cmdlet, but it makes sense to decide by ErrorAction whether the terminating error is echoed or something.

Current behavior is equivalent to the following:

  1. Execute the cmdlet.
  2. If the cmdlet calls $PSCmdlet.ThrowTerminatingError(x), store x and terminate the cmdlet.
  3. Back in the caller's scope, call $PSCmdlet.WriteError(x). (This is not true if the caller isn't a PSCmdlet, but it serves the purpose of exposition.)

In other words, the terminating error is written to the error stream in the caller's scope, respecting the caller's $ErrorActionPreference. In the mean time, ErrorAction only controls how error records written by the callee behaves.

  • Current behavior: a terminating error is written to the stream in the caller's context.
  • What the OP wants: the terminating error is written to the stream in the callee's context.

As I said, intuitively, throwing a terminating error could be interpreted as calling WriteError and then break. However, it is now keep the error, call break, go back to the caller's scope, write error.

@mklement0
Copy link
Contributor

mklement0 commented Mar 15, 2019

Get-Item -NoSuch produces a non-terminating error.

It doesn't, it creates a statement-terminating error, which you can verify as follows:

PS> Get-Item -NoSuch -ErrorAction Ignore; 'Done'
Get-Item : A parameter cannot be found that matches parameter name 'nosuch'.
...
Done

That is, -ErrorAction was ignored from which you can infer that the error is not non-terminating. Since Done still printed, you can infer that it wasn't a script-terminating (thread-terminating) error either.

PS> (Get-Item -NoSuch) + '!'
Get-Item : A parameter cannot be found that matches parameter name 'NoSuch'.

That is, only the error message printed and not also !, which implies that the statement as a whole was terminated (contrast this with (Get-Item \nosuch\path) + '!', which does print !).

No one is disputing that it makes sense to not even attempt execution if a given command invocation's arguments are formally incorrect.


Thanks for the technical explanation for the current behavior, but what matters in the end is whether the behavior:

(a) makes sense to users: it shouldn't surprise them to begin with and should be simple enough to remember, so that it doesn't trip them up repeatedly.

To put it in @chriskuech's words again:

I assumed $ErrorActionPreference and -ErrorAction would have the same behavior, and in my opinion, the fact that they don't is even more confusing than the behavior I assumed.

(b) doesn't create unnecessary work for the typical use case.

It makes much more sense for a severe error condition to also be script-terminating by default.

@GeeLaw
Copy link

GeeLaw commented Mar 15, 2019

In other words, it is that the statement never gets executed because it's unclear how to do so, and it is not the case that the statement gets terminated.

By saying that, I meant "the error is non-terminating for the calling scope". Since the statement never gets executed, there isn't a child scope to talk about, whence my phrasing.

Could @mklement0 provide a reference to the term "statement-terminating error"? Inventing new jargons doesn't always help understanding the issue. Also, I am not sure how each participant in the thread understands the issue, so I provided my understanding. Put in a list,

  • The following phrases have the same meaning:
    • To write an ErrorRecord.
    • To produce a non-terminating error.
  • When an ErrorRecord is about to be written, how it behaves is specified by ErrorActionPreference in the current scope.
  • ErrorAction changes the ErrorActionPreference of the cmdlet's scope, which is a subscope of the caller's.
  • The following phrases have the same meaning (in current implementation):
    • To stop the current cmdlet and write an ErrorRecord in the scope in which the cmdlet terminated was called.
    • To throw a terminating error.
  • Based on the above paraphrasing: throwing a terminating error stops the cmdlet and produces a non-terminating error for the caller. It doesn't make sense to talk about terminating-ness of an ErrorRecord without specifying the scope.

The counter-intuitive behavior is due to the (arte-)fact that the writing happens in the calling scope, in which ErrorAction is out of effect.

In other words, I was conveying the idea that the current description of this issue is incorrect. It is not that terminating/non-terminating errors are treated differently. It is that the terminating error's ErrorRecord is written (as a non-terminating error) in the caller's scope.


I'd like to state my position on this issue clearly (so that no one has to guess my position from the verbose replies): I find the current behavior counter-intuitive and it should be changed. The proposed change is: Throwing a terminating is performed as if an ErrorRecord is written by the callee, after which the callee terminates.


The following is a technical analysis of how one could implement the change if it is to be done so.

The proposed change is to change the behavior from

$PSCmdlet.ThrowTerminatingError = Stop cmdlet + Write ErrorRecord in caller's scope.

to

$PSCmdlet.ThrowTerminatingError = Write ErrorRecord in the callee's context + Stop cmdlet.

I browsed the current implementation and did some code-path chasing. Here's what I found:

  • Calling ThrowTerminatingError will eventually throw an exception (throw as a C# statement). (See MshCommandRuntime.cs.)
  • The WasThrownFromThrowStatement corresponds to whether a RuntimeException was thrown by a PowerShell Throw statement. (Compiler.cs L4954; MiscOps.cs L1906)
  • The compiler uses a state machine to implement error handling. (See MiscOps.cs L1633; Compiler.cs L2789)
  • From the places mentioned in point 3, we can see that if the callee throws an exception due to ThrowTerminatingError, the control is transferred to the caller's compiled handler, which in turn handles the exception in the caller's context.

At this point, OP's wishes could be implemented by the following technical change:

  • Make ThrowTerminatingError do the following:
    1. Call WriteError.
    2. throw new TerminatingErrorThrownException(calleeException, calleeErrorAction).
  • Make the compiler compile statements with a trap like this:
int dispatchIndex = 0;
dispatchNextStmt:
try
{
    switch (dispatchIndex)
    {
        case 0: goto L0;
        case 1: goto L1;
        case 2: goto L2;
    }
    L0: dispatchIndex = 1; stmt1;
    L1: dispatchIndex = 2; stmt2;
}
catch (FlowControlException) { throw; }
catch (TerminatingErrorThrownException) { goto dispatchNextStmt; }
catch (Exception e)
{
    if (!(e is TerminatingErrorThrownException))
    {
        ExceptionHandlingOps.CheckActionPreference(functionContext, e);
    }
    goto dispatchNextStmt;
}
L2:

@GeeLaw
Copy link

GeeLaw commented Mar 15, 2019

Follow-up: after more reading of the docs, it is unclear to me what consists of a "pipeline" (which a terminating error terminates). Does every statement count as a pipeline? Does the whole invocation (line of interactive command) count as a pipeline? The correct behavior can only be determined if we know what a pipeline is.


Case 1: each statement counts as a pipeline

Then the following script should produce 2:

function x { [cmdletbinding()]param()
write-error 1 -ea stop;
}

& { $ErrorActionPreference = 'Continue'; x; 2; }

However, it doesn't. The written record is terminating the whole invocation.


Case 2: the whole invocation counts as a pipeline

This is highly undesirable, as it renders terminating error nearly unrecoverable.

In this case, ThrowTerminatingError creating what @mklement0 call "statement-terminating error" is a bug in the trapped code compiler -- it should have killed the whole invocation. ThrowTerminatingError should behave the same way Throw in PowerShell does. As for Get-Item -NoSuch, I still insist this is a non-terminating error. I suspect Invoke-WebRequest is giving terminating error due to buggy implementation.

@mklement0
Copy link
Contributor

Just a quick meta note, @GeeLaw:

Could @mklement0 provide a reference to the term "statement-terminating error"?

Please don't refer to me in the third person when I'm clearly a direct participant in a discussion - it's off-putting.

Inventing new jargons doesn't always help understanding the issue.

Please refrain from making such incendiary statements - they add nothing to the discussion and serve only to antagonize.

@GeeLaw
Copy link

GeeLaw commented Mar 15, 2019

In reply to @mklement0: My apologies if you find the comments offending. I used at because I felt tired of using "In reply to" and the jargon thing is expressing the preference to reason about the issue in PowerShell's documented terminology & ideas/views.

@mklement0
Copy link
Contributor

Thanks, @GeeLaw.

I felt tired of using "In reply to"

No need for that; simply @-mentioning is sufficient (and widely used, not just on GitHub); e.g.:

@mklement0 Can you provide a reference ...

Or, if you believe that punctuation saves lives:

@mklement0, can you provide a reference ...
@mklement0: Can you provide a reference ...


preference to reason about the issue in PowerShell's documented terminology & ideas/views

I fully agree with that preference; I took issue with how you expressed that preference. (That what prompted this expression of preference was based on a misconception is secondary - see later comments.)

@mklement0
Copy link
Contributor

@GeeLaw: Re terminology:

Generally:

  • (In full agreement with your sentiment:) If established terminology exists, it's always best to use it to avoid ambiguity.

  • If it doesn't, it's best to not only discuss the issue at hand, but to separately try to establish new technology, which - hopefully - will find its way into the official documentation.

(There are several cases where I feel that the lack of explaining things conceptually and/or giving concepts an official name in the official documentation has hindered understanding / adoption of certain features; to name a few: member enumeration, namespace variable notation, delay-bind script blocks (the latter were given that name only very recently).)

Specifically:

  • The official terms regarding error types are non-terminating error and terminating error.

  • This abstract dichotomy is insufficient, because there are clearly two subtypes of terminating errors, as exemplified by Throw 'outta here' and Get-Item -NoSuchParam.

  • A while back I wrote a comprehensive overview of PowerShell's de facto error-handling behavior, in which I also also asked for these new subtypes to be documented; the names I proposed were:

    • statement-terminating errors: they terminate the statement (as defined in the linked unit-of-execution terminology proposal) they're a part of.

    • script-terminating errors: they terminate the script as a whole (as @BrucePay has clarified, it is technically the thread of execution that is terminated, but we've agreed on the term script as being more end-user-friendly).

    • (I've also tried to give the terminology wider exposure in this Stack Overflow answer).

  • So, as you can see, these terms are the closest thing we currently have to official terminology with respect to error handling.

@mklement0
Copy link
Contributor

mklement0 commented Mar 16, 2019

P.S., @GeeLaw: By saying (emphasis added):

Get-Item -NoSuch produces a non-terminating error.

you did use official terminology - but you used it to mean something else - which prompted my clarification.

@mklement0
Copy link
Contributor

To get back on track, @GeeLaw.

  • I'm glad that @chriskuech, you, and I agree that the behavior should be changed.

  • Your technical analysis helped illuminate the underlying causes and it sounds like you're implementation suggestions will come in handy (I personally can't judge that).

    • Your analysis also explains why setting the preference variable as opposed to using the common parameter does result in a script-terminating error - which is currently documented incorrectly (from about_Preference_Variables, emphasis added):

Neither $ErrorActionPreference nor the ErrorAction common parameter affect how PowerShell responds to terminating errors (those that stop cmdlet processing).

@jzabroski
Copy link

See @KirkMunro mosaic on control flow variables: PowerShell/PowerShell-RFC#198

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution-Duplicate The issue is a duplicate.
Projects
None yet
Development

No branches or pull requests

6 participants