PowerShell / PowerShell

PowerShell for every system!
https://microsoft.com/PowerShell
MIT License
43.66k stars 7.08k forks source link

exit status `0` on syntax errors, when reading scrip from stdin #21784

Closed philcerf closed 1 week ago

philcerf commented 3 weeks ago

Prerequisites

Steps to reproduce

Hey.

This also seems to go back to the days of powershell.exe and seems particular disturbing.

When running a script with a deliberate syntax error (the -EQfoo):

$ErrorActionPreference = "Stop"; try{ $null -EQfoo $null} catch {exit 44}

like this:

$ pwsh -NonInteractive -NoProfile -Command '$ErrorActionPreference = "Stop"; try{ $null -EQfoo $null} catch {exit 44}' ; echo exit status: $?

The exit status returned by PowerShell is non-zero (as it should be):

ParserError: 
Line |
   1 |  $ErrorActionPreference = "Stop"; try{ $null -EQfoo $null} catch {exit …
     |                                              ~~~~~~
     | Unexpected token '-EQfoo' in expression or statement.
exit status: 1

When running it however via stdin like so:

$ { pwsh -NonInteractive -NoProfile -Command - <<'END'
$ErrorActionPreference = "Stop"; try{ $null -EQfoo $null} catch {exit 44}

END
} ; echo exit status: $?

The exit status is 0:

ParserError: 
Line |
   1 |  $ErrorActionPreference = "Stop"; try{ $null -EQfoo $null} catch {exit …
     |                                              ~~~~~~
     | Unexpected token '-EQfoo' in expression or statement.
exit status: 0

Expected behavior

use non-zero exit status on syntax error, even when command is read from stdin

Actual behavior

uses 0 as exit status

Error details

No response

Environment data

N/A

Visuals

No response

rhubarb-geek-nz commented 3 weeks ago

The good news is I can just refer back to 21808 for much of this.

My explanations often start with PowerShell is not a UNIX shell. UNIX programs are pipe/stream based, PowerShell is record based.

Two more details are significant

Lets see this in action

pwsh -NonInteractive -NoProfile -Command '-' <<'END'
$ErrorActionPreference = "Stop"; try{ $null -EQfoo $null} catch {exit 44}
$ErrorActionPreference
END

with the result

ParserError:
Line |
   1 |  $ErrorActionPreference = "Stop"; try{ $null -EQfoo $null} catch {exit …
     |                                              ~~~~~~
     | Unexpected token '-EQfoo' in expression or statement.
Continue

What you see is that none of the first record was executed. The assignment of $ErrorActionPreference never happened. So that record failed and on to the next record which shows the current state of $ErrorActionPreference.

mklement0 commented 3 weeks ago

Fundamentally, a parser error cannot be handled with runtime constructs such as $ErrorActionPreference = "Stop" and try / catch.

It is the statement-by-statement, REPL-like processing exhibited by -Command - that isolates a parser error to a given statement, but the fact that a parser error in the last statement doesn't result in a nonzero exit code should indeed be considered a bug, as argued in detail in https://github.com/PowerShell/PowerShell/issues/21808#issuecomment-2103258649

philcerf commented 3 weeks ago

@rhubarb-geek-nz Well I guess my reply would be mostly what I've wrote over in the other ticket, i.e. that it's at least undocumented, that - for -Command and -File make them behave completely differently than with values other than -... but more practically spoken: it seems not so good design with, to have a single option, e.g. -Command behave so dramatically different just by changing the source of the data.

If there'd really be a need for a mode of that semi-interactive parsing, it would IMO be better to have separate option for that and not for -Command which behaves just as expected with real commands given.

@mklement0 My main point here wasn't about the try not working (which I think is obvious in case of a syntax error), it was about 0 being returned despite the syntax error... so a caller has no chance of knowing that it didn't work.

mklement0 commented 3 weeks ago

so a caller has no chance of knowing that it didn't work.

Yes. That's why I said that what you've uncovered should indeed be considered a bug.

SeeminglyScience commented 1 week ago

The Interactive WG discussed this. The feature -Command - is designed to imitate a REPL, so the behavior is by design. While we understand and agree that this feature should have been appropriately named to indicate the functionality it offers (which is unfortunately different than how most applications handle -), we cannot change this now.

mklement0 commented 1 week ago

@philcerf, to bring closure to the specific aspect of the -Command - behavior that prompted this issue (the exit code):

As it turns out, the observed behavior is actually consistent with the pseudo-interactive, REPL-like behavior (which has just been declared to be by design), and the reason is now mentioned in the doc-update PR that @sdwheeler has since submitted:

A simple example:

@'
# If you uncomment the following statement, $LASTEXITCODE will be 1, because it is then
# this command's $? value that determines the exit code.
# 1 / 0

& {
  1 /  # This syntactically broken command has NO impact on the exit code.
}

'@ | pwsh -NoProfile -Command -; $LASTEXITCODE  # -> 0(!)

It follows that if you use the workaround to make -Command - behave like a script (enclose all code in & { ... } and append an extra newline), $LASTEXITCODE will always be 0 if the code is syntactically broken.


While the -Command behavior is consistent with the interactive experience it replicates, you could argue that it would make more sense to make parsing errors cause $? to reflect $false too, not just runtime errors.

While this wouldn't be much of an improvement in a true interactive session, it would help in the -Command - case when -Command - is used with the behave-like-a-script workaround.

philcerf commented 1 week ago

TBH, I kinda stopped following these issues a bit, because the general way they seem to be dealt with is: defining a completely broken behaviour as defective by design and thus allegedly valid.

I mean for me it's not really a big problem, since I simply pass my commands now as a multiline strings argument to -Command and with that, behaviour seems to be more or less what one would expect and what makes sense.

But I guess more people will fall into the traps involved when commands are read (non-interactively) from stdin with a behaviour that probably makes no sense in any situation.

Anyway, thanks for the workaround suggestions, but I rather go by my multiline string argument, because there's always the danger that there's more weird behaviour with the semi-interactive mode which no one can reasonably expect - I mean I found already a handful in just a few days.

Thanks, Philippe.

microsoft-github-policy-service[bot] commented 1 week ago

This issue has been marked as by-design and has not had any activity for 1 day. It has been closed for housekeeping purposes.

microsoft-github-policy-service[bot] commented 1 week ago

📣 Hey @philcerf, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

Microsoft Forms
mklement0 commented 3 days ago

@philcerf, yes, using -Command with the code as a direct string argument is the safest choice.

The current behavior does have its uses, but they're exotic and to me they never should have been exposed via -File - or -Command -, given the different behavior that users justifiably expect (at least in the -File - case; from the read-all-input-and-treat-it-as-a-script expectation, there's no need to also support -Command -).

I just remembered that there was a discussion about providing a new CLI entry point under a different name, say, pwsh-np, originally prompted by not wanting to load profiles by default (another notably deviation from what POSIX-compatible shells do).

Such a new CLI entry point would be an opportunity to address a number of issues with the current CLI, compiled in https://github.com/PowerShell/PowerShell/discussions/23872 (which was just converted to a discussion from #8072, so I'm not holding my breath), to which I've just added the issue at hand.

GitHub
Make the new, future no-profile CLI for use in shebang lines implement other CLI fixes too · PowerShell PowerShell · Discussion #23872
Follow-up from a conversation with @SteveL-MSFT in #7989. PowerShell's existing CLI has a number of problems, relating to: (a) unconditionally dot-sourcing $PROFILE, making for unpredictable execut...