Code changed in jenkins
User: Jesse Glick
Path:
aggregator/src/test/java/org/jenkinsci/plugins/workflow/SerializationTest.java
http://jenkins-ci.org/commit/workflow-plugin/e5d9b321b7490d811cf3db5e4e93630c2f912412
Log:
JENKINS-31314 Demonstrating problem in test.
The sample flow cannot possibly work, yet it runs without errors, just weird behavior.

Code changed in jenkins
User: Jesse Glick
Path:
aggregator/src/test/java/org/jenkinsci/plugins/workflow/SerializationTest.java
http://jenkins-ci.org/commit/workflow-plugin/59e6cf394ba79dea4db3dd2b99b7cca891341f16
Log:
Merge pull request #233 from jglick/test-JENKINS-31314

JENKINS-31314 Demonstrating problem in test

Compare: https://github.com/jenkinsci/workflow-plugin/compare/1b83632c5b8d...59e6cf394ba7

Code changed in jenkins
User: Jesse Glick
Path:
aggregator/src/test/java/org/jenkinsci/plugins/workflow/SerializationTest.java
http://jenkins-ci.org/commit/workflow-cps-plugin/7a03a4bcf2c08fa05afb7c5dd39ed73b28fef48c
Log:
JENKINS-31314 Demonstrating problem in test.
The sample flow cannot possibly work, yet it runs without errors, just weird behavior.
Originally-Committed-As: e5d9b321b7490d811cf3db5e4e93630c2f912412

Code changed in jenkins
User: Jesse Glick
Path:
aggregator/src/test/java/org/jenkinsci/plugins/workflow/SerializationTest.java
http://jenkins-ci.org/commit/workflow-cps-plugin/f1b67bb381d1e55ccfe494c80e9d7ec3037d3e71
Log:
Merge pull request #233 from jglick/test-JENKINS-31314

JENKINS-31314 Demonstrating problem in test
Originally-Committed-As: 59e6cf394ba79dea4db3dd2b99b7cca891341f16

Thanks for the first explanation of what is goes wrong when I try writing any non-trivial CPS script. Usually I'm prepared to do the following:

1. Define @NonCPS functions at the top level of the script, and only call them from the master node
2. Use data processing in CPS very carefully on any non-master node (e.g. no closures, not even a regexp for string manipulation)
3. Expect any feature of Groovy to fail at any time.

Needless to say, that I have a hard time explaining people how to solve a problem with CPS, even if my solutions demonstrate that it can work. Maybe it would be easier to raise the priority of such issues if CPS didn't work at all.

only call them from the master node

All Pipeline scripts runs inside the master process. node blocks have no effect on this whatsoever. They only control where nested sh steps and the like connect to.

Is there a potential workaround for this issue?  I tried catching CpsCallableInvocation but in the versions I'm using it doesn't seem to be raised.

When calling httpRequest within an ArrayList.each block, it preemptively returns the value of only the first invocation of httpRequest rather than continuing with the rest of the code in the parent method. @NonCPS on the method does not seem to have an effect.

There is no possible “workaround” for a purely diagnostic issue. Either you understand why your code failed, or you do not. What adampatts is apparently asking for is not a workaround for this issue but for Array.each to actually work in CPS mode, which is accomplished by updating the workflow-cps plugin as per its changelog.

Extremely vague sketch of solution for at least the simple case of

@NonCPS
def play() {
  for (int i = 0; i < 10; i++) {
    sh "echo oops, will not work #$i"
  }
}
node {
  play()
}

would be as follows: ContinuationGroup.methodCall is expecting to call WorkflowScript.play. DSL.invokeStep calls Continuable.suspend which throws the CpsCallableInvocation; it should pass in the name of the method (virtual method DSL.sh in this case) that actually suspended, and this should be recorded in the CpsCallableInvocation. When methodCall catches it, it can then see that there is a mismatch between the method it thought it was calling and the method that actually suspended, and throw an error (or perhaps just print a warning).

(Or, since Caller.record is already being called, we could perhaps do a check in CpsCallableInvocation.<init> that the methods match.)

The tricky part would be tracking down all the less common cases where CpsCallableInvocation is thrown or caught, and either fixing those too, or making the fix conservative so it quietly lets those pass.

A clearer example for svanoort: prior to the introduction of CpsDefaultGroovyMethods,

def r = [1, 2, 3].collect {x -> readFile "data$x"}

would return a String (from data1) rather than a 3-element List<String> as you would expect, due to CpsCallableInvocation breaking out of the loop in DefaultGroovyMethods.collect, and being misinterpreted as the CPS return value of that method. Ouch!

perhaps just print a warning

This is the most attractive option since any mistakes would not result in a serious regression, just a misleading message. I think this could be accomplished with the help of a new method PrintStream Invoker.getLog() which CpsFlowExecution.start could delegate to owner.getListener().getLogger(), thus making messages appear in the build log (outside the scope of any particular step).

jglick - so, I'm lost. DSL.invokeStep is only happening for actual Step implementations, right? So any other CPS-transformed code never calls Continuable.suspend. And CpsClosure throws CpsCallableInvocation from its call and doCall methods. Given that most of the problem cases are actually CpsClosure instances throwing CpsCallableInvocation in non-CPS contexts, how would any of what you said work there?

Not sure. Linked to a sketch of doing the detection for the originally suggested case. Definitely there are others, and those might be harder to handle—I am not sure. Maybe you can figure something out from these hints.

Seems to be catching the cases I would want it to catch. For now it is catching too much, and it will be some work to prune the warnings back to legitimate cases.

Any progress on this?

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

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?

fwilhe JFR is being tracked in JENKINS-58407.

Sorry, closed by mistake

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 Feel free to open a PR to change this line to also support activation via system property, see for example a similar property here.

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.