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

Can't distinguish between durable task abort and failure in workflow plugin

    XMLWordPrintable

    Details

    • Similar Issues:
    • Released As:
      workflow-durable-task-step 2.24

      Description

      When a workflow is manually aborted in Jenkins it fires a hudson.AbortException. This is the same thing that happens when a step fails. Thus, it's impossible to properly set the build status to ABORTED on a manual abort and to FAILED on a failed step because you can't programmatically tell the difference.

      Jesse Glick’s recommended fix: see comment as of 2017-06-28

        Attachments

          Issue Links

            Activity

            Hide
            macdrega Joerg Schwaerzler added a comment -

            Thanks for the info. After updating to 2.21 I can confirm that the issue we faced is fixed.

            Show
            macdrega Joerg Schwaerzler added a comment - Thanks for the info. After updating to 2.21 I can confirm that the issue we faced is fixed.
            Hide
            dnusbaum Devin Nusbaum added a comment -

            I am working on this in https://github.com/jenkinsci/workflow-durable-task-step-plugin/pull/75. My main concern is whether fixing it now will break all of the workarounds the people are currently using, so I want to do a few tests with some of the workarounds posted here to get an idea of the impact.

            Show
            dnusbaum Devin Nusbaum added a comment - I am working on this in https://github.com/jenkinsci/workflow-durable-task-step-plugin/pull/75 . My main concern is whether fixing it now will break all of the workarounds the people are currently using, so I want to do a few tests with some of the workarounds posted here to get an idea of the impact.
            Hide
            dnusbaum Devin Nusbaum added a comment -

            There are 2 main differences that my change would cause:

            1. If a sh step is manually aborted, the exception thrown will be a FlowInterruptedException rather than an AbortException (and FlowInterruptedException will be thrown even if returnStatus is true). The script will exit with RESULT.ABORTED both before and after my change. The behavior of Chris Russell's workaround is unaffected by my change in this case.
            2. If a sh step is automatically aborted by a timeout step, the exception thrown will be a FlowInterruptedException rather than an AbortException (and FlowInterruptedException will be thrown even if returnStatus is true), and as a result the script will exit with Result.ABORTED instead of RESULT.FAILURE. Chris Russell's workaround does not handle this case, so the behavior would change in the same way that it would for people not using a workaround. This change also makes timeout}}s of {{sh steps consistent with other steps such as sleep. (ABORTED instead of FAILURE)

            It looks like the workaround from Kai Howelmeyer does not work, because FlowInterruptedException is thrown in the sleeping parallel branch when the pipeline is manually aborted, when the sh step is aborted by a timeout step, or when the sh step fails because of an issue in the script itself, so wasAborted is set to true in all cases and we always rethrow an AbortedException and never execute the block where the sh step's script failed.

            Based on that info, I think it is ok to move forward with the change, but if anyone knows of other common workarounds that may be broken by this change, feel free leave a comment.

            Show
            dnusbaum Devin Nusbaum added a comment - There are 2 main differences that my change would cause: If a sh step is manually aborted, the exception thrown will be a FlowInterruptedException rather than an AbortException (and FlowInterruptedException will be thrown even if returnStatus is true). The script will exit with RESULT.ABORTED both before and after my change. The behavior of Chris Russell 's workaround is unaffected by my change in this case. If a sh step is automatically aborted by a timeout step, the exception thrown will be a FlowInterruptedException rather than an AbortException (and FlowInterruptedException will be thrown even if returnStatus is true), and as a result the script will exit with Result.ABORTED instead of RESULT.FAILURE. Chris Russell 's workaround does not handle this case, so the behavior would change in the same way that it would for people not using a workaround. This change also makes timeout}}s of {{sh steps consistent with other steps such as sleep . (ABORTED instead of FAILURE) It looks like the workaround from Kai Howelmeyer does not work, because FlowInterruptedException is thrown in the sleeping parallel branch when the pipeline is manually aborted, when the sh step is aborted by a timeout step, or when the sh step fails because of an issue in the script itself, so wasAborted is set to true in all cases and we always rethrow an AbortedException and never execute the block where the sh step's script failed. Based on that info, I think it is ok to move forward with the change, but if anyone knows of other common workarounds that may be broken by this change, feel free leave a comment.
            Hide
            haridsv Hari Dara added a comment -

            Chris Russell: I see that your workaround uses instanceof checks, but these are not permitted in pipeline sandbox. Using instanceof generates the below sort of error:

            org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method java.lang.Class isInstance java.lang.Object 

            I just submitted this PR that whitelists isinstance check: https://github.com/jenkinsci/script-security-plugin/pull/226

            Show
            haridsv Hari Dara added a comment - Chris Russell : I see that your workaround uses instanceof checks, but these are not permitted in pipeline sandbox. Using instanceof generates the below sort of error: org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method java.lang.Class isInstance java.lang.Object I just submitted this PR that whitelists isinstance  check:  https://github.com/jenkinsci/script-security-plugin/pull/226
            Hide
            dnusbaum Devin Nusbaum added a comment - - edited

            Fixed in Pipeline Nodes and Processes 2.24. See my comment here for an overview of the changes, but in short, externally aborted tasks should throw FlowInterruptedException, while tasks that fail in the script itself should throw AbortException.

            Show
            dnusbaum Devin Nusbaum added a comment - - edited Fixed in Pipeline Nodes and Processes 2.24. See my comment here for an overview of the changes, but in short, externally aborted tasks should throw FlowInterruptedException, while tasks that fail in the script itself should throw AbortException.

              People

              • Assignee:
                dnusbaum Devin Nusbaum
                Reporter:
                jmif Joe Mifsud
              • Votes:
                20 Vote for this issue
                Watchers:
                42 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: