pnp / powershell

PnP PowerShell
https://pnp.github.io/powershell
MIT License
680 stars 349 forks source link

[BUG] Connect-PnPOnline does not respect ErrorAction parameter #3683

Open rgsteele opened 10 months ago

rgsteele commented 10 months ago

Reporting an Issue or Missing Feature

Issue (migrated from https://github.com/pnp/PnP-PowerShell/issues/2196)

Expected behavior

Connect-PnPOnline -ErrorAction Stop stops script execution on error, e.g. when the URL is wrong.

Actual behavior

The -ErrorAction parameter is being ignored, script execution continues.

Steps to reproduce behavior

Connect-PnPOnline to a bogus tenant url

What is the version of the Cmdlet module you are running?

2.2.0

Which operating system/environment are you running PnP PowerShell on?

Studermarc commented 9 months ago

There is the -ValidateConnection parameter which provides this feature.

It is described in the Documentation.

imbrish commented 9 months ago

I do not think -ErrorAction and -ValidateConnection work the same, for example when invalid credentials are given. I am using 1.12.0 in which -ErrorAction was supposedly added, but I do not think it works as expected.

Get-Content "foo/bar" # -ErrorAction Stop
Connect-PnPOnline -Url "https://foo.bar" -Credentials "foobar" -ErrorAction Stop
Get-PnPListItem -List "foobar" -ErrorAction Stop
Get-PnPFile -Url "foo/bar" -AsFile -Path "." -Filename "bar" -ErrorAction Stop
Write-Host "finished"

This will show 4 errors and "finished" message. If I uncomment the -ErrorAction in the first line, the script will terminate after the first error.

AndersRask commented 6 months ago

ErrorAction was not added in any version, it is integral part of every cmdlet/advanced function.

Some errors in PowerShell are terminating, some are not. By setting -ErrorAction Stop you ensure that the error will be terminating. Get-Content for example does not throw a terminating error if it cant find the file.

This is what often tricks people that wants error handling with try/catch. If the error is not terminating, it will not go into the catch and trigger your error handling!

For the case with Connect-PnPOnline, this was not treated as a "real" error, so you could not force the error into a catch to ensure your code flow. This was fixed when -ValidateConnection was added. Was it the right solution? I would have preferred a terminating error, and adding a new parameter also has issues for legacy code, but hey ho it works :)

https://devblogs.microsoft.com/scripting/understanding-non-terminating-errors-in-powershell/

imbrish commented 6 months ago

@AndersRask thanks for the explanation. I read the article you linked, and I guess I understand the difference between terminating and a non-terminating errors.

Compare the following script:

Get-InstalledModule -Name "PnP.PowerShell"
Connect-PnPOnline -Url "https://foo.bar" -Credentials "foobar" -ErrorAction Stop
Write-Host "finished"

image

And this one:

$ErrorActionPreference = "Stop"
Get-InstalledModule -Name "PnP.PowerShell"
Connect-PnPOnline -Url "https://foo.bar" -Credentials "foobar"
Write-Host "finished"

image

Note that the "finished" line is missing. Shouldn't -ErrorAction Stop and $ErrorActionPreference = "Stop" be equivalent?

See https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_commonparameters?view=powershell-7.4#-erroraction

Adding -ValidateConnection does not change anything.