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

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

    XMLWordPrintable

Details

    • 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.

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

      Attachments

        Issue Links

          Activity

            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.

            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.
            haridsv Hari Dara added a comment -

            crussell52: 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

            haridsv Hari Dara added a comment - crussell52 : 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
            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 crussell52'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. crussell52'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 kaihowl 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.

            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 crussell52 '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. crussell52 '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 kaihowl 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.
            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.

            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.

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

            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.

            People

              dnusbaum Devin Nusbaum
              jmif Joe Mifsud
              Votes:
              20 Vote for this issue
              Watchers:
              44 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: