Print stderr on kitops execution error

[notniknot]

When subprocess.run throws an error it is unclear what caused this error. It only says:
subprocess.CalledProcessError: Command '['kit', 'login', 'xxx', '--username', 'xxx', '--password-stdin']' returned non-zero exit status 1.

It should output the stderr as well to make troubleshooting easier.

[jfuss]

Makes sense to me. I think we should be able to catch the Error and then do a LOG.error().

[jfuss]

@notniknot I was looking more into this. You should be able to get the stderr by:

except subprocess.CalledProcessError as e:
    print(e.stderr)

CalledProcessError docs

We capture both stdout and stderr. So that should be enough, unless I am miss understanding something here.

[notniknot]

@jfuss Yes, that would be one way to get the exact error message. How do you feel about enclosing the run command with a try-except, enriching the error message with the stderr output and reraising the error?

[amisevsk]

I think I did notice at one point that the way things are structured means a failed command throws an exception which discards the captured error/stdout.

Also, worth noting that I believe all error logs will be printed to stderr from Kit, so it likely will not be a single line. Additionally, often (especially with debug or trace logging), the regular log lines immediately preceding the error are useful as context.

[jfuss]

How do you feel about enclosing the run command with a try-except, enriching the error message with the stderr output and reraising the error?

@notniknot The information is already there. What do you mean by enriching in this context?

To be up front: My initial reaction is to have users fetch this information and output as they need. I guess the con to that is every customer would need to do this. But it is information already provided to you. If the information wasn't already provided, adding it makes sense but you already have it, just not in a way you are accessing it today.

I am going to play with this more today but I thought we were not providing this information at all, which changes how I think about it.

[notniknot]

@jfuss Ah I see. I didn’t expect that I have to catch that specific exception and print the stderr by myself. This is probably some knowledge users don’t have upfront. Might be subjective, but I think the error message of the underlying library should already contain the necessary information what caused the error.