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

Unique exception for input step abort

    Details

    • Similar Issues:

      Description

      When using input in conjunction with milestone it is impossible to detect the difference between the user choosing to "abort" (e.g. say "no" to the prompt) and the build being abandoned because it was superseded. Both cases throw FlowInterruptedException

      This makes it impossible to take meaningful actions upon the user choosing to abort that you would not want to take if the input goes un-answered.

      One possible solution would be for the input-invoked Abort to throw a unique exception type which extends FlowInterruptedException.

        Attachments

          Issue Links

            Activity

            Hide
            jglick Jesse Glick added a comment -

            You can already check for org.jenkinsci.plugins.workflow.support.steps.input.Rejection as one of the causes but this is not @Whitelisted or otherwise convenient.

            Show
            jglick Jesse Glick added a comment - You can already check for org.jenkinsci.plugins.workflow.support.steps.input.Rejection as one of the causes but this is not @Whitelisted or otherwise convenient.
            Hide
            abayer Andrew Bayer added a comment -

            So we can't switch to a different exception here - too many places expect/handle FlowInterruptedException for all abort cases, including input, and FlowInterruptedException is a final class, so we can't extend it. As Jesse mentioned, looking for Rejection in the causes is the best approach - I'll get a PR up to whitelist FlowInterruptedException#getCauses() and post an example here once it's up.

            Show
            abayer Andrew Bayer added a comment - So we can't switch to a different exception here - too many places expect/handle FlowInterruptedException for all abort cases, including input , and FlowInterruptedException is a final class, so we can't extend it. As Jesse mentioned, looking for Rejection in the causes is the best approach - I'll get a PR up to whitelist FlowInterruptedException#getCauses() and post an example here once it's up.
            Hide
            abayer Andrew Bayer added a comment -

            Added whitelisting in https://github.com/jenkinsci/workflow-step-api-plugin/pull/32 - once that's merged and released, you can do something like this:

            try {
              ...
            } catch (FlowInterruptedException e) {
              if (e.getCauses().any { it instanceof org.jenkinsci.plugins.workflow.support.steps.input.Rejection }) {
              echo "This build was aborted due to user input."
            }
            
            Show
            abayer Andrew Bayer added a comment - Added whitelisting in https://github.com/jenkinsci/workflow-step-api-plugin/pull/32 - once that's merged and released, you can do something like this: try { ... } catch (FlowInterruptedException e) { if (e.getCauses().any { it instanceof org.jenkinsci.plugins.workflow.support.steps.input.Rejection }) { echo "This build was aborted due to user input." }
            Hide
            crussell52 Chris Russell added a comment -

            Implemented suggestions from here and other issue(s) related to challenges accurately detecting abort conditions that works reasonably well and covers detection of input rejection.  Details posted here, if it is useful to anybody else:

            https://issues.jenkins-ci.org/browse/JENKINS-28822?focusedCommentId=346492&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-346492

             

            Show
            crussell52 Chris Russell added a comment - Implemented suggestions from here and other issue(s) related to challenges accurately detecting abort conditions that works reasonably well and covers detection of input rejection.  Details posted here, if it is useful to anybody else: https://issues.jenkins-ci.org/browse/JENKINS-28822?focusedCommentId=346492&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-346492  
            Hide
            hogarthj James Hogarth added a comment -

            Andrew Bayer  "Lemme think on JENKINS-41272 and leave this sitting here for the moment."

            Have you thought about this? I've got a developer who wants to access the causes for an abort but of course getCauses() on FlowInteruptedException isn't whitelisted yet by default...

             

            Show
            hogarthj James Hogarth added a comment - Andrew Bayer   "Lemme think on JENKINS-41272 and leave this sitting here for the moment." Have you thought about this? I've got a developer who wants to access the causes for an abort but of course getCauses() on FlowInteruptedException isn't whitelisted yet by default...  
            Hide
            dnusbaum Devin Nusbaum added a comment -

            Given JENKINS-41272 went with the approach of exposing the causes as JSON to avoid having to whitelist all causes as well, I think it would make sense to take the same approach here. If anyone still wants to work on it, I would file a new PR to add new methods to FlowInterruptedException similar to the approach in https://github.com/jenkinsci/workflow-support-plugin/pull/78.

            Show
            dnusbaum Devin Nusbaum added a comment - Given JENKINS-41272 went with the approach of exposing the causes as JSON to avoid having to whitelist all causes as well, I think it would make sense to take the same approach here. If anyone still wants to work on it, I would file a new PR to add new methods to FlowInterruptedException similar to the approach in https://github.com/jenkinsci/workflow-support-plugin/pull/78 .
            Hide
            jglick Jesse Glick added a comment -

            exposing the causes as JSON to avoid having to whitelist all causes as well

            That is also in line with what I proposed as an api step.

            Show
            jglick Jesse Glick added a comment - exposing the causes as JSON to avoid having to whitelist all causes as well That is also in line with what I proposed as an api step .

              People

              • Assignee:
                Unassigned
                Reporter:
                crussell52 Chris Russell
              • Votes:
                4 Vote for this issue
                Watchers:
                10 Start watching this issue

                Dates

                • Created:
                  Updated: