When using json() failing command has output on stdout and not stderr

[bambamboole]

When the option json() is turned on, a not successful execution has its output in on stdout and not on stderr.

I am not sure whats the best way to get around the issue, that a failing execution does not output anything

Option 1:
Check in runProcess if the ANSIBLE_STDOUT_CALLBACK is set to json and then return $process->getOutput() instead of $process->getErrorOutput().

Option 2:
Make the process accesible from the outside and make the user responsible to get the right output

Option 3:
just always return $process->getOutput() (don't know how it's without json enabled)

[bambamboole]

@maschmann do you accept a PR for one of these cases?

[maschmann]

Hi,
sorry for taking that long to react.
Since we're already handling output from the process in case of errors, it's way easier to check if JSON output is used and accordingly return the STDOUT instead of STDERR. In my opinion this is more consistent behavior.

[maschmann]

@bambamboole I thought of something more like this: https://github.com/maschmann/php-ansible/tree/fix/use-stdout-for-error-in-json-mode

[bambamboole]

@maschmann thats absolutely fine. This would also solve my issue (which leads to no need of my fork anymore) 🚀

[maschmann]

Would be great if you could also tests this in your environment. Worked fine for me, but the more testing, the better :-)

[bambamboole]

ok, but then I need a few days.

[maschmann]

Just let me know if got around to testing it - I'll be around a bit more frequent than the past months.
The other option would be to release an early version of all the recent changes and letting it run for a while.
Since there are lots of deprecations and new dependency versions, breaking bc, it warrants a new "major" version - so it should be a concious decision to update/upgrade for anyone.

[bambamboole]

Could be a 5.0.0-rc1

[maschmann]

Exactly - I'd include the host sprintf change you made and prepare the rc 👍

[maschmann]

Hi @bambamboole, I've added most changes to the rc. You'd still have to contribute the sprintf change for hosts. Otherwise I'm pretty sure, we're mostly through.

[bambamboole]

will check in the evening ! thx for reminding me.

[bambamboole]

I think you merged it here. I just had it also in the other PR, but made a separate PR for it. Here it is in main.

[maschmann]

Thanks, totally missed that 🙄 . So, great, works again!
If you don't find any other issues, i'll build the release within the next days.