-
Notifications
You must be signed in to change notification settings - Fork 123
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
Native Command Error Handling #261
Conversation
Co-Authored-By: Steve Lee <[email protected]>
Co-authored-by: James Truher [MSFT] <[email protected]> Co-authored-by: Robert Holt <[email protected]>
Co-authored-by: James Truher [MSFT] <[email protected]> Co-authored-by: Joey Aiello <[email protected]>
Comments Due: 10/31/2020 | ||
--- | ||
|
||
# Native Command Error Handling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see the PS team dogfood the implementation of this feature by using it instead of the Start-NativeExecution function used in the PS build module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was one of the motivations of doing this feature so we can remove that helper function
from a list. This default behavior is controlled with the preference variable | ||
`$ErrorActionPreference` default of `Continue`. | ||
|
||
In production, often customers prefer that script execution stops when a non-terminating error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In production, often customers prefer that script execution stops when a non-terminating error | |
In production, often customers prefer that script execution stops when any error, including non-terminating errors, |
|
||
## Alternative Approaches and Considerations | ||
|
||
One way of checking for a single native command and handling its exit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include that on Windows, many native commands do not strictly follow the non-zero is error, so we may add a special list to handle those commands differently where known non-zero exit code is not treated as an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SteveL-MSFT, I suggest we not do that, because it adds complexity while providing little benefit: the "rogue" CLIs fall into two categories:
-
True rogue CLIs that simply ignore exit codes and always return
0
- There's nothing we can generally do to compensate for that, short of - brittlely - trying to analyze stderr output, which I don't think is worth doing.
-
CLIs such as
robocopy.exe
that repurpose specific nonzero exit codes to communicate additional information about success conditions (while also using (other) nonzero exit codes to communicate failure). -
These exit codes are CLI-specific, and the user needs to know about them and handle them; while we could hard-code exceptions for the nonzero exit codes that signal success, the problems are:
- Do we really want to code such exceptions for all utilities that ship with Windows (assuming there are others - I don't know)?
- Users may expect the same magic to apply to all utilities, whether in-box or not.
- Maintenance burden: What if utilities later introduce additional nonzero exit codes that aren't failures?
Instead, I think we should make it as easy as possible to ignore a nonzero exit code ad hoc, which can already be done, though it's not the most obvious solution:
# Call robocopy and ignore any failure that a nonzero exit code may trigger.
robocopy ... || @()
# ... now analyze $LASTEXITCODE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
msiexec has two values that indicate success, but that a reboot is required (1641 and 3010), along with 0 also indicating success. That's the other one that I run into constantly (besides robocopy).
Discussed here: https://docs.microsoft.com/en-us/windows/win32/msi/error-codes
I'm sure there are more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @essentialexch, that's good to know.
However, instead of getting into the hard-coded exception business, I suggest documenting such high-profile exceptions as part of a conceptual help topic dedicated to external-program calls: see MicrosoftDocs/PowerShell-Docs#5152
|
||
Native commands usually return an exit code to the calling application which will be zero for | ||
success or non-zero for failure. However, native commands currently do not participate in the | ||
PowerShell error stream. Redirected `stderr` output is not interpreted the same as the PowerShell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stderr not being interpreted as an error by $ErrorActionPreference was a new experimental feature added to 7.1
stale. | ||
|
||
In POSIX shells, this need to terminate on command error is addressed by the `set -e` configuration, | ||
which causes the shell to exit when a command fails. In addition, to ensure that an error is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which causes the shell to exit when a command fails. In addition, to ensure that an error is | |
which causes the shell to exit when a command fails due to non-zero exit code. In addition, to ensure that an error is |
|
||
In POSIX shells, this need to terminate on command error is addressed by the `set -e` configuration, | ||
which causes the shell to exit when a command fails. In addition, to ensure that an error is | ||
returned if any command in a pipeline fails, POSIX shells address this need with `set -o pipefail` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to have an example script showing the difference in behavior between pipefail and w/o pipefail
| Source: | The full path to the application | ||
| ProcessInfo | details of failed command including path, exit code, and PID | ||
|
||
## Alternative Approaches and Considerations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add that additional configuration may be enabled to disable pipefail if there is a customer need for it
- For non-zero exit codes and except for the value `Ignore`, an `ErrorRecord` will be added to | ||
`$Error` that wraps the exit code and the command executed that returned the exit code. | ||
- Initially, only the existing values of `$ErrorActionPreference` will be supported. | ||
- The set of values may be extended later to include `MatchErrorActionPreference`, which should |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should move this to Considerations section
| Value | Definition | ||
---------------- | ------------------- | ||
| Break | Enter the debugger when an error occurs or when an exception is raised. | ||
| Continue | (Default) - Displays the error message and continues executing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a mismatch here between Continue being default and line 83
| Continue | (Default) - Displays the error message and continues executing. | ||
| Ignore | Suppresses the error message and continues to execute the command. | ||
| Inquire | Displays the error message and asks you whether you want to continue. | ||
| SilentlyContinue| No effect. The error message isn't displayed and execution continues without interruption. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really "no effect" as the error record is added to $Error
| Inquire | Displays the error message and asks you whether you want to continue. | ||
| SilentlyContinue| No effect. The error message isn't displayed and execution continues without interruption. | ||
| Stop | Displays the error message and stops executing. In addition to the error generated, the Stop value generates an ActionPreferenceStopException object to the error stream. stream | ||
| Suspend | Automatically suspends a workflow job to allow for further investigation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suspend is not supported as it's only for workflows
execution status after each call using a helper function similar to below: | ||
|
||
```Powershell | ||
if ($LASTEXITCODE -ne 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth mentioning the v7+ alternative based on the pipeline-chain operators:
# Note the need for $(...)
ls /no/such/path || $(throw "Command failed. See above errors for details")
| Ignore | Suppresses the error message and continues to execute the command. | ||
| Inquire | Displays the error message and asks you whether you want to continue. | ||
| SilentlyContinue| No effect. The error message isn't displayed and execution continues without interruption. | ||
| Stop | Displays the error message and stops executing. In addition to the error generated, the Stop value generates an ActionPreferenceStopException object to the error stream. stream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: "stream" is doubled.
---------------- | ------------------- | ||
| ExitCode: | The exit code of the failed command. | ||
| ErrorID: | `"Program {0} ended with non-zero exit code {1}"`, with the command name and the exit code, from resource string `ProgramFailedToComplete`. | ||
| ErrorCategory: | `ErrorCategory.NotSpecified`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it's worth creating a specific category for this, such as NativeFailure
- not sure how adding an enum
value would affect backward compatibility (at least in PowerShell code it shouldn't).
support all cases as `$?` can be false from a cmdlet or function error, making `$LASTEXITCODE` | ||
stale. | ||
|
||
In POSIX shells, this need to terminate on command error is addressed by the `set -e` configuration, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is implied by the current revision, but let me spell it out to see if my understanding is correct:
-
Unlike
bash
'sset -e
, PowerShell will not (and should not) make the failure detection dependent on whether or not it is used in a conditional (e.g., as part of anif
statement).- Unlike in POSIX-like shells, where conditionals act on exit codes, PowerShell's conditionals act on data, so it make sense to treat failure detection separately and to thereby avoid the rule complexity surrounding
set -e
.
- Unlike in POSIX-like shells, where conditionals act on exit codes, PowerShell's conditionals act on data, so it make sense to treat failure detection separately and to thereby avoid the rule complexity surrounding
-
Like
bash
'sset -e
, PowerShell will bypass failure detection if a failure occurs in a non-final segment of a pipeline chain, so as to allow overriding$PSNativeCommandErrorAction
ad hoc.
Examples:
$PSNativeCommandErrorAction = 'Stop'
# SHOULD fail, if `ls` reports a nonzero exit code.
if (-not ($name = /bin/ls nosuch.txt)) {
$name = 'default.txt'
}
# Should NOT fail, because || is used to explicitly handle the failure:
/bin/ls nosuch.txt || 'default.txt'
Closing as this is superceded by #277 |
No description provided.