appengine-plugins icon indicating copy to clipboard operation
appengine-plugins copied to clipboard

Log error (stdout and stderr) on command failures (non-zero exits) for Managed Cloud SDK processes

Open saturnism opened this issue 6 years ago • 8 comments

Currently, when command fails (such as ManagedCloudSDK.isUpToDate()) fails, there is no insight into detailed error message, even though, error message is captured in CommandExitException.

https://github.com/GoogleCloudPlatform/appengine-plugins-core/blob/master/src/main/java/com/google/cloud/tools/managedcloudsdk/ManagedCloudSdk.java#L162

Suggest that upon CommandExitException, log error with high priority and/or propagate to the wrapping Exception, or, use it as part of the message of CommandExitException

saturnism avatar Nov 08 '19 16:11 saturnism

https://github.com/GoogleCloudPlatform/app-maven-plugin/issues/409 https://github.com/GoogleCloudPlatform/cloud-code-intellij/issues/2662

saturnism avatar Nov 08 '19 16:11 saturnism

This makes sense though I don't think it's diagnosed quite right. The problem is that CommandCaller doesn't save stderr into the exception. It should.

Logging here we don't need to do. Including more information in the exception is the right approach.

elharo avatar Nov 08 '19 17:11 elharo

@elharo it does save error log into CommandExitException. I can see the error message. but no where in the stack trace and/or execution actually displayed it.

saturnism avatar Nov 08 '19 17:11 saturnism

Although, the error should probably to be logged and/or surfaced by the consumer of this API instead.

saturnism avatar Nov 08 '19 17:11 saturnism

That's a decision for the dependent of this library to make. This library does not attempt to talk to end users.

elharo avatar Nov 08 '19 18:11 elharo

As described here, I'm no longer sure there is an issue for appengine-plugins-core to resolve. This library makes that information available downstream. It's up to the individual callers to make use of it. Perhaps this bug should be addressed in app-maven-pluign and cloud-code-intellij?

elharo avatar Nov 11 '19 18:11 elharo

Issue transferred from appengine-plugins-core to app-maven-plugin. I'll create a new one for app-gradle-plugin.

chanseokoh avatar Nov 11 '19 21:11 chanseokoh

Cross-commenting from the gradle issue; not planning to close the individual plugin ones.

I think this should be resolved here in appengine-plugins-core.

Yes, ManagedSdkVerificationException contains the cause and can return it if prompted, but then it's on the caller (maven and gradle plugins) to get the cause exception, compare its type to CommandExecutionException and get the output.

It would be easier if CommandExecutionException.getErrorLog() got logged before it's wrapped in ManagedSdkVerificationException.

elefeint avatar Aug 11 '22 13:08 elefeint