Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-64882

returnStdout does not capture output of failing command

XMLWordPrintable

      It turned out that 'sh(returnStdout: true' works only if the command exit code is successful.

      node('linux') {
          def failingCmd = 'rm .' // expected stdout: "rm: cannot remove '.': Is a directory"
          def stdout = null
         
          try {
              stdout = sh(returnStdout: true, script: failingCmd)
              print("stdout(success)=" + stdout)
          } catch (Exception e) {
              print("stdout(failed)=" + stdout) // I WANT STDOUT HERE NEVERTHELESS !!!
          }
      }
      

      Output:

      rm: cannot remove '.': Is a directory
      stdout(failed)=null
      

      It is absolutely inappropriate that there is no possibility to capture output of failed commands! It is 100% natural that users need to analyze output of failed commands in many cases.

      Root cause analysis
      The problem is that in the https://github.com/jenkinsci/workflow-durable-task-step-plugin/blob/master/src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java in handleExit() method these two things are self-exclusive:

      • getContext().onSuccess(returnStatus ? exitCode : returnStdout ? new String(output.produce(), StandardCharsets.UTF_8) : null); // calls CpsStepContext.completed(new Outcome(returnValue, null))
      • getContext().onFailure(new AbortException("script returned exit code " + exitCode)); // calls CpsStepContext.completed(new Outcome(null, t))

      Unfortunately it ends up with the usage of the class https://github.com/cloudbees/groovy-cps/blob/master/lib/src/main/java/com/cloudbees/groovy/cps/Outcome.java which semantically does not allow both values non-null which is clear from this assertion:

          public Outcome(Object normal, Throwable abnormal) {
              assert normal==null || abnormal==null;
              this.normal = normal;
              this.abnormal = abnormal;
          }
      

      So it seems to be already wrongly programmed as "either-or" and on my opinion it was a strategical mistake in the beginning to represent the step result as "either exception or data".

      Suggested solution
      Short-term solution (workaround) would be to extend the text that we pass into onFailure so:
      getContext().onFailure(new AbortException("script returned exit code " + exitCode + returnStdout ? (", output: " + new String(output.produce(), StandardCharsets.UTF_8))));

      Long-term solution: I suggest to refuse of this Outcome class completely.
      Replace it with the own CmdResponse class which has 3 parameters returnStatus, returnStdout, returnStderr parameters and use it in CpsStepContext.java instead Outcome. I think the users will be very happy even if stdout and stderr are in the same returnOutput variable for the first time. The most important problem is to allow returnStatus AND any data as two independent entities.

      P. S.:
      Another point. There is a ticket https://issues.jenkins.io/browse/JENKINS-44930 which I hope a lot will be resolved some day in the Pipeline users favor. But it won't become possible without resolving the above mentioned problem, because if we are aimed to return at once exit code, stdout and stderr then it will be totally useless to get null values in stdout and stderr unless exit code is 0. I kindly ask to think about it in the scope of JENKINS-44930 as well and refer to this ticket.

            Unassigned Unassigned
            alexander_samoylov Alexander Samoylov
            Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: