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

Running asynchronous code inside a @NonCPS method should fail cleanly

    • workflow-cps 2.71

      User-visible bit: throws an error

       

      Currently if you run a step with asynchronous execution (which is to say, almost all of them, since AbstractSynchronousNonBlockingStepExecution counts) inside a method marked @NonCPS, or otherwise not transformed (for example because of JENKINS-26481), you get bizarre results: the return value of the step becomes the return value of the method, regardless of what was supposed to happen in the rest of the method. This is because CpsCallableInvocation is thrown from DSL.invokeMethod and interrupts the method's execution.

      This is very confusing behavior. It would be better for CpsCallableInvocation.<init> to somehow detect that it is inside an untransformed method and throw a comprehensible error.

          [JENKINS-31314] Running asynchronous code inside a @NonCPS method should fail cleanly

          Andrew Bayer added a comment -

          I’ll take a look tomorrow.

          Andrew Bayer added a comment - I’ll take a look tomorrow.

          Any progress on this?

          Yngvar Kristiansen added a comment - Any progress on this?

          Jesse Glick added a comment -

          No, I had a prototype but stopped work on this.

          Jesse Glick added a comment - No, I had a prototype but stopped work on this.

          Devin Nusbaum added a comment -

          A fix for this issue was released in Pipeline: Groovy Plugin version 2.71.

          Devin Nusbaum added a comment - A fix for this issue was released in Pipeline: Groovy Plugin version 2.71.

          Hi dnusbaum, I’m afraid this change broke our Jenkinsfile runner based testing because the warnings will be thrown as an exception there. Reproducer here https://github.com/fwilhe/repro-cps-issue

          I know we have to fix this in a proper way, but is there some way to turn it into warnings also on Jenkinsfile runner and not have it throw an exception?

          Florian Wilhelm added a comment - Hi dnusbaum , I’m afraid this change broke our Jenkinsfile runner based testing because the warnings will be thrown as an exception there. Reproducer here  https://github.com/fwilhe/repro-cps-issue I know we have to fix this in a proper way, but is there some way to turn it into warnings also on Jenkinsfile runner and not have it throw an exception?

          Jesse Glick added a comment -

          fwilhe JFR is being tracked in JENKINS-58407.

          Jesse Glick added a comment - fwilhe JFR is being tracked in JENKINS-58407 .

          Sorry, closed by mistake

          Sergei Parshev added a comment - Sorry, closed by mistake

          kutzi added a comment -

          Why is this resolved?
          As far as I can see, those mismatches are only logged as warnings and builds often run completely different groovy code than the author expects.
          Can we at least have a global property to have builds fail with a hard error, if this happens?

          kutzi added a comment - Why is this resolved? As far as I can see, those mismatches are only logged as warnings and builds often run completely different groovy code than the author expects. Can we at least have a global property to have builds fail with a hard error, if this happens?

          Devin Nusbaum added a comment -

          kutzi Feel free to open a PR to change this line to also support activation via system property, see for example a similar property here.

          Devin Nusbaum added a comment - kutzi Feel free to open a PR to change this line to also support activation via system property, see for example a similar property here .

          Jesse Glick added a comment -

          those mismatches are only logged as warnings

          Because there are tons of corner cases and it is hard to be sure there are no false positives.

          Jesse Glick added a comment - those mismatches are only logged as warnings Because there are tons of corner cases and it is hard to be sure there are no false positives.

            jglick Jesse Glick
            jglick Jesse Glick
            Votes:
            16 Vote for this issue
            Watchers:
            27 Start watching this issue

              Created:
              Updated:
              Resolved: