Fail on standard error option for azure powershell task #6324

arjgupta · 2018-01-31T14:42:06Z

This change is meant to introduce "Fail on Standard Error" option for the Azure PowerShell task. The default value of the option is true. This is because the current behaviour of the task is that if anything is written to the error stream ( terminating or non-terminating ), the task fails.
If the option is unchecked, then it should be possible to use write-error in the script without causing the task to fail.

… set to false and non-terminating errors are written to the error stream

arjgupta · 2018-02-01T09:18:29Z

for fixing the issue #5182

ericsciple · 2018-02-01T20:52:13Z

Tasks/AzurePowerShell/AzurePowerShell.ps1

@@ -105,7 +106,7 @@ try {
            ,$_

            # Set the task result to failed if the object is an error record.
-            if ($_ -is [System.Management.Automation.ErrorRecord]) {
+            if ($_ -is [System.Management.Automation.ErrorRecord] -and $failOnStandardError -eq $true) {


Have you considered adding two options like the latest version of the powerShell task? ErrorActionPreference and FailOnStderr?

Or trying to limit the change to actual stderr from external processes? We added the ignore stderr flag because folks had a hard time suppressing stderr from external tools.

Does the AzurePowerShell task have a different problem? What happens today if someone sets the error action preference to continue in their script. Does it always fail the task due to this line?

the current problem is that if a script writes a non-terminating error, or if a cmdlet writes a non-terminating error, the task fails. also, stderr from external commands is converted into error records. Today we set the erroractionpreference to 'continue' before we execute the script. So unless the user's script overrides it, the task always fails for non-terminating errors.

Ideally the options would be aligned with the powershell, so customers get a consistent experience.

In the new powershell task, I switched the default error action preference to 'Stop'. Although this is different from the powershell default, I think Stop generally aligns with customer scripting patterns (and they can override if needed). By setting to Stop, powershell will exit with a non-zero exit code if there is an error written to the error pipeline (and stderr will not interfere).

And then it's an explicit choice for the user to opt into failing on stderr. The key is, i run a separate powershell process for the user's script. So stderr means anything written to the error pipeline or any nested processes that wrote to the inherited stderr stream from powershell. Programs commonly write to stderr, so we definitely wanted failing on stderr to be opt-in.

The Azure Powershell task would need to major-version-rev'd to match exactly that behavior. Otherwise it would break compat since historically the task has always failed when any error is written to the error pipeline, and also when any nested process write to stderr.

So I think you have two options today without rev'ing the major version.

You could match the same options but with different defaults. I think you could achieve this by adding the current logic into a separate script, and invoke that as a separate process. Then you could monitor stderr from the outer process.

What you are doing here conflates stderr and error-action-preference. If you choose to move forward with this approach I would recommend being very clear in the help markdown that if they unset fail-on-stderr then they need to rely on bubbling a terminating error or exception to communicate task failure.

So for option no. 1, I'll have to add both error action preference and a fail on std error inputs, and they'll have the values 'stop' and true respectively ( this won't be an opt-in experience) so as to match the current behaviour. But how does starting a new process help in this? Currently the way both tasks(azureps and powershell) monitor the output stream, any error written by an external tool comes off as a non-terminating error record, and then whether or not the task fails depends on the Bool failonstderr (the last part being true only for the powershell task as of now). So if i handle the setting up of the error action preference for the user script, shouldn't the current changes work?

This statement is not true: Currently the way both tasks(azureps and powershell) monitor the output stream, any error written by an external tool comes off as a non-terminating error record

Often times when you run an external command (separate process), the external process inherits the STDOUT and STDERR streams. When the streams are inherited, the calling powershell process doesn't even know that anything was written to STDERR, and therefore no error record gets created. For example, paste this into a powershell.exe prompt:

& powershell.exe -noninteractive -command "[system.console]::Error.WriteLine('foo')"

and you will notice that you do not get an error record.

I'm mentally trying to walk through the cases of error-action-preference and fail-on-stderr to determine whether this change would be consistent with the current v2 powershell task...

I guess the only difference would be related to the same differences that exists today. When using the Azure PowerShell task, when an external processes writes to STDERR an error record will get created in powershell. This was an explicit choice because I migrated this task from the legacy handler to the powershell3 handler, and I explicitly held compat on that behavior. The legacy handler used the powershell engine rather than powershell.exe - and that is a behavioral difference of between the two.

So error-action=stop, fail-on-stderr=false, would behave differently between the two tasks, when an external command writes to STDERR. The powershell task would not create an error record and the task would succeed. The Azure PowerShell task would create a terminating error.

And also a subtle behavior difference for error-action=stop, fail-on-stderr=true. The powershell task would run to completion and fail due to stderr. The Azure PowerShell task would again create a terminating error.

Without bumping the major version, I think you want to hold compat on the behavior that causes stderr to create error records.

Let me know if you have all the info you need.

I think the differences that we are down to, are minimal and a result of holding back compat.

The differences are subtle, but it's also the subtle differences that causes customer confusion. It's the motivation for attempting to match the powershell.exe behavior. So customers aren't confused why it works one way on their machine, and then differently in vsts.

kmkumaran · 2018-03-13T07:26:18Z

Tasks/AzurePowerShell/AzurePowerShell.ps1

    # Trace the expression as it will be invoked.
    $__vstsAzPSInlineScriptPath = $null
    If ($scriptType -eq "InlineScript") {
+        $scriptArguments = $null
        $__vstsAzPSInlineScriptPath = [System.IO.Path]::Combine(([System.IO.Path]::GetTempPath()), ([guid]::NewGuid().ToString() + ".ps1"));


Should we be using Agent.TempDirectory instead of users temp path?

will update

kmkumaran · 2018-03-13T07:36:32Z

Tasks/AzurePowerShell/AzurePowerShell.ps1

-            if ($_ -is [System.Management.Automation.ErrorRecord]) {
-                "##vso[task.complete result=Failed]"
+            if($_ -is [System.Management.Automation.ErrorRecord]) {
+                if($_.FullyQualifiedErrorId -eq "NativeCommandError" -or $_.FullyQualifiedErrorId -eq "NativeCommandErrorMessage") {


Is this errorId localizable?

kmkumaran · 2018-03-13T07:38:13Z

Tasks/AzurePowerShell/AzurePowerShell.ps1

@@ -6,6 +6,8 @@ $scriptType = Get-VstsInput -Name ScriptType -Require
 $scriptPath = Get-VstsInput -Name ScriptPath
 $scriptInline = Get-VstsInput -Name Inline
 $scriptArguments = Get-VstsInput -Name ScriptArguments
+$__vsts_input_errorActionPreference = Get-VstsInput -Name errorActionPreference


why have a different naming convention just for these inputs?

erroractionpreference is a well known name. the other reason is that we remove the variables and functions from the current scope before executing the script, but we cannot remove these variables since we need them as the script is executing. And so, to avoid a conflict between these names and similar names defined in the user script, we use this scheme.

kmkumaran · 2018-03-13T07:45:20Z

Tasks/AzurePowerShell/task.json

@@ -15,14 +15,22 @@
    ],
    "author": "Microsoft Corporation",
    "version": {
-        "Major": 2,
+        "Major": 3,


Add "releaseNotes" attribute to call out what is changed in this major version.

kmkumaran · 2018-03-13T07:51:04Z

Tasks/Common/VstsAzureHelpers_/ImportFunctions.ps1

-                } else {
-                    throw (Get-VstsLocString -Key AZ_ModuleNotFound -ArgumentList "Any version")
-                }
+                ThrowAzureModuleNotFoundException -azurePsVersion $azurePsVersion -modules "Azure, AzureRM"
            }
        } elseif ($PreferredModule -contains 'Azure') {
            # Attempt to import Azure but fallback to AzureRM.


The logic and this comment do not match anymore, since we don't fallback always.

kmkumaran · 2018-03-13T10:10:26Z

Tasks/Common/VstsAzureHelpers_/InitializeFunctions.ps1

-                {
-                    Write-Host "##[command]Add-AzureRMAccount -ServicePrincipal -Tenant $($Endpoint.Auth.Parameters.TenantId) -Credential $psCredential -EnvironmentName $environmentName"
-                    $null = Add-AzureRMAccount -ServicePrincipal -Tenant $Endpoint.Auth.Parameters.TenantId -Credential $psCredential -EnvironmentName $environmentName
+                if (Get-Command -Name "Add-AzureRmAccount" -ErrorAction "SilentlyContinue") {


Should you update in releases/M131 so that earlier version of task also get this?

…well

… directory for generating inline script

arjgupta added 4 commits January 31, 2018 18:05

adding failonstderror option to azure ps task

aa2ead0

updating the patch version to the correct number

691d7a9

moving failonstandarderror option to the advanced section

1501e39

fix tests for redirecting errors

a4542c6

arjgupta requested review from KrishnaAdityaB, chshrikh and pavanadepu2 January 31, 2018 14:42

adding test to check that the task succeeds if failonstandarderror is…

273d548

… set to false and non-terminating errors are written to the error stream

ericsciple reviewed Feb 1, 2018

View reviewed changes

kmkumaran self-requested a review February 2, 2018 03:29

kmkumaran self-assigned this Feb 2, 2018

kmkumaran added the Area: Release label Feb 2, 2018

arjgupta added 9 commits March 7, 2018 14:37

adding changes for erroractionpreference and failonstandarderror

2d1516f

resolve merge conflicts

8377129

minor changes

cd1ab79

fixing tests

3655c6f

adding L0 test to check that failonstandarderror is honored

b597558

adding L0 test to check that failonstandarderror is honored

8599f04

fix common tests

01e8914

fix common tests

31dcb74

mitigate alias not found bug in latest azure powershell releases

1e74ce5

kmkumaran reviewed Mar 13, 2018

View reviewed changes

arjgupta added 3 commits March 14, 2018 12:50

use connect-azurermaccount cmdlet for usernamepassword authscheme as …

b95368d

…well

changing temp directory path from system temp directory to agent temp…

51dd7db

… directory for generating inline script

update releasenotes and update comments

a1c2a47

kmkumaran approved these changes Mar 14, 2018

View reviewed changes

arjgupta merged commit a5ac2c0 into master Mar 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fail on standard error option for azure powershell task #6324

Fail on standard error option for azure powershell task #6324

arjgupta commented Jan 31, 2018

arjgupta commented Feb 1, 2018 •

edited

Loading

ericsciple Feb 1, 2018

arjgupta Feb 1, 2018

ericsciple Feb 1, 2018

ericsciple Feb 2, 2018

arjgupta Feb 5, 2018 •

edited

Loading

ericsciple Feb 7, 2018

ericsciple Feb 7, 2018

kmkumaran Mar 13, 2018

arjgupta Mar 13, 2018

arjgupta Mar 14, 2018

kmkumaran Mar 13, 2018

arjgupta Mar 14, 2018

kmkumaran Mar 13, 2018

arjgupta Mar 13, 2018

kmkumaran Mar 13, 2018

arjgupta Mar 14, 2018

kmkumaran Mar 13, 2018

arjgupta Mar 14, 2018

kmkumaran Mar 13, 2018

arjgupta Mar 14, 2018

Fail on standard error option for azure powershell task #6324

Fail on standard error option for azure powershell task #6324

Conversation

arjgupta commented Jan 31, 2018

arjgupta commented Feb 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arjgupta Feb 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arjgupta commented Feb 1, 2018 •

edited

Loading

arjgupta Feb 5, 2018 •

edited

Loading