Native Command Error Handling #277

theJasonHelmick · 2021-02-09T16:06:27Z

Link to original: #261

…into jch-natcmderr

1-Draft/RFC00XX-Native-Command-Error-Handling.md

Co-authored-by: James Truher [MSFT] <[email protected]>

SteveL-MSFT · 2021-03-24T22:26:05Z

1-Draft/RFC00XX-Native-Command-Error-Handling.md

+
+### set -e/ $ErrorActionPreference
+
+In the example below, `set -e` is not equivalent to `$ErrorActionPReference` for native commands.


Suggested change

In the example below, `set -e` is not equivalent to `$ErrorActionPReference` for native commands.

In the example below, `set -e` is not equivalent to `$ErrorActionPreference` for native commands.

SteveL-MSFT · 2021-03-24T22:31:03Z

1-Draft/RFC00XX-Native-Command-Error-Handling.md

+/bin/cat ./nofile
+/bin/echo "Message After failure"
+/bin/echo ""
+/bin/echo "With set -e : Will NOT receive message after failure"


should say "Will NOT continue script execution after failure", apply globally

SteveL-MSFT · 2021-03-24T22:35:31Z

1-Draft/RFC00XX-Native-Command-Error-Handling.md

+```bash
+bash-3.2$ cat file
+#!/bin/bash
+/bin/echo "Without set -o pipefail : returns 0"


Need to clarify what 0 and 1 means for those not familiar

SteveL-MSFT · 2021-03-24T22:43:02Z

1-Draft/RFC00XX-Native-Command-Error-Handling.md

+
+| Property        | Definition
+----------------  | -------------------
+| ExitCode:      |  The exit code of the failed command.


some alignment issues with vertical bars

ForNeVeR · 2021-03-28T06:06:28Z

In Bash, it is possible to temporarily disable set -e by using command || true, e.g.

set -e
cat ./nonexistent || true
echo "Test" # will be written

From the specification, it is unclear whether a similar trick will work with $PSNativeCommandErrorAction. I believe it should be written explicitly.

rkeithhill · 2021-03-29T05:34:42Z

@ForNeVeR you would do this:

$PSNativeCommandErrorAction = $true
& {
    $PSNativeCommandErrorAction = $false
    cat ./nonexistent
    "Test" # will be written
}

cat ./nonexistent
"Will not get output"

The disabling of $PSNativeCommandErrorAction will only be valid for that scriptblock.

@theJasonHelmick When can we get this? Please let this ship in 7.2 - at least as an experimental feature!!! Maybe I should submit a PR to remove Start-NativeExecution from the PowerShell build scripts. I think the team isn't feeling the pain as much as the rest of us in the community. :-)

joeyaiello · 2021-03-31T22:50:22Z

@PowerShell/powershell-committee reviewed this and are okay to move forward with an experimental implementation based on this RFC.

However, it's not currently committed for 7.2. As such, we're adding the up-for-grabs label.

…Preference

rkeithhill · 2021-04-01T18:09:30Z

1-Draft/RFC00XX-Native-Command-Error-Handling.md

@@ -282,8 +282,8 @@ The reported error record object will be the new type: `NativeCommandException`
 ### Preference variable resolves error handling conflicts

 In cases where an existing script already handles non-zero native command errors, the preference
-variable `$PSNativeCommandErrorAction` may be set to `$false`. Error handling behavior of the
-PowerShell cmdlets is handled separately with `$ErrorActionPreference`.
+variable `$PSNativeCommandUseErrorActionPreference` may be set to `$false`. Error handling behavior


Making this name longer seems like the wrong direction to go. Also, if it ends in ActionPreference then I would expect it to be of type [System.Management.Automation.ActionPreference] instead of [bool].

I agree that the variable being boolean and the name ending with Preference sounds weird. The name is so long that Use gets lost while reading. PSNativeCommandErrorActionEnabled, PSEnableNativeCommandErrorAction may better highlight the implicit type of the variable. Still they are long names, though.

rkeithhill · 2021-07-18T07:06:09Z

1-Draft/RFC00XX-Native-Command-Error-Handling.md

+| Property        | Definition
+----------------  | -------------------
+| 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`.


One thing that concerns me is that we're kind of all over the place on terminology: NativeCommand, Program and the official get-command type is Application. That said, I'm not sure I like Application by itself e.g. Application "whoami.exe" ended with non-zero exit code 1. Application doesn't seem to be appropriate for cli utilities like whoami. Native command "whoami.exe" ended with non-zero exit code 1 seems OK.

rkeithhill · 2021-07-18T18:32:18Z

1-Draft/RFC00XX-Native-Command-Error-Handling.md

+| ErrorCategory:  | `ErrorCategory.NotSpecified`.
+| object:         | Exit code
+| Source:         | The full path to the application
+| ProcessInfo     | Details of failed command including path, exit code, and PID


I think it would be helpful to see the arguments a a string. Or maybe rather than command path, show the "command line". Thoughts?

Is the process ID of any use given that the process no longer exists?

I think it would be helpful to see the arguments a a string. Or maybe rather than command path, show the "command line". Thoughts?

This is not guaranteed after terminating of the process - an attempt to read a property of the Process class is likely to raise an exception. We can report reliably only current script line string or ProcessInfo as is.

Is the process ID of any use given that the process no longer exists?

This can be useful as correlation data with event logs.

After talking this over with wg-engine group we decided that process id is useful for log correlation and arguments should not be recorded because it make contain passwords / api tokens.

rkeithhill · 2021-07-23T06:02:06Z

@theJasonHelmick The RFC does not mention anything about this being an experimental feature. I don't think that it needs to be experimental since it is "opt-in" via setting $PSNativeCommandUseErrorActionPreference to $true. Just wanted to get clarification on this. Do you agree?

rkeithhill · 2021-07-24T06:54:49Z

@theJasonHelmick Have you seen this ApplicationFailedException? This covers the case where a system exception is thrown when PS tries to run the application. My concern is that someone looking at the code base may be confused by seeing ApplicationFailedException and NativeCommandException.

joeyaiello · 2021-07-27T19:05:54Z

@rkeithhill even as an opt-in feature, it still needs to be marked as an experimental feature to denote to end-users that the design is not final and may change in the future.

Ideally, it should be code-fenced such that the opt-in isn't possible without the experimental feature turned on, but if the Maintainers agree with you that it's prohibitively difficult to do that, it should still have a placeholder experimental feature (where turning it off would do nothing) to denote the potential instability of the interface.

joeyaiello · 2021-07-27T19:13:45Z

@rkeithhill agreement that, especially given what's in the docs today for ApplicationFailedException ("Defines the exception that is thrown if a native command fails", it might make sense to rename NativeCommandException to something else.

@SteveL-MSFT is suggesting NativeCommandExitException, but we're open to other possibilities.

We're also referring this RFC to the Engine WG for review.

iSazonov · 2021-07-28T03:27:10Z

1-Draft/RFC00XX-Native-Command-Error-Handling.md

+
+- When enabled (1), `$ErrorActionPreference` will affect native commands with described behavior.
+- When disabled (0), `$ErrorActionPreference` will have no effect to the original behavior.
+- The default value is disabled (0) to maintain backward compatibility.


Suggested change

- The default value is disabled (0) to maintain backward compatibility.

- The default value is 0 (disabled) to maintain backward compatibility.

iSazonov · 2021-07-28T04:32:29Z

~~Why not re-use ApplicationFailedException if it is about exit?~~

daxian-dbw · 2021-08-20T18:37:23Z

1-Draft/RFC00XX-Native-Command-Error-Handling.md

+Simply relaying the errors through the error stream isn't the solution. The example itself doesn't
+support all cases as `$?` can be false from a cmdlet or function error, making `$LASTEXITCODE`
+stale.


The example itself doesn't support all cases

What example is it referring to? The if ($LASTEXITCODE -ne 0) { ... } one right above? If so, it's not clear to me how $? will affect that example, and can you maybe elaborate a bit more on how $LASTEXITCODE would be stale?

