PowerShell - Improve error checking (#80984)

Improves the error checking when running PowerShell modules using
Ansible.ModuleUtils.Legacy. It will only return an rc of 1 if both the
PowerShell module runner signalled an error occurred and those error
records were present in the output. This should reduce some false
positive errors when using the older module style.

changelogs/fragments/powershell-module-error-handling.yml

@@ -0,0 +1,4 @@
+bugfixes:
+- >-
+  powershell modules - Only set an rc of 1 if the PowerShell pipeline signaled an error occurred AND there are error
+  records present. Previously it would do so only if the error signal was present without checking the error count.

lib/ansible/executor/powershell/module_wrapper.ps1

@@ -207,7 +207,10 @@ if ($null -ne $rc) {
 # with the trap handler that's now in place, this should only write to the output if
 # $ErrorActionPreference != "Stop", that's ok because this is sent to the stderr output
 # for a user to manually debug if something went horribly wrong
-if ($ps.HadErrors -or ($PSVersionTable.PSVersion.Major -lt 4 -and $ps.Streams.Error.Count -gt 0)) {
+if (
+    $ps.Streams.Error.Count -and
+    ($ps.HadErrors -or $PSVersionTable.PSVersion.Major -lt 4)
+) {
     Write-AnsibleLog "WARN - module had errors, outputting error info $ModuleName" "module_wrapper"
     # if the rc wasn't explicitly set, we return an exit code of 1
     if ($null -eq $rc) {

test/integration/targets/win_exec_wrapper/action_plugins/test_rc_1.py

@@ -0,0 +1,35 @@
+# Copyright: (c) 2023, Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+import json
+
+from ansible.plugins.action import ActionBase
+
+
+class ActionModule(ActionBase):
+
+    def run(self, tmp=None, task_vars=None):
+        super().run(tmp, task_vars)
+        del tmp
+
+        exec_command = self._connection.exec_command
+
+        def patched_exec_command(*args, **kwargs):
+            rc, stdout, stderr = exec_command(*args, **kwargs)
+
+            new_stdout = json.dumps({
+                "rc": rc,
+                "stdout": stdout.decode(),
+                "stderr": stderr.decode(),
+                "failed": False,
+                "changed": False,
+            }).encode()
+
+            return (0, new_stdout, b"")
+
+        try:
+            # This is done to capture the raw rc/stdio from the module exec
+            self._connection.exec_command = patched_exec_command
+            return self._execute_module(task_vars=task_vars)
+        finally:
+            self._connection.exec_command = exec_command

test/integration/targets/win_exec_wrapper/library/test_rc_1.ps1

@@ -0,0 +1,17 @@
+#!powershell
+
+# This scenario needs to use Legacy, the same HadErrors won't be set if using
+# Ansible.Basic
+#Requires -Module Ansible.ModuleUtils.Legacy
+
+# This will set `$ps.HadErrors` in the running pipeline but with no error
+# record written. We are testing that it won't set the rc to 1 for this
+# scenario.
+try {
+    Write-Error -Message err -ErrorAction Stop
+}
+catch {
+    Exit-Json @{}
+}
+
+Fail-Json @{} "This should not be reached"

test/integration/targets/win_exec_wrapper/tasks/main.yml

@@ -272,3 +272,12 @@
   assert:
     that:
     - ps_log_count.stdout | int == 0
+
+- name: test module that sets HadErrors with no error records
+  test_rc_1:
+  register: module_had_errors
+
+- name: assert test module that sets HadErrors with no error records
+  assert:
+    that:
+    - module_had_errors.rc == 0