JamesWTruher · 2022-03-09T23:18:57Z

@theJasonHelmick - would you please confirm that the implemented experimental feature matches what is written here?

per-oestergaard · 2022-03-23T13:22:16Z

Is this now part of PowerShell as an experimental features (cannot find it in 7.2.2) or when will it come?

rkeithhill · 2022-03-23T15:18:34Z

It's in the current 7.3.0 preview - PSNativeCommandErrorActionPreference. After enabling the feature and restarting pwsh, try this:

whoami -xyzzy
get-date
& {
    $ErrorActionPreference = 'Stop'
    $PSNativeCommandUseErrorActionPreference = $true
    whoami -xyzzy
    get-date
}

jtmoon79 · 2023-12-02T07:49:17Z

Noted in this Stackoverflow Answer.

Added Native Command Error Handling

717787c

unfurl-links bot mentioned this pull request Feb 17, 2021

Cmdlet.ThrowTerminatingError() is not affected by -ErrorAction PowerShell/PowerShell#14819

Closed

theJasonHelmick added 2 commits March 22, 2021 09:09

Merge branch 'master' of https://github.com/PowerShell/PowerShell-RFC …

1fa25ed

…into jch-natcmderr

updated demo.txt and spec

5d04d21

JamesWTruher reviewed Mar 22, 2021

View reviewed changes

theJasonHelmick and others added 11 commits March 23, 2021 08:06

Update 1-Draft/RFC00XX-Native-Command-Error-Handling.md

a985804

Co-authored-by: James Truher [MSFT] <[email protected]>

Update 1-Draft/RFC00XX-Native-Command-Error-Handling.md

c03f3df

Co-authored-by: James Truher [MSFT] <[email protected]>

Update 1-Draft/RFC00XX-Native-Command-Error-Handling.md

1a38126

Co-authored-by: James Truher [MSFT] <[email protected]>

Update 1-Draft/RFC00XX-Native-Command-Error-Handling.md

d29e4c3

Co-authored-by: James Truher [MSFT] <[email protected]>

Update 1-Draft/RFC00XX-Native-Command-Error-Handling.md

77f46e2

Co-authored-by: James Truher [MSFT] <[email protected]>

Update 1-Draft/RFC00XX-Native-Command-Error-Handling.md

bef6778

Co-authored-by: James Truher [MSFT] <[email protected]>

Update 1-Draft/RFC00XX-Native-Command-Error-Handling.md

fce539d

Co-authored-by: James Truher [MSFT] <[email protected]>

updated post Jim review

e274268

Apply suggestions from code review

177a002

Co-authored-by: James Truher [MSFT] <[email protected]>

cleaned demo.txt

98a1055

added bash/powershell demo clarity

ac7edb8

joeyaiello mentioned this pull request Mar 24, 2021

Native Command Error Handling #261

Closed

SteveL-MSFT reviewed Mar 24, 2021

View reviewed changes

theJasonHelmick added 2 commits March 27, 2021 10:40

improved demo.txt

15f408d

changed $PSNativeCommandErrorAction to boolean

84b263b

joeyaiello added the Committee-Reviewed label Mar 31, 2021

joeyaiello added the Up-for-Grabs label Mar 31, 2021

changed $PSNativeCommandErrorAction to $PSNativeCommandUseErrorAction…

6fb05cd

…Preference

rkeithhill reviewed Apr 1, 2021

View reviewed changes

b-rad15 mentioned this pull request Jun 29, 2021

Equivalent of bash set -e PowerShell/PowerShell#3415

Closed

rkeithhill mentioned this pull request Jul 11, 2021

Initial impl of RFC-277 - native exe throws on non-zero exit code PowerShell/PowerShell#15757

Closed

22 tasks

rkeithhill reviewed Jul 18, 2021

View reviewed changes

joeyaiello added Experimental - Approved This RFC has been approved for a code submission as an experimental feature in PS/PS and removed Up-for-Grabs labels Jul 27, 2021

joeyaiello added WG-DevEx-Portability authoring cross-platform or cross-architecture modules, cmdlets, and scripts WG-Engine core PowerShell engine, interpreter, and runtime labels Jul 27, 2021

iSazonov reviewed Jul 28, 2021

View reviewed changes

rjmholt mentioned this pull request Aug 9, 2021

Implement RFC 277: native error throwing PowerShell/PowerShell#15897

Merged

22 tasks

daxian-dbw requested changes Aug 20, 2021

View reviewed changes

oblivioncth mentioned this pull request May 25, 2022

Windows auto builds don't terminate when an external process fails oblivioncth/Qx#20

Open

ThomasNieto mentioned this pull request Jul 29, 2023

PSNativeCommandErrorActionPreference non-zero 0 exit code exception false positive PowerShell/PowerShell#20034

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Native Command Error Handling #277

Native Command Error Handling #277

theJasonHelmick commented Feb 9, 2021

SteveL-MSFT Mar 24, 2021

SteveL-MSFT Mar 24, 2021 •

edited

Loading

SteveL-MSFT Mar 24, 2021

SteveL-MSFT Mar 24, 2021

ForNeVeR commented Mar 28, 2021

rkeithhill commented Mar 29, 2021 •

edited

Loading

joeyaiello commented Mar 31, 2021 •

edited

Loading

rkeithhill Apr 1, 2021

ceztko Apr 9, 2021 •

edited

Loading

rkeithhill Jul 18, 2021 •

edited

Loading

rkeithhill Jul 18, 2021

rkeithhill Jul 20, 2021

iSazonov Jul 28, 2021 •

edited

Loading

rkeithhill Aug 3, 2021

rkeithhill commented Jul 23, 2021

rkeithhill commented Jul 24, 2021

joeyaiello commented Jul 27, 2021 •

edited

Loading

joeyaiello commented Jul 27, 2021

iSazonov Jul 28, 2021

iSazonov commented Jul 28, 2021 •

edited

Loading

daxian-dbw Aug 20, 2021

JamesWTruher commented Mar 9, 2022

per-oestergaard commented Mar 23, 2022

rkeithhill commented Mar 23, 2022

jtmoon79 commented Dec 2, 2023


		### set -e/ $ErrorActionPreference

		In the example below, `set -e` is not equivalent to `$ErrorActionPReference` for native commands.

	- The default value is disabled (0) to maintain backward compatibility.
	- The default value is 0 (disabled) to maintain backward compatibility.

Native Command Error Handling #277

Are you sure you want to change the base?

Native Command Error Handling #277

Conversation

theJasonHelmick commented Feb 9, 2021

Choose a reason for hiding this comment

SteveL-MSFT Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ForNeVeR commented Mar 28, 2021

rkeithhill commented Mar 29, 2021 • edited Loading

joeyaiello commented Mar 31, 2021 • edited Loading

Choose a reason for hiding this comment

ceztko Apr 9, 2021 • edited Loading

Choose a reason for hiding this comment

rkeithhill Jul 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iSazonov Jul 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rkeithhill commented Jul 23, 2021

rkeithhill commented Jul 24, 2021

joeyaiello commented Jul 27, 2021 • edited Loading

joeyaiello commented Jul 27, 2021

Choose a reason for hiding this comment

iSazonov commented Jul 28, 2021 • edited Loading

Choose a reason for hiding this comment

JamesWTruher commented Mar 9, 2022

per-oestergaard commented Mar 23, 2022

rkeithhill commented Mar 23, 2022

jtmoon79 commented Dec 2, 2023

SteveL-MSFT Mar 24, 2021 •

edited

Loading

rkeithhill commented Mar 29, 2021 •

edited

Loading

joeyaiello commented Mar 31, 2021 •

edited

Loading

ceztko Apr 9, 2021 •

edited

Loading

rkeithhill Jul 18, 2021 •

edited

Loading

iSazonov Jul 28, 2021 •

edited

Loading

joeyaiello commented Jul 27, 2021 •

edited

Loading

iSazonov commented Jul 28, 2021 •

edited

Loading