From 390427a89a177bd17ba9398ccc77bdb912f669fc Mon Sep 17 00:00:00 2001 From: Evgeny Klimov Date: Tue, 6 Jun 2023 11:10:31 +0200 Subject: [PATCH 01/42] CpsFlowExecution: parseScript(): log "Method Too Large" situations more readably --- .../jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index ded38b7b0..f1d586779 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -632,11 +632,15 @@ private CpsScript parseScript() throws IOException { trusted = new CpsGroovyShellFactory(this).forTrusted().build(); shell = new CpsGroovyShellFactory(this).withParent(trusted).build(); - s = (CpsScript) shell.reparse("WorkflowScript",script); + s = (CpsScript) shell.reparse("WorkflowScript", script); for (Entry e : loadedScripts.entrySet()) { shell.reparse(e.getKey(), e.getValue()); } + } catch (groovyjarjarasm.asm.MethodTooLargeException x) { + LOGGER.log(Level.SEVERE, "FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException: " + x.toString()); + closeShells(); + throw x; } catch (RuntimeException | Error x) { closeShells(); throw x; From b91c7f663fdd950486c93769785ac03ff2113b80 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Mar 2024 09:45:31 +0100 Subject: [PATCH 02/42] CpsFlowExecution: import groovyjarjarasm.asm.MethodTooLargeException to avoid catching by full name Signed-off-by: Jim Klimov --- .../org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 120071f95..6767b8270 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -44,6 +44,7 @@ import com.thoughtworks.xstream.io.HierarchicalStreamReader; import com.thoughtworks.xstream.io.HierarchicalStreamWriter; import com.thoughtworks.xstream.mapper.Mapper; +import groovyjarjarasm.asm.MethodTooLargeException; import groovy.lang.GroovyShell; import hudson.ExtensionList; import hudson.model.Action; @@ -638,7 +639,7 @@ private CpsScript parseScript() throws IOException { for (Entry e : loadedScripts.entrySet()) { shell.reparse(e.getKey(), e.getValue()); } - } catch (groovyjarjarasm.asm.MethodTooLargeException x) { + } catch (MethodTooLargeException x) { LOGGER.log(Level.SEVERE, "FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException: " + x.toString()); closeShells(); throw x; From 28cb0c3e54468cc8b8f527358d4c8d74b8fd9ace Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Mar 2024 10:56:02 +0100 Subject: [PATCH 03/42] CpsScriptTest: add methodTooLargeExceptionFabricated() and methodTooLargeExceptionRealistic() tests Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsScriptTest.java | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java index 3d94dbd22..df5e98caa 100644 --- a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java +++ b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java @@ -84,4 +84,112 @@ public void evaluateShallSandbox() throws Exception { r.assertLogContains("Scripts not permitted to use method groovy.lang.Script run java.io.File java.lang.String[]", b); } + @Test public void methodTooLargeExceptionFabricated() throws Exception { + // Fabricate a MethodTooLargeException which "normally" happens when evaluated + // groovy script becomes a Java class too large for Java to handle internally. + // In Jenkins practice this can happen not only due to large singular pipelines + // (one big nudge to offload code into shared libraries), but was also seen due + // to heavy nesting of exception handling and other loops (simple refactoring + // can help). + WorkflowJob p = r.createProject(WorkflowJob.class); + // sandbox == false to allow creation of the exception here: + p.setDefinition(new CpsFlowDefinition( + "import groovyjarjarasm.asm.MethodTooLargeException;\n\n" + + "throw new MethodTooLargeException('className', 'methodName', 'methodDescriptor', 65535);" + , false)); + WorkflowRun b = r.buildAndAssertStatus(Result.FAILURE, p); + r.assertLogContains("groovyjarjarasm.asm.MethodTooLargeException: Method too large: className.methodName methodDescriptor", b); + r.assertLogContains("at WorkflowScript.run(WorkflowScript:3)", b); + r.assertLogContains("at ___cps.transform___(Native Method)", b); + } + + @Test public void methodTooLargeExceptionRealistic() throws Exception { + // See comments above. Here we try to really induce a "method too large" + // condition by abusing the nesting of exception-handling, too many stages + // or methods, and whatever else we can throw at it. + WorkflowJob p = r.createProject(WorkflowJob.class); + StringBuffer sbMethods = new StringBuffer(); + StringBuffer sbStages = new StringBuffer(); + int i, max = 255; + + for (i = 0; i < 250; i++) { + // Up to 255 stages allowed + sbStages.append("stage('Stage " + i + "') { steps { method" + i + "(); } }\n"); + } + + for (i = 0; i < max; i++) { + sbMethods.append("def method" + i + "() { echo 'i = " + i + "'; }\n"); + } + + sbMethods.append("def method() {\n"); + for (i = 0; i < max; i++) { + sbMethods.append("try { // " + i + "\n"); + } + sbMethods.append(" Integer x = 'zzz'; // incur conversion exception\n"); + for (i = 0; i < max; i++) { + sbMethods.append("} catch (Throwable t) { // " + i + "\n method" + i + "(); throw t; }\n"); + } + sbMethods.append("}\n"); + + p.setDefinition(new CpsFlowDefinition(sbMethods.toString() + + "pipeline {\n" + + " agent none;\n" + + " stages {\n" + + " stage ('Test stage') {\n" + + " steps {\n" + + " script {\n" + + " echo 'BEGINNING TEST IN PIPELINE';\n" + + " method();\n" + + " echo 'ENDED TEST IN PIPELINE';\n" + + " }\n" + + " }\n" + + " }\n" + + sbStages.toString() + + " }\n" + + "}\n" + + "//echo 'BEGINNING TEST OUT OF PIPELINE';\n" + + "//method();\n" + + "//echo 'ENDED TEST OUT OF PIPELINE';\n" + , true)); + + WorkflowRun b = p.scheduleBuild2(0).get(); + + // DEV-TEST // System.out.println(b.getLog()); + + r.assertLogContains("MethodTooLargeException", b); + +/* + // Report as of release 3880.vb_ef4b_5cfd270 (Feb 2024) + // and same pattern seen since at least Jun 2022 (note + // that numbers after ___cps___ differ from job to job): + +org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed: +General error during class generation: Method too large: WorkflowScript.___cps___1 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + +groovyjarjarasm.asm.MethodTooLargeException: Method too large: WorkflowScript.___cps___1 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + at groovyjarjarasm.asm.MethodWriter.computeMethodInfoSize(MethodWriter.java:2087) + at groovyjarjarasm.asm.ClassWriter.toByteArray(ClassWriter.java:447) + at org.codehaus.groovy.control.CompilationUnit$17.call(CompilationUnit.java:850) + at org.codehaus.groovy.control.CompilationUnit.applyToPrimaryClassNodes(CompilationUnit.java:1087) + at org.codehaus.groovy.control.CompilationUnit.doPhaseOperation(CompilationUnit.java:624) + at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:602) + at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:579) + at groovy.lang.GroovyClassLoader.doParseClass(GroovyClassLoader.java:323) + at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:293) + at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox$Scope.parse(GroovySandbox.java:163) + at org.jenkinsci.plugins.workflow.cps.CpsGroovyShell.doParse(CpsGroovyShell.java:190) + at org.jenkinsci.plugins.workflow.cps.CpsGroovyShell.reparse(CpsGroovyShell.java:175) + at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.parseScript(CpsFlowExecution.java:637) + at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.start(CpsFlowExecution.java:583) + at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:335) + at hudson.model.ResourceController.execute(ResourceController.java:101) + at hudson.model.Executor.run(Executor.java:442) +*/ + + r.assertLogContains("Method too large: WorkflowScript.___cps___", b); + r.assertLogContains("()Lcom/cloudbees/groovy/cps/impl/CpsFunction;", b); + + // Assert separately from (and after) log parsing, to facilitate test maintenance + r.assertBuildStatus(Result.FAILURE, b); + } } From df2b78f77debe8ff6aea07bc60897a60119d5aa1 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Mar 2024 12:30:53 +0100 Subject: [PATCH 04/42] CpsFlowExecution, CpsScriptTest: handle also MultipleCompilationErrorsException as a carrier of MethodTooLargeException; re-throw with just a compact message to appear in the build log Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 59 ++++++++++++++++++- .../plugins/workflow/cps/CpsScriptTest.java | 9 ++- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 6767b8270..1cd681434 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -52,6 +52,9 @@ import hudson.util.Iterators; import jenkins.model.CauseOfInterruption; import jenkins.model.Jenkins; +import org.codehaus.groovy.control.ErrorCollector; +import org.codehaus.groovy.control.MultipleCompilationErrorsException; +import org.codehaus.groovy.control.messages.Message; import org.jboss.marshalling.Unmarshaller; import org.jenkinsci.plugins.workflow.actions.ErrorAction; import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn; @@ -639,10 +642,60 @@ private CpsScript parseScript() throws IOException { for (Entry e : loadedScripts.entrySet()) { shell.reparse(e.getKey(), e.getValue()); } - } catch (MethodTooLargeException x) { - LOGGER.log(Level.SEVERE, "FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException: " + x.toString()); + } catch (MethodTooLargeException | MultipleCompilationErrorsException x) { + MethodTooLargeException mtlEx = null; + int ecCount = 0; + closeShells(); - throw x; + + if (x instanceof MethodTooLargeException) { + mtlEx = (MethodTooLargeException)x; + ecCount = 1; + } else if (x instanceof MultipleCompilationErrorsException) { + ErrorCollector ec = ((MultipleCompilationErrorsException)x).getErrorCollector(); + ecCount = ec.getErrorCount(); + + for (int i = 0; i < ecCount; i++) { + Exception ex = ec.getException(i); + if (ex == null) + continue; + + LOGGER.log(Level.FINE, "Collected Exception #" + i + ": " + ex.toString()); + if (ex instanceof MethodTooLargeException) { + mtlEx = (MethodTooLargeException) ex; + break; + } + } + } + + if (mtlEx == null) { + // Some other exception type, or collection did not include MTL, rethrow as-is + throw x; + } + + String msg = "FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException"; + if (ecCount > 1) { + msg += " (and other issues)"; + } + // Short message suffices, not much that a pipeline developer + // can do with the stack trace into the guts of groovy + msg += ": " + mtlEx.getMessage(); + + // Make a note in server log + LOGGER.log(Level.SEVERE, msg); + + if (ecCount > 1) { + // Not squashing with explicit MethodTooLargeException + // re-thrown below, in this codepath we have other errors. + throw new RuntimeException(msg, x); + } else { + // Do not confuse pipeline devs by a wall of text in the + // build console, but let the full context be found in + // server log with some dedication. + LOGGER.log(Level.FINE, mtlEx.getMessage()); + //throw new RuntimeException(msg, mtlEx); + throw new RuntimeException(msg); + } } catch (RuntimeException | Error x) { closeShells(); throw x; diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java index df5e98caa..02db95c75 100644 --- a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java +++ b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java @@ -147,9 +147,9 @@ public void evaluateShallSandbox() throws Exception { sbStages.toString() + " }\n" + "}\n" + - "//echo 'BEGINNING TEST OUT OF PIPELINE';\n" + - "//method();\n" + - "//echo 'ENDED TEST OUT OF PIPELINE';\n" + "echo 'BEGINNING TEST OUT OF PIPELINE';\n" + + "method();\n" + + "echo 'ENDED TEST OUT OF PIPELINE';\n" , true)); WorkflowRun b = p.scheduleBuild2(0).get(); @@ -158,6 +158,9 @@ public void evaluateShallSandbox() throws Exception { r.assertLogContains("MethodTooLargeException", b); + // "Prettier" explanation added by CpsFlowExecution.parseScript(): + r.assertLogContains("FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException:", b); + /* // Report as of release 3880.vb_ef4b_5cfd270 (Feb 2024) // and same pattern seen since at least Jun 2022 (note From ffe1f1ac7fe22f61b4073f52415feafd7a8cc83d Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Mar 2024 14:58:29 +0100 Subject: [PATCH 05/42] CpsScriptTest.methodTooLargeExceptionRealistic(): reduce iteration count to satisfy different CI platforms Signed-off-by: Jim Klimov --- .../jenkinsci/plugins/workflow/cps/CpsScriptTest.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java index 02db95c75..4fa11bf33 100644 --- a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java +++ b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java @@ -110,10 +110,14 @@ public void evaluateShallSandbox() throws Exception { WorkflowJob p = r.createProject(WorkflowJob.class); StringBuffer sbMethods = new StringBuffer(); StringBuffer sbStages = new StringBuffer(); - int i, max = 255; - for (i = 0; i < 250; i++) { - // Up to 255 stages allowed + // Limits to the "max": + // * java.lang.StackOverflowError varies per JDK platform + // (CI on Linux was unhappy with 255, on Windows with 1023) + // * Up to 255 stages allowed + int i, max = 200; + + for (i = 0; i < max; i++) { sbStages.append("stage('Stage " + i + "') { steps { method" + i + "(); } }\n"); } From fa95de790987a8ecc415189eece573397b7d353b Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Mar 2024 15:13:44 +0100 Subject: [PATCH 06/42] CpsFlowExecution.parseScript(): handle MethodTooLargeException without importing the class Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 1cd681434..00c2e1dea 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -44,7 +44,6 @@ import com.thoughtworks.xstream.io.HierarchicalStreamReader; import com.thoughtworks.xstream.io.HierarchicalStreamWriter; import com.thoughtworks.xstream.mapper.Mapper; -import groovyjarjarasm.asm.MethodTooLargeException; import groovy.lang.GroovyShell; import hudson.ExtensionList; import hudson.model.Action; @@ -54,7 +53,6 @@ import jenkins.model.Jenkins; import org.codehaus.groovy.control.ErrorCollector; import org.codehaus.groovy.control.MultipleCompilationErrorsException; -import org.codehaus.groovy.control.messages.Message; import org.jboss.marshalling.Unmarshaller; import org.jenkinsci.plugins.workflow.actions.ErrorAction; import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn; @@ -642,17 +640,25 @@ private CpsScript parseScript() throws IOException { for (Entry e : loadedScripts.entrySet()) { shell.reparse(e.getKey(), e.getValue()); } - } catch (MethodTooLargeException | MultipleCompilationErrorsException x) { - MethodTooLargeException mtlEx = null; + } catch (Exception x) { + // Suspected groovyjarjarasm.asm.MethodTooLargeException or a + // org.codehaus.groovy.control.MultipleCompilationErrorsException + // whose collection of errors refers to MethodTooLargeException. + // Per review comments, we do not want to statically compile a + // dependency on the groovyjarjarasm.asm.MethodTooLargeException + // internals, so gauge hitting it via String name comparisons. + // Other cases may be (subclasses of) RuntimeException or Error. + Exception mtlEx = null; int ecCount = 0; + // Clean up first closeShells(); - if (x instanceof MethodTooLargeException) { - mtlEx = (MethodTooLargeException)x; + if (x.getClass().getSimpleName().equals("MethodTooLargeException")) { + mtlEx = x; ecCount = 1; } else if (x instanceof MultipleCompilationErrorsException) { - ErrorCollector ec = ((MultipleCompilationErrorsException)x).getErrorCollector(); + ErrorCollector ec = ((MultipleCompilationErrorsException) x).getErrorCollector(); ecCount = ec.getErrorCount(); for (int i = 0; i < ecCount; i++) { @@ -661,14 +667,14 @@ private CpsScript parseScript() throws IOException { continue; LOGGER.log(Level.FINE, "Collected Exception #" + i + ": " + ex.toString()); - if (ex instanceof MethodTooLargeException) { - mtlEx = (MethodTooLargeException) ex; + if (ex.getClass().getSimpleName().equals("MethodTooLargeException")) { + mtlEx = ex; break; } } } - if (mtlEx == null) { + if (mtlEx == null || ecCount < 1) { // Some other exception type, or collection did not include MTL, rethrow as-is throw x; } @@ -696,9 +702,6 @@ private CpsScript parseScript() throws IOException { //throw new RuntimeException(msg, mtlEx); throw new RuntimeException(msg); } - } catch (RuntimeException | Error x) { - closeShells(); - throw x; } s.execution = this; From 66afa75051a313372ce123ca3ad24d0494e38112 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Mar 2024 15:16:49 +0100 Subject: [PATCH 07/42] CpsFlowExecution.parseScript(): MethodTooLargeException: actionable suggestions for pipeline devs Signed-off-by: Jim Klimov --- .../org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 00c2e1dea..36ed80234 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -685,7 +685,8 @@ private CpsScript parseScript() throws IOException { } // Short message suffices, not much that a pipeline developer // can do with the stack trace into the guts of groovy - msg += ": " + mtlEx.getMessage(); + msg += "; please refactor to simplify code structure and/or move logic to a Jenkins Shared Library: "; + msg += mtlEx.getMessage(); // Make a note in server log LOGGER.log(Level.SEVERE, msg); From 74a125ccea68c61ffa7b75cae43f43447e3db7c6 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Mar 2024 15:20:23 +0100 Subject: [PATCH 08/42] CpsScriptTest.methodTooLargeExceptionRealistic(): update comments and expected matching line Signed-off-by: Jim Klimov --- .../org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java index 4fa11bf33..9242352ef 100644 --- a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java +++ b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java @@ -115,6 +115,8 @@ public void evaluateShallSandbox() throws Exception { // * java.lang.StackOverflowError varies per JDK platform // (CI on Linux was unhappy with 255, on Windows with 1023) // * Up to 255 stages allowed + // FIXME? Tune the value per platform and/or dynamically + // based on stack overflow mention in build log? int i, max = 200; for (i = 0; i < max; i++) { @@ -160,10 +162,13 @@ public void evaluateShallSandbox() throws Exception { // DEV-TEST // System.out.println(b.getLog()); + // Do we have the expected error at all? + // (Maybe also stack overflow on some platforms, + // possibly success on others) r.assertLogContains("MethodTooLargeException", b); // "Prettier" explanation added by CpsFlowExecution.parseScript(): - r.assertLogContains("FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException:", b); + r.assertLogContains("FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException", b); /* // Report as of release 3880.vb_ef4b_5cfd270 (Feb 2024) From 5a50a571e770da31c5e12a85f74145ce61edf101 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Mar 2024 15:52:34 +0100 Subject: [PATCH 09/42] CpsScriptTest.methodTooLargeExceptionRealistic(): decouple maxStagesMethods (class complexity) from maxTryCatch (nesting depth) Signed-off-by: Jim Klimov --- .../jenkinsci/plugins/workflow/cps/CpsScriptTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java index 9242352ef..2bb9223f5 100644 --- a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java +++ b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java @@ -117,22 +117,22 @@ public void evaluateShallSandbox() throws Exception { // * Up to 255 stages allowed // FIXME? Tune the value per platform and/or dynamically // based on stack overflow mention in build log? - int i, max = 200; + int i, maxStagesMethods = 250, maxTryCatch = 127; - for (i = 0; i < max; i++) { + for (i = 0; i < maxStagesMethods; i++) { sbStages.append("stage('Stage " + i + "') { steps { method" + i + "(); } }\n"); } - for (i = 0; i < max; i++) { + for (i = 0; i < maxStagesMethods; i++) { sbMethods.append("def method" + i + "() { echo 'i = " + i + "'; }\n"); } sbMethods.append("def method() {\n"); - for (i = 0; i < max; i++) { + for (i = 0; i < maxTryCatch; i++) { sbMethods.append("try { // " + i + "\n"); } sbMethods.append(" Integer x = 'zzz'; // incur conversion exception\n"); - for (i = 0; i < max; i++) { + for (i = 0; i < maxTryCatch; i++) { sbMethods.append("} catch (Throwable t) { // " + i + "\n method" + i + "(); throw t; }\n"); } sbMethods.append("}\n"); From 89bbd4036f93a6ba8d3b226f6503c2cd6cad5046 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 11 Jun 2024 08:57:38 +0200 Subject: [PATCH 10/42] Update plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java Do not log to server console log (not all server admins are those who run the projects with issues). Co-authored-by: Jesse Glick --- .../org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 354125da6..5a016c474 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -687,9 +687,6 @@ private CpsScript parseScript() throws IOException { msg += "; please refactor to simplify code structure and/or move logic to a Jenkins Shared Library: "; msg += mtlEx.getMessage(); - // Make a note in server log - LOGGER.log(Level.SEVERE, msg); - if (ecCount > 1) { // Not squashing with explicit MethodTooLargeException // re-thrown below, in this codepath we have other errors. From 92aadc958a0198d034a0b01b6f0ccd7dd2bf2a8d Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 26 Jun 2024 08:51:43 +0200 Subject: [PATCH 11/42] CpsFlowExecution: restore the multi-catch for "RuntimeException | Error" Notably, the latter is a Throwable but not an Exception so e.g. LinkageError might be not handled. Signed-off-by: Jim Klimov --- .../jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 5a016c474..28bea0193 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -639,7 +639,7 @@ private CpsScript parseScript() throws IOException { for (Entry e : loadedScripts.entrySet()) { shell.reparse(e.getKey(), e.getValue()); } - } catch (Exception x) { + } catch (RuntimeException | Error x) { // Suspected groovyjarjarasm.asm.MethodTooLargeException or a // org.codehaus.groovy.control.MultipleCompilationErrorsException // whose collection of errors refers to MethodTooLargeException. @@ -647,7 +647,9 @@ private CpsScript parseScript() throws IOException { // dependency on the groovyjarjarasm.asm.MethodTooLargeException // internals, so gauge hitting it via String name comparisons. // Other cases may be (subclasses of) RuntimeException or Error. - Exception mtlEx = null; + // Note that both MultipleCompilationErrorsException and + // MethodTooLargeException are descended from RuntimeException. + Throwable mtlEx = null; int ecCount = 0; // Clean up first From 8c36e276c28254dd00694cc6689da2dc20c43885 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 26 Jun 2024 09:22:28 +0200 Subject: [PATCH 12/42] Revert "Update plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java " This reverts commit 89bbd4036f93a6ba8d3b226f6503c2cd6cad5046 to do emit the server log message but at hushed verbosity level. --- .../org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 28bea0193..399c9f94d 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -689,6 +689,9 @@ private CpsScript parseScript() throws IOException { msg += "; please refactor to simplify code structure and/or move logic to a Jenkins Shared Library: "; msg += mtlEx.getMessage(); + // Make a note in server log + LOGGER.log(Level.SEVERE, msg); + if (ecCount > 1) { // Not squashing with explicit MethodTooLargeException // re-thrown below, in this codepath we have other errors. From 82470d21e975984188495df27a520fd10cc90cf8 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 26 Jun 2024 09:26:45 +0200 Subject: [PATCH 13/42] CpsFlowExecution: restore the write into server log, but only at FINE/FINER level - so it is usually hidden Signed-off-by: Jim Klimov --- .../jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 399c9f94d..fccf59c30 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -651,6 +651,7 @@ private CpsScript parseScript() throws IOException { // MethodTooLargeException are descended from RuntimeException. Throwable mtlEx = null; int ecCount = 0; + String xStr = x.getMessage() + "\n" + Functions.printThrowable(x); // Clean up first closeShells(); @@ -689,8 +690,8 @@ private CpsScript parseScript() throws IOException { msg += "; please refactor to simplify code structure and/or move logic to a Jenkins Shared Library: "; msg += mtlEx.getMessage(); - // Make a note in server log - LOGGER.log(Level.SEVERE, msg); + // Make a full note in server log + LOGGER.log(Level.FINER, xStr); if (ecCount > 1) { // Not squashing with explicit MethodTooLargeException @@ -702,7 +703,9 @@ private CpsScript parseScript() throws IOException { // server log with some dedication. LOGGER.log(Level.FINE, mtlEx.getMessage()); //throw new RuntimeException(msg, mtlEx); - throw new RuntimeException(msg); + throw new RuntimeException(msg + + "\nComplete details can be seen in server log at FINE/FINER level " + + "(Jenkins admin access is required)"); } } From db51a8aedea7679e2ece686d00973592a2ca572f Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 26 Jun 2024 09:34:55 +0200 Subject: [PATCH 14/42] CpsFlowExecution: comment that the remaining case is "ecCount == 1 exactly" and what this means practically Signed-off-by: Jim Klimov --- .../org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 1 + 1 file changed, 1 insertion(+) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index fccf59c30..2dfb54257 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -698,6 +698,7 @@ private CpsScript parseScript() throws IOException { // re-thrown below, in this codepath we have other errors. throw new RuntimeException(msg, x); } else { + // ecCount == 1 exactly, this is the only problem we saw. // Do not confuse pipeline devs by a wall of text in the // build console, but let the full context be found in // server log with some dedication. From 20213eaf784403eb836a136e8432d92ffa541543 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 26 Jun 2024 16:43:02 +0200 Subject: [PATCH 15/42] CpsFlowExecution: recognize also CpsCompilationErrorsException for MethodTooLargeException handling Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 2dfb54257..292fcfd75 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -138,6 +138,8 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.LongAdder; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; @@ -647,11 +649,14 @@ private CpsScript parseScript() throws IOException { // dependency on the groovyjarjarasm.asm.MethodTooLargeException // internals, so gauge hitting it via String name comparisons. // Other cases may be (subclasses of) RuntimeException or Error. - // Note that both MultipleCompilationErrorsException and - // MethodTooLargeException are descended from RuntimeException. + // Note that all of MultipleCompilationErrorsException, and + // MethodTooLargeException and CpsCompilationErrorsException + // are descended from RuntimeException. Throwable mtlEx = null; int ecCount = 0; String xStr = x.getMessage() + "\n" + Functions.printThrowable(x); + final Pattern LINE_SEP_PATTERN = Pattern.compile("\\R"); + String[] xLines = LINE_SEP_PATTERN.split(xStr); // Clean up first closeShells(); @@ -674,6 +679,50 @@ private CpsScript parseScript() throws IOException { break; } } + } else if (x instanceof CpsCompilationErrorsException) { + // Defined in this plugin, to clone a message and stack trace + // from a MultipleCompilationErrorsException and be serializable. + // Grep it as text for "MethodTooLargeException" and "1 error" + // (as a complete line, surrounded by blank lines, with no other + // similar lines in text) to be sure we've got it as the only + // problem. Note the code overflow may be not in "WorkflowScript" + // of the pipeline, but in a JSL step (global variable) or even + // class with an actual huge method that should be refactored. + if (xStr.contains("MethodTooLargeException")) { + final Pattern NUM_ERROR_PATTERN = Pattern.compile("^\\d+ error$"); + boolean blankBefore = false, patternMatchedAfterBlank = false; + + for (String l : xLines) { + if (l.isBlank()) { + // Is this the blank before or after the pattern we seek? + // Rule out several blank lines before the match, too... + if (!blankBefore && !patternMatchedAfterBlank) { + blankBefore = true; + } else if (patternMatchedAfterBlank) { + // Got a blank line after a pattern match + patternMatchedAfterBlank = false; + blankBefore = false; + ecCount++; + } + } else if (blankBefore) { + // Ignore pattern when no blank line was before it + Matcher matcher = NUM_ERROR_PATTERN.matcher(l); + if (matcher.find()) { + patternMatchedAfterBlank = true; + } else { + // red herring + blankBefore = false; + } + } else { + // part of wall of text + patternMatchedAfterBlank = false; + } + } + + if (ecCount > 0) { + mtlEx = x; + } + } } if (mtlEx == null || ecCount < 1) { From f23b61078255ffbc65f302dc68a765721103fce9 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 26 Jun 2024 16:44:58 +0200 Subject: [PATCH 16/42] CpsFlowExecution, CpsScriptTest: recognize also Groovy (JSL step or class) for MethodTooLargeException handling And re-word logging (impacted class name, possible excerpts from trace). Not only WorkflowScript (pipeline) suffers from this, see examples in https://github.com/jenkinsci/workflow-cps-plugin/pull/817 Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 50 +++++++++++++++++-- .../plugins/workflow/cps/CpsScriptTest.java | 2 +- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 292fcfd75..aca4e3bbd 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -730,14 +730,58 @@ private CpsScript parseScript() throws IOException { throw x; } - String msg = "FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException"; + // Collect the relevant part of stack trace through groovy (JSL), + // if any, which the pipeline developer can impact and fix. + // Some real-life sample patterns are posted in + // https://github.com/jenkinsci/workflow-cps-plugin/pull/817 + String overflowedClassName = null; + List overflowedClassNameMentionsList = new ArrayList(); + // groovyjarjarasm.asm.MethodTooLargeException: Method too large: cloudBranch.___cps___586328 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + final Pattern MTLE_CLASSNAME_PATTERN = Pattern.compile("^.*MethodTooLargeException.*: ([^\\s.]+)\\.___cps___\\d+.*$"); + Pattern CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|\\.groovy):\\d+\\).*$"); + for (String l : xLines) { + if (!(l.isBlank())) { + if (overflowedClassName == null) { + Matcher matcher = MTLE_CLASSNAME_PATTERN.matcher(l); + if (matcher.find()) { + try { + overflowedClassName = matcher.group(1); + if (!(mtlEx.getMessage().contains(overflowedClassName))) + overflowedClassNameMentionsList.add(l); + + // Update the matching pattern in case we manage + // to spot our problematic source in the stack trace + CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|" + overflowedClassName + ".*|\\.groovy):\\d+\\).*$"); + continue; + } catch (Throwable ignored) { + } + } + } + + Matcher matcher = CLASSNAME_MENTIONS_PATTERN.matcher(l); + if (matcher.find()) { + overflowedClassNameMentionsList.add(l); + } + } + } + + if (overflowedClassName == null) + overflowedClassName = "WorkflowScript (the pipeline script) or one of its constituents"; + + String msg = "FAILED to parse " + overflowedClassName + " due to MethodTooLargeException"; if (ecCount > 1) { msg += " (and other issues)"; } // Short message suffices, not much that a pipeline developer // can do with the stack trace into the guts of groovy - msg += "; please refactor to simplify code structure and/or move logic to a Jenkins Shared Library: "; - msg += mtlEx.getMessage(); + msg += "; please refactor to simplify code structure"; + if (overflowedClassName.contains("WorkflowScript")) + msg += " and/or move logic to a Jenkins Shared Library"; + msg += ": " + mtlEx.getMessage(); + if (!(overflowedClassNameMentionsList.isEmpty())) { + msg += "\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n" + + String.join("\n", overflowedClassNameMentionsList); + } // Make a full note in server log LOGGER.log(Level.FINER, xStr); diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java index 2bb9223f5..a13d0f2d6 100644 --- a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java +++ b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java @@ -168,7 +168,7 @@ public void evaluateShallSandbox() throws Exception { r.assertLogContains("MethodTooLargeException", b); // "Prettier" explanation added by CpsFlowExecution.parseScript(): - r.assertLogContains("FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException", b); + r.assertLogContains("FAILED to parse WorkflowScript due to MethodTooLargeException", b); /* // Report as of release 3880.vb_ef4b_5cfd270 (Feb 2024) From 21a3f8d2dc2942e20776fc7cb4f4196bf0b1725c Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Thu, 27 Jun 2024 20:46:06 +0200 Subject: [PATCH 17/42] CpsFlowExecution: introduce METHOD_TOO_LARGE_LOGGER for separated logging of pretty-printing certain error handling Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index aca4e3bbd..5db73e7b8 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -453,6 +453,8 @@ Timing time(TimingKind kind) { static final Logger TIMING_LOGGER = Logger.getLogger(CpsFlowExecution.class.getName() + ".timing"); + static final Logger METHOD_TOO_LARGE_LOGGER = Logger.getLogger(CpsFlowExecution.class.getName() + ".MethodTooLargeLogging"); + void logTimings() { if (TIMING_LOGGER.isLoggable(Level.FINE)) { Map formatted = new TreeMap<>(); @@ -673,7 +675,7 @@ private CpsScript parseScript() throws IOException { if (ex == null) continue; - LOGGER.log(Level.FINE, "Collected Exception #" + i + ": " + ex.toString()); + METHOD_TOO_LARGE_LOGGER.log(Level.FINE, "Collected Exception #" + i + ": " + ex.toString()); if (ex.getClass().getSimpleName().equals("MethodTooLargeException")) { mtlEx = ex; break; @@ -784,7 +786,7 @@ private CpsScript parseScript() throws IOException { } // Make a full note in server log - LOGGER.log(Level.FINER, xStr); + METHOD_TOO_LARGE_LOGGER.log(Level.FINER, xStr); if (ecCount > 1) { // Not squashing with explicit MethodTooLargeException @@ -795,11 +797,11 @@ private CpsScript parseScript() throws IOException { // Do not confuse pipeline devs by a wall of text in the // build console, but let the full context be found in // server log with some dedication. - LOGGER.log(Level.FINE, mtlEx.getMessage()); + METHOD_TOO_LARGE_LOGGER.log(Level.FINE, mtlEx.getMessage()); //throw new RuntimeException(msg, mtlEx); throw new RuntimeException(msg + "\nComplete details can be seen in server log at FINE/FINER level " + - "(Jenkins admin access is required)"); + "(Jenkins admin access for " + METHOD_TOO_LARGE_LOGGER.getName() + " is required)"); } } From 47a5e84b5002fc401478f37ed5fee52486c6ddd6 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Thu, 27 Jun 2024 20:57:51 +0200 Subject: [PATCH 18/42] CpsFlowExecution: annotate what we post into METHOD_TOO_LARGE_LOGGER Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 5db73e7b8..b2dc65aa9 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -675,7 +675,9 @@ private CpsScript parseScript() throws IOException { if (ex == null) continue; - METHOD_TOO_LARGE_LOGGER.log(Level.FINE, "Collected Exception #" + i + ": " + ex.toString()); + METHOD_TOO_LARGE_LOGGER.log(Level.FINE, + "CpsFlowExecution.reportSuspectedMethodTooLarge: " + + "Collected Exception #" + i + ": " + ex.toString()); if (ex.getClass().getSimpleName().equals("MethodTooLargeException")) { mtlEx = ex; break; @@ -786,7 +788,7 @@ private CpsScript parseScript() throws IOException { } // Make a full note in server log - METHOD_TOO_LARGE_LOGGER.log(Level.FINER, xStr); + METHOD_TOO_LARGE_LOGGER.log(Level.FINER, "CpsFlowExecution.reportSuspectedMethodTooLarge: full original Throwable message:\n" + xStr); if (ecCount > 1) { // Not squashing with explicit MethodTooLargeException @@ -796,8 +798,10 @@ private CpsScript parseScript() throws IOException { // ecCount == 1 exactly, this is the only problem we saw. // Do not confuse pipeline devs by a wall of text in the // build console, but let the full context be found in - // server log with some dedication. - METHOD_TOO_LARGE_LOGGER.log(Level.FINE, mtlEx.getMessage()); + // server log with some dedication. Note it is seen at + // a different logging verbosity level. + METHOD_TOO_LARGE_LOGGER.log(Level.FINE, "CpsFlowExecution.reportSuspectedMethodTooLarge: detected details of MethodTooLargeException:\n" + mtlEx.getMessage()); + //throw new RuntimeException(msg, mtlEx); throw new RuntimeException(msg + "\nComplete details can be seen in server log at FINE/FINER level " + From 2adbecf61322a6ca4418f1ec6073dacb24b794ee Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 08:52:09 +0200 Subject: [PATCH 19/42] CpsFlowExecution: refactor whole reportSuspectedMethodTooLarge() magic from parseScript() into a method of its own I first wanted it to just throw the exceptions, but IDEA was upset that we might return from the catch block and then use the uninitialized "s" variable. So this new method returns the entity which we clearly throw from the catch block. Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index b2dc65aa9..381ea3c71 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -631,19 +631,7 @@ Invoker createInvoker() { return LoggingInvoker.create(isSandbox()); } - private CpsScript parseScript() throws IOException { - // classloader hierarchy. See doc/classloader.md - CpsScript s; - try { - trusted = new CpsGroovyShellFactory(this).forTrusted().build(); - shell = new CpsGroovyShellFactory(this).withParent(trusted).build(); - - s = (CpsScript) shell.reparse("WorkflowScript", script); - - for (Entry e : loadedScripts.entrySet()) { - shell.reparse(e.getKey(), e.getValue()); - } - } catch (RuntimeException | Error x) { + protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // Suspected groovyjarjarasm.asm.MethodTooLargeException or a // org.codehaus.groovy.control.MultipleCompilationErrorsException // whose collection of errors refers to MethodTooLargeException. @@ -660,9 +648,6 @@ private CpsScript parseScript() throws IOException { final Pattern LINE_SEP_PATTERN = Pattern.compile("\\R"); String[] xLines = LINE_SEP_PATTERN.split(xStr); - // Clean up first - closeShells(); - if (x.getClass().getSimpleName().equals("MethodTooLargeException")) { mtlEx = x; ecCount = 1; @@ -731,7 +716,7 @@ private CpsScript parseScript() throws IOException { if (mtlEx == null || ecCount < 1) { // Some other exception type, or collection did not include MTL, rethrow as-is - throw x; + return x; } // Collect the relevant part of stack trace through groovy (JSL), @@ -793,7 +778,7 @@ private CpsScript parseScript() throws IOException { if (ecCount > 1) { // Not squashing with explicit MethodTooLargeException // re-thrown below, in this codepath we have other errors. - throw new RuntimeException(msg, x); + return new RuntimeException(msg, x); } else { // ecCount == 1 exactly, this is the only problem we saw. // Do not confuse pipeline devs by a wall of text in the @@ -802,11 +787,30 @@ private CpsScript parseScript() throws IOException { // a different logging verbosity level. METHOD_TOO_LARGE_LOGGER.log(Level.FINE, "CpsFlowExecution.reportSuspectedMethodTooLarge: detected details of MethodTooLargeException:\n" + mtlEx.getMessage()); - //throw new RuntimeException(msg, mtlEx); - throw new RuntimeException(msg + + //return new RuntimeException(msg, mtlEx); + return new RuntimeException(msg + "\nComplete details can be seen in server log at FINE/FINER level " + "(Jenkins admin access for " + METHOD_TOO_LARGE_LOGGER.getName() + " is required)"); } + } + + private CpsScript parseScript() throws IOException { + // classloader hierarchy. See doc/classloader.md + CpsScript s; + try { + trusted = new CpsGroovyShellFactory(this).forTrusted().build(); + shell = new CpsGroovyShellFactory(this).withParent(trusted).build(); + + s = (CpsScript) shell.reparse("WorkflowScript", script); + + for (Entry e : loadedScripts.entrySet()) { + shell.reparse(e.getKey(), e.getValue()); + } + } catch (RuntimeException | Error x) { + // Clean up first + closeShells(); + + throw CpsFlowExecution.reportSuspectedMethodTooLarge(x); } s.execution = this; From 5954624c70b8b32081f950f3e046a02abd59266c Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 10:31:23 +0200 Subject: [PATCH 20/42] CpsFlowExecution: parseScript(): IDEA complains for "throw reportSuspectedMethodTooLarge()" as too generic Curiously, before this PR #817 it had no qualms to "throw x" which could be a RuntimeException or Error, and the method itself only declares that it "throws IOException" which is neither. Even now explicitly-cast rethrows seem acceptable :\ Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 381ea3c71..13bedbcf7 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -810,7 +810,22 @@ private CpsScript parseScript() throws IOException { // Clean up first closeShells(); - throw CpsFlowExecution.reportSuspectedMethodTooLarge(x); + // This method ends up throwing something (original + // or changed exception, depending on situation). + // Here we anticipate a MethodTooLargeException + // (or traces of its message stack), possibly + // wrapped into further exception, for actionable + // logging in the job. + Throwable t = CpsFlowExecution.reportSuspectedMethodTooLarge(x); + if (t instanceof RuntimeException) + throw (RuntimeException)t; + if (t instanceof Error) + throw (Error)t; + + // NOTE: In practice we should not get here, due + // to practical type of "x" and what of it is + // returned by reportSuspectedMethodTooLarge(). + throw new RuntimeException(t); } s.execution = this; From dac4ca64e84d19faa7ce042d32f5cc3c43fd37ed Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 10:37:40 +0200 Subject: [PATCH 21/42] CpsScript: getProperty(): involve CpsFlowExecution.reportSuspectedMethodTooLarge(x) Some loaded properties are in fact calls to resolve code from a Jenkins Shared Library (steps aka global variables, or directly class methods if using script{} blocks and not just declarative). First accesses to such code cause CPS, compilation and possible failure with a MethodTooLargeException (maybe hidden in another Throwable) which we also want pretty-printed to be actionable for a pipeline developer. Signed-off-by: Jim Klimov --- .../jenkinsci/plugins/workflow/cps/CpsScript.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScript.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScript.java index d13da5073..e96e0858c 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScript.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScript.java @@ -134,8 +134,18 @@ public Object getProperty(String property) { GlobalVariable v = GlobalVariable.byName(property, b); if (v != null) { try { - return v.getValue(this); - } catch (Exception x) { + try { + return v.getValue(this); + } catch (RuntimeException | Error x) { + // This method ends up throwing something (original + // or changed exception, depending on situation). + // Here we anticipate a MethodTooLargeException + // (or traces of its message stack), possibly + // wrapped into further exception, for actionable + // logging in the job. + throw CpsFlowExecution.reportSuspectedMethodTooLarge(x); + } + } catch (Throwable x) { throw new InvokerInvocationException(x); } } From 4126b1118a2f3c6e75eba05cac74554732410b3f Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 08:55:23 +0200 Subject: [PATCH 22/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): fix indentations after refactoring out of earlier codebase Also avoid the final if/else to have a clear final code path Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 286 +++++++++--------- 1 file changed, 143 insertions(+), 143 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 13bedbcf7..f585bf708 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -632,166 +632,166 @@ Invoker createInvoker() { } protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { - // Suspected groovyjarjarasm.asm.MethodTooLargeException or a - // org.codehaus.groovy.control.MultipleCompilationErrorsException - // whose collection of errors refers to MethodTooLargeException. - // Per review comments, we do not want to statically compile a - // dependency on the groovyjarjarasm.asm.MethodTooLargeException - // internals, so gauge hitting it via String name comparisons. - // Other cases may be (subclasses of) RuntimeException or Error. - // Note that all of MultipleCompilationErrorsException, and - // MethodTooLargeException and CpsCompilationErrorsException - // are descended from RuntimeException. - Throwable mtlEx = null; - int ecCount = 0; - String xStr = x.getMessage() + "\n" + Functions.printThrowable(x); - final Pattern LINE_SEP_PATTERN = Pattern.compile("\\R"); - String[] xLines = LINE_SEP_PATTERN.split(xStr); - - if (x.getClass().getSimpleName().equals("MethodTooLargeException")) { - mtlEx = x; - ecCount = 1; - } else if (x instanceof MultipleCompilationErrorsException) { - ErrorCollector ec = ((MultipleCompilationErrorsException) x).getErrorCollector(); - ecCount = ec.getErrorCount(); - - for (int i = 0; i < ecCount; i++) { - Exception ex = ec.getException(i); - if (ex == null) - continue; - - METHOD_TOO_LARGE_LOGGER.log(Level.FINE, - "CpsFlowExecution.reportSuspectedMethodTooLarge: " + - "Collected Exception #" + i + ": " + ex.toString()); - if (ex.getClass().getSimpleName().equals("MethodTooLargeException")) { - mtlEx = ex; - break; - } + // Suspected groovyjarjarasm.asm.MethodTooLargeException or a + // org.codehaus.groovy.control.MultipleCompilationErrorsException + // whose collection of errors refers to MethodTooLargeException. + // Per review comments, we do not want to statically compile a + // dependency on the groovyjarjarasm.asm.MethodTooLargeException + // internals, so gauge hitting it via String name comparisons. + // Other cases may be (subclasses of) RuntimeException or Error. + // Note that all of MultipleCompilationErrorsException, and + // MethodTooLargeException and CpsCompilationErrorsException + // are descended from RuntimeException. + Throwable mtlEx = null; + int ecCount = 0; + String xStr = x.getMessage() + "\n" + Functions.printThrowable(x); + final Pattern LINE_SEP_PATTERN = Pattern.compile("\\R"); + String[] xLines = LINE_SEP_PATTERN.split(xStr); + + if (x.getClass().getSimpleName().equals("MethodTooLargeException")) { + mtlEx = x; + ecCount = 1; + } else if (x instanceof MultipleCompilationErrorsException) { + ErrorCollector ec = ((MultipleCompilationErrorsException) x).getErrorCollector(); + ecCount = ec.getErrorCount(); + + for (int i = 0; i < ecCount; i++) { + Exception ex = ec.getException(i); + if (ex == null) + continue; + + METHOD_TOO_LARGE_LOGGER.log(Level.FINE, + "CpsFlowExecution.reportSuspectedMethodTooLarge: " + + "Collected Exception #" + i + ": " + ex.toString()); + if (ex.getClass().getSimpleName().equals("MethodTooLargeException")) { + mtlEx = ex; + break; } - } else if (x instanceof CpsCompilationErrorsException) { - // Defined in this plugin, to clone a message and stack trace - // from a MultipleCompilationErrorsException and be serializable. - // Grep it as text for "MethodTooLargeException" and "1 error" - // (as a complete line, surrounded by blank lines, with no other - // similar lines in text) to be sure we've got it as the only - // problem. Note the code overflow may be not in "WorkflowScript" - // of the pipeline, but in a JSL step (global variable) or even - // class with an actual huge method that should be refactored. - if (xStr.contains("MethodTooLargeException")) { - final Pattern NUM_ERROR_PATTERN = Pattern.compile("^\\d+ error$"); - boolean blankBefore = false, patternMatchedAfterBlank = false; - - for (String l : xLines) { - if (l.isBlank()) { - // Is this the blank before or after the pattern we seek? - // Rule out several blank lines before the match, too... - if (!blankBefore && !patternMatchedAfterBlank) { - blankBefore = true; - } else if (patternMatchedAfterBlank) { - // Got a blank line after a pattern match - patternMatchedAfterBlank = false; - blankBefore = false; - ecCount++; - } - } else if (blankBefore) { - // Ignore pattern when no blank line was before it - Matcher matcher = NUM_ERROR_PATTERN.matcher(l); - if (matcher.find()) { - patternMatchedAfterBlank = true; - } else { - // red herring - blankBefore = false; - } - } else { - // part of wall of text + } + } else if (x instanceof CpsCompilationErrorsException) { + // Defined in this plugin, to clone a message and stack trace + // from a MultipleCompilationErrorsException and be serializable. + // Grep it as text for "MethodTooLargeException" and "1 error" + // (as a complete line, surrounded by blank lines, with no other + // similar lines in text) to be sure we've got it as the only + // problem. Note the code overflow may be not in "WorkflowScript" + // of the pipeline, but in a JSL step (global variable) or even + // class with an actual huge method that should be refactored. + if (xStr.contains("MethodTooLargeException")) { + final Pattern NUM_ERROR_PATTERN = Pattern.compile("^\\d+ error$"); + boolean blankBefore = false, patternMatchedAfterBlank = false; + + for (String l : xLines) { + if (l.isBlank()) { + // Is this the blank before or after the pattern we seek? + // Rule out several blank lines before the match, too... + if (!blankBefore && !patternMatchedAfterBlank) { + blankBefore = true; + } else if (patternMatchedAfterBlank) { + // Got a blank line after a pattern match patternMatchedAfterBlank = false; + blankBefore = false; + ecCount++; } + } else if (blankBefore) { + // Ignore pattern when no blank line was before it + Matcher matcher = NUM_ERROR_PATTERN.matcher(l); + if (matcher.find()) { + patternMatchedAfterBlank = true; + } else { + // red herring + blankBefore = false; + } + } else { + // part of wall of text + patternMatchedAfterBlank = false; } + } - if (ecCount > 0) { - mtlEx = x; - } + if (ecCount > 0) { + mtlEx = x; } } + } - if (mtlEx == null || ecCount < 1) { - // Some other exception type, or collection did not include MTL, rethrow as-is - return x; - } - - // Collect the relevant part of stack trace through groovy (JSL), - // if any, which the pipeline developer can impact and fix. - // Some real-life sample patterns are posted in - // https://github.com/jenkinsci/workflow-cps-plugin/pull/817 - String overflowedClassName = null; - List overflowedClassNameMentionsList = new ArrayList(); - // groovyjarjarasm.asm.MethodTooLargeException: Method too large: cloudBranch.___cps___586328 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; - final Pattern MTLE_CLASSNAME_PATTERN = Pattern.compile("^.*MethodTooLargeException.*: ([^\\s.]+)\\.___cps___\\d+.*$"); - Pattern CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|\\.groovy):\\d+\\).*$"); - for (String l : xLines) { - if (!(l.isBlank())) { - if (overflowedClassName == null) { - Matcher matcher = MTLE_CLASSNAME_PATTERN.matcher(l); - if (matcher.find()) { - try { - overflowedClassName = matcher.group(1); - if (!(mtlEx.getMessage().contains(overflowedClassName))) - overflowedClassNameMentionsList.add(l); - - // Update the matching pattern in case we manage - // to spot our problematic source in the stack trace - CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|" + overflowedClassName + ".*|\\.groovy):\\d+\\).*$"); - continue; - } catch (Throwable ignored) { - } - } - } + if (mtlEx == null || ecCount < 1) { + // Some other exception type, or collection did not include MTL, rethrow as-is + return x; + } - Matcher matcher = CLASSNAME_MENTIONS_PATTERN.matcher(l); + // Collect the relevant part of stack trace through groovy (JSL), + // if any, which the pipeline developer can impact and fix. + // Some real-life sample patterns are posted in + // https://github.com/jenkinsci/workflow-cps-plugin/pull/817 + String overflowedClassName = null; + List overflowedClassNameMentionsList = new ArrayList(); + // groovyjarjarasm.asm.MethodTooLargeException: Method too large: cloudBranch.___cps___586328 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + final Pattern MTLE_CLASSNAME_PATTERN = Pattern.compile("^.*MethodTooLargeException.*: ([^\\s.]+)\\.___cps___\\d+.*$"); + Pattern CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|\\.groovy):\\d+\\).*$"); + for (String l : xLines) { + if (!(l.isBlank())) { + if (overflowedClassName == null) { + Matcher matcher = MTLE_CLASSNAME_PATTERN.matcher(l); if (matcher.find()) { - overflowedClassNameMentionsList.add(l); + try { + overflowedClassName = matcher.group(1); + if (!(mtlEx.getMessage().contains(overflowedClassName))) + overflowedClassNameMentionsList.add(l); + + // Update the matching pattern in case we manage + // to spot our problematic source in the stack trace + CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|" + overflowedClassName + ".*|\\.groovy):\\d+\\).*$"); + continue; + } catch (Throwable ignored) { + } } } + + Matcher matcher = CLASSNAME_MENTIONS_PATTERN.matcher(l); + if (matcher.find()) { + overflowedClassNameMentionsList.add(l); + } } + } - if (overflowedClassName == null) - overflowedClassName = "WorkflowScript (the pipeline script) or one of its constituents"; + if (overflowedClassName == null) + overflowedClassName = "WorkflowScript (the pipeline script) or one of its constituents"; - String msg = "FAILED to parse " + overflowedClassName + " due to MethodTooLargeException"; - if (ecCount > 1) { - msg += " (and other issues)"; - } - // Short message suffices, not much that a pipeline developer - // can do with the stack trace into the guts of groovy - msg += "; please refactor to simplify code structure"; - if (overflowedClassName.contains("WorkflowScript")) - msg += " and/or move logic to a Jenkins Shared Library"; - msg += ": " + mtlEx.getMessage(); - if (!(overflowedClassNameMentionsList.isEmpty())) { - msg += "\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n" - + String.join("\n", overflowedClassNameMentionsList); - } + String msg = "FAILED to parse " + overflowedClassName + " due to MethodTooLargeException"; + if (ecCount > 1) { + msg += " (and other issues)"; + } + // Short message suffices, not much that a pipeline developer + // can do with the stack trace into the guts of groovy + msg += "; please refactor to simplify code structure"; + if (overflowedClassName.contains("WorkflowScript")) + msg += " and/or move logic to a Jenkins Shared Library"; + msg += ": " + mtlEx.getMessage(); + if (!(overflowedClassNameMentionsList.isEmpty())) { + msg += "\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n" + + String.join("\n", overflowedClassNameMentionsList); + } - // Make a full note in server log - METHOD_TOO_LARGE_LOGGER.log(Level.FINER, "CpsFlowExecution.reportSuspectedMethodTooLarge: full original Throwable message:\n" + xStr); + // Make a full note in server log + METHOD_TOO_LARGE_LOGGER.log(Level.FINER, "CpsFlowExecution.reportSuspectedMethodTooLarge: full original Throwable message:\n" + xStr); - if (ecCount > 1) { - // Not squashing with explicit MethodTooLargeException - // re-thrown below, in this codepath we have other errors. - return new RuntimeException(msg, x); - } else { - // ecCount == 1 exactly, this is the only problem we saw. - // Do not confuse pipeline devs by a wall of text in the - // build console, but let the full context be found in - // server log with some dedication. Note it is seen at - // a different logging verbosity level. - METHOD_TOO_LARGE_LOGGER.log(Level.FINE, "CpsFlowExecution.reportSuspectedMethodTooLarge: detected details of MethodTooLargeException:\n" + mtlEx.getMessage()); - - //return new RuntimeException(msg, mtlEx); - return new RuntimeException(msg + - "\nComplete details can be seen in server log at FINE/FINER level " + - "(Jenkins admin access for " + METHOD_TOO_LARGE_LOGGER.getName() + " is required)"); - } + if (ecCount > 1) { + // Not squashing with explicit MethodTooLargeException + // re-thrown below, in this codepath we have other errors. + return new RuntimeException(msg, x); + } + + // ecCount == 1 exactly, this is the only problem we saw. + // Do not confuse pipeline devs by a wall of text in the + // build console, but let the full context be found in + // server log with some dedication. Note it is seen at + // a different logging verbosity level. + METHOD_TOO_LARGE_LOGGER.log(Level.FINE, "CpsFlowExecution.reportSuspectedMethodTooLarge: detected details of MethodTooLargeException:\n" + mtlEx.getMessage()); + + //return new RuntimeException(msg, mtlEx); + return new RuntimeException(msg + + "\nComplete details can be seen in server log at FINE/FINER level " + + "(Jenkins admin access for " + METHOD_TOO_LARGE_LOGGER.getName() + " is required)"); } private CpsScript parseScript() throws IOException { From b078e7a9dda21a34f07bb28d3ab3c3710bd78f01 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 09:01:49 +0200 Subject: [PATCH 23/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): when returning short exception with a curated message to go into job log, chop off the stack trace leading to the reporter Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index f585bf708..70421ad78 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -789,9 +789,15 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { METHOD_TOO_LARGE_LOGGER.log(Level.FINE, "CpsFlowExecution.reportSuspectedMethodTooLarge: detected details of MethodTooLargeException:\n" + mtlEx.getMessage()); //return new RuntimeException(msg, mtlEx); - return new RuntimeException(msg + + mtlEx = new RuntimeException(msg + "\nComplete details can be seen in server log at FINE/FINER level " + - "(Jenkins admin access for " + METHOD_TOO_LARGE_LOGGER.getName() + " is required)"); + "(Jenkins admin access for " + METHOD_TOO_LARGE_LOGGER.getName() + " is required)", + null); + + // Avoid having a stack trace leading to this pretty log-printer in the build log + StackTraceElement[] emptyStack = new StackTraceElement[0]; + mtlEx.setStackTrace(emptyStack); + return mtlEx; } private CpsScript parseScript() throws IOException { From b02afc313a9c44e74572816d1dfd4ad9f0f96c67 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 09:03:20 +0200 Subject: [PATCH 24/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): xStr should not include x.getMessage(), a copy is already there Signed-off-by: Jim Klimov --- .../org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 70421ad78..6d0359f84 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -644,7 +644,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // are descended from RuntimeException. Throwable mtlEx = null; int ecCount = 0; - String xStr = x.getMessage() + "\n" + Functions.printThrowable(x); + String xStr = Functions.printThrowable(x); // includes x.getMessage() contents final Pattern LINE_SEP_PATTERN = Pattern.compile("\\R"); String[] xLines = LINE_SEP_PATTERN.split(xStr); From 5336cbecb0d968e326c5d4b3b435f2680fcaf2b1 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 09:05:07 +0200 Subject: [PATCH 25/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): at least CpsCompilationErrorsException.getMessage() is too long with all the original stack trace Introduce an xMsgStart with curated start of original Throwable's message (until the first "\tat somewhere(file:line)" match). Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 6d0359f84..9602103ff 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -727,15 +727,37 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { List overflowedClassNameMentionsList = new ArrayList(); // groovyjarjarasm.asm.MethodTooLargeException: Method too large: cloudBranch.___cps___586328 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; final Pattern MTLE_CLASSNAME_PATTERN = Pattern.compile("^.*MethodTooLargeException.*: ([^\\s.]+)\\.___cps___\\d+.*$"); + + // Collect text of just the start of original exception + // (at least CpsCompilationErrorsException carries the + // whole original stack trace there) + final Pattern SAW_AT_PATTERN = Pattern.compile("^\\s+at .*:\\d+\\)$"); + StringBuilder xMsgStart = new StringBuilder(); + boolean sawAt = false; + Pattern CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|\\.groovy):\\d+\\).*$"); + for (String l : xLines) { if (!(l.isBlank())) { + if (!sawAt) { + Matcher matcher = SAW_AT_PATTERN.matcher(l); + if (matcher.find()) { + sawAt = true; + } else { + xMsgStart.append(l).append("\n"); + } + } + if (overflowedClassName == null) { Matcher matcher = MTLE_CLASSNAME_PATTERN.matcher(l); if (matcher.find()) { try { overflowedClassName = matcher.group(1); - if (!(mtlEx.getMessage().contains(overflowedClassName))) + + // Only report it in potential bread-crumb log if we + // did not have a reference to this script/step/class + // from the start of x.getMessage() effectively. + if (!(xMsgStart.toString().contains(overflowedClassName))) overflowedClassNameMentionsList.add(l); // Update the matching pattern in case we manage @@ -766,7 +788,9 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { msg += "; please refactor to simplify code structure"; if (overflowedClassName.contains("WorkflowScript")) msg += " and/or move logic to a Jenkins Shared Library"; - msg += ": " + mtlEx.getMessage(); + if (xMsgStart.length() > 0) { + msg += ": " + xMsgStart.toString(); + } if (!(overflowedClassNameMentionsList.isEmpty())) { msg += "\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n" + String.join("\n", overflowedClassNameMentionsList); From 4e19e1f081836988059df29d47d9e58d3afe01cf Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 09:10:01 +0200 Subject: [PATCH 26/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): describe what we see in pretty-printed overflowedClassName Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 9602103ff..09f036fe4 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -776,17 +776,25 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { } } - if (overflowedClassName == null) - overflowedClassName = "WorkflowScript (the pipeline script) or one of its constituents"; + String overflowedClassNameReport; + if (overflowedClassName == null) { + overflowedClassNameReport = "WorkflowScript (the pipeline script) or one of its constituents"; + } else if (overflowedClassName.equals("WorkflowScript")) { + overflowedClassNameReport = "WorkflowScript (the pipeline script)"; + } else { + // quote the step/class name pretty: + // FAILED to parse 'stepName' due to... + overflowedClassNameReport = "'" + overflowedClassName + "'"; + } - String msg = "FAILED to parse " + overflowedClassName + " due to MethodTooLargeException"; + String msg = "FAILED to parse " + overflowedClassNameReport + " due to MethodTooLargeException"; if (ecCount > 1) { msg += " (and other issues)"; } // Short message suffices, not much that a pipeline developer // can do with the stack trace into the guts of groovy msg += "; please refactor to simplify code structure"; - if (overflowedClassName.contains("WorkflowScript")) + if (overflowedClassNameReport.contains("WorkflowScript")) msg += " and/or move logic to a Jenkins Shared Library"; if (xMsgStart.length() > 0) { msg += ": " + xMsgStart.toString(); From 93adf33f4cb157227ff1d6c9ee25cf5a65c18cd8 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 09:27:05 +0200 Subject: [PATCH 27/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): document MTLE_CLASSNAME_PATTERN and CLASSNAME_MENTIONS_PATTERN expectations/assumptions in code Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 09f036fe4..2ac216d10 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -725,7 +725,15 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // https://github.com/jenkinsci/workflow-cps-plugin/pull/817 String overflowedClassName = null; List overflowedClassNameMentionsList = new ArrayList(); - // groovyjarjarasm.asm.MethodTooLargeException: Method too large: cloudBranch.___cps___586328 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + + // For this matcher, caught patterns of interest include: + // * alphanumeric-only token: step (global variable) from a Jenkins shared library + // groovyjarjarasm.asm.MethodTooLargeException: Method too large: cloudBranch.___cps___586328 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + // * ... or a name generated by this plugin for pipeline script + // (variants detailed below): + // groovyjarjarasm.asm.MethodTooLargeException: Method too large: WorkflowScript.___cps___20692 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + // * alphanumeric token with slashes: class from a Jenkins shared library + // groovyjarjarasm.asm.MethodTooLargeException: Method too large: com/myproject/ci/BranchResync.___cps___679414 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; final Pattern MTLE_CLASSNAME_PATTERN = Pattern.compile("^.*MethodTooLargeException.*: ([^\\s.]+)\\.___cps___\\d+.*$"); // Collect text of just the start of original exception @@ -735,6 +743,12 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { StringBuilder xMsgStart = new StringBuilder(); boolean sawAt = false; + // Match a number of interesting source code names, or + // what we assume them to be in overflowedClassName: + // * "WorkflowScript": generated by CpsFlowExecution.parseScript() below + // * "*.groovy:LINENUM": possible path through Jenkins shared library + // (if pipeline and some steps/classes are okay, and call one too big) + // * later would add overflowedClassName if/when we detect one Pattern CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|\\.groovy):\\d+\\).*$"); for (String l : xLines) { From a5123688ef2ee74aed036726ca46fefd95f36bcd Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 10:16:43 +0200 Subject: [PATCH 28/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): overflowedClassName may have slashes for class or not for step Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 2ac216d10..8c7e12639 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -724,6 +724,9 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // Some real-life sample patterns are posted in // https://github.com/jenkinsci/workflow-cps-plugin/pull/817 String overflowedClassName = null; + // Use the short "base name" string of the detected class name + // for subsequent matching of bread-crumbs: + String overflowedClassNameShort = null; List overflowedClassNameMentionsList = new ArrayList(); // For this matcher, caught patterns of interest include: @@ -748,7 +751,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // * "WorkflowScript": generated by CpsFlowExecution.parseScript() below // * "*.groovy:LINENUM": possible path through Jenkins shared library // (if pipeline and some steps/classes are okay, and call one too big) - // * later would add overflowedClassName if/when we detect one + // * later would add overflowedClassNameShort if/when we detect one Pattern CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|\\.groovy):\\d+\\).*$"); for (String l : xLines) { @@ -774,9 +777,16 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { if (!(xMsgStart.toString().contains(overflowedClassName))) overflowedClassNameMentionsList.add(l); + String[] overflowedClassNameSplit = overflowedClassName.split("/"); + if (overflowedClassNameSplit.length > 1) { + overflowedClassNameShort = overflowedClassNameSplit[overflowedClassNameSplit.length - 1]; + } else { + overflowedClassNameShort = overflowedClassName; + } + // Update the matching pattern in case we manage // to spot our problematic source in the stack trace - CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|" + overflowedClassName + ".*|\\.groovy):\\d+\\).*$"); + CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|" + overflowedClassNameShort + ".*|\\.groovy):\\d+\\).*$"); continue; } catch (Throwable ignored) { } @@ -795,10 +805,14 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { overflowedClassNameReport = "WorkflowScript (the pipeline script) or one of its constituents"; } else if (overflowedClassName.equals("WorkflowScript")) { overflowedClassNameReport = "WorkflowScript (the pipeline script)"; + } else if (overflowedClassName.contains("/")) { + // quote the step/class name pretty: + // FAILED to parse 'stepName' due to... + overflowedClassNameReport = "presumed JSL class '" + overflowedClassName + "'"; } else { // quote the step/class name pretty: // FAILED to parse 'stepName' due to... - overflowedClassNameReport = "'" + overflowedClassName + "'"; + overflowedClassNameReport = "presumed JSL step '" + overflowedClassName + "'"; } String msg = "FAILED to parse " + overflowedClassNameReport + " due to MethodTooLargeException"; From bfa34cb351cc1fc290f3ee58c9bcfe87ac9bf766 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 10:28:31 +0200 Subject: [PATCH 29/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): support also pipeline scripts named like "Script" Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 8c7e12639..144a332e1 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -737,6 +737,8 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // groovyjarjarasm.asm.MethodTooLargeException: Method too large: WorkflowScript.___cps___20692 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; // * alphanumeric token with slashes: class from a Jenkins shared library // groovyjarjarasm.asm.MethodTooLargeException: Method too large: com/myproject/ci/BranchResync.___cps___679414 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + // Note that we do not include a "." character, so any ".run", ".pipeline()" + // or ".groovy" suffix is not in overflowedClassName* strings. final Pattern MTLE_CLASSNAME_PATTERN = Pattern.compile("^.*MethodTooLargeException.*: ([^\\s.]+)\\.___cps___\\d+.*$"); // Collect text of just the start of original exception @@ -749,10 +751,13 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // Match a number of interesting source code names, or // what we assume them to be in overflowedClassName: // * "WorkflowScript": generated by CpsFlowExecution.parseScript() below + // * "Script": generated by CpsGroovyShell.generateScriptName() // * "*.groovy:LINENUM": possible path through Jenkins shared library // (if pipeline and some steps/classes are okay, and call one too big) // * later would add overflowedClassNameShort if/when we detect one - Pattern CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|\\.groovy):\\d+\\).*$"); + Pattern CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|Script\\d+|\\.groovy):\\d+\\).*$"); + // Used in a few checks later for the "Script" case: + Pattern CLASSNAME_SCRIPTNUM_PATTERN = Pattern.compile("^Script\\d+$"); for (String l : xLines) { if (!(l.isBlank())) { @@ -786,7 +791,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // Update the matching pattern in case we manage // to spot our problematic source in the stack trace - CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|" + overflowedClassNameShort + ".*|\\.groovy):\\d+\\).*$"); + CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|Script\\d+|" + overflowedClassNameShort + ".*|\\.groovy):\\d+\\).*$"); continue; } catch (Throwable ignored) { } @@ -803,8 +808,8 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { String overflowedClassNameReport; if (overflowedClassName == null) { overflowedClassNameReport = "WorkflowScript (the pipeline script) or one of its constituents"; - } else if (overflowedClassName.equals("WorkflowScript")) { - overflowedClassNameReport = "WorkflowScript (the pipeline script)"; + } else if (overflowedClassName.equals("WorkflowScript") || CLASSNAME_SCRIPTNUM_PATTERN.matcher(overflowedClassName).find()) { + overflowedClassNameReport = overflowedClassName + " (the pipeline script)"; } else if (overflowedClassName.contains("/")) { // quote the step/class name pretty: // FAILED to parse 'stepName' due to... @@ -822,7 +827,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // Short message suffices, not much that a pipeline developer // can do with the stack trace into the guts of groovy msg += "; please refactor to simplify code structure"; - if (overflowedClassNameReport.contains("WorkflowScript")) + if (overflowedClassNameReport.contains("WorkflowScript") || CLASSNAME_SCRIPTNUM_PATTERN.matcher(overflowedClassName).find()) msg += " and/or move logic to a Jenkins Shared Library"; if (xMsgStart.length() > 0) { msg += ": " + xMsgStart.toString(); From 2fd729968448727f0cf3c726130462649e86a010 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 11:26:15 +0200 Subject: [PATCH 30/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): wrap original log excerpts into snip-markers to separate visibly Signed-off-by: Jim Klimov --- .../org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 144a332e1..ebea96ff6 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -830,12 +830,15 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { if (overflowedClassNameReport.contains("WorkflowScript") || CLASSNAME_SCRIPTNUM_PATTERN.matcher(overflowedClassName).find()) msg += " and/or move logic to a Jenkins Shared Library"; if (xMsgStart.length() > 0) { - msg += ": " + xMsgStart.toString(); + msg += ":\n-----\n" + xMsgStart.toString(); } if (!(overflowedClassNameMentionsList.isEmpty())) { msg += "\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n" + String.join("\n", overflowedClassNameMentionsList); } + if (xMsgStart.length() > 0) { + msg += "\n-----\n"; + } // Make a full note in server log METHOD_TOO_LARGE_LOGGER.log(Level.FINER, "CpsFlowExecution.reportSuspectedMethodTooLarge: full original Throwable message:\n" + xStr); From c5e79daf457337b3c49e1b17f8b1c289392f84e8 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 11:36:30 +0200 Subject: [PATCH 31/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): refactor actionableMsg as a StringBuilder Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 52 ++++++++++++------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index ebea96ff6..193b2d224 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -820,33 +820,42 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { overflowedClassNameReport = "presumed JSL step '" + overflowedClassName + "'"; } - String msg = "FAILED to parse " + overflowedClassNameReport + " due to MethodTooLargeException"; - if (ecCount > 1) { - msg += " (and other issues)"; - } // Short message suffices, not much that a pipeline developer // can do with the stack trace into the guts of groovy - msg += "; please refactor to simplify code structure"; + StringBuilder actionableMsg = new StringBuilder(); + actionableMsg + .append("FAILED to parse ") + .append(overflowedClassNameReport) + .append(" due to MethodTooLargeException"); + if (ecCount > 1) { + actionableMsg.append(" (and other issues)"); + } + actionableMsg.append("; please refactor to simplify code structure"); if (overflowedClassNameReport.contains("WorkflowScript") || CLASSNAME_SCRIPTNUM_PATTERN.matcher(overflowedClassName).find()) - msg += " and/or move logic to a Jenkins Shared Library"; + actionableMsg.append(" and/or move logic to a Jenkins Shared Library"); if (xMsgStart.length() > 0) { - msg += ":\n-----\n" + xMsgStart.toString(); + actionableMsg + .append(":\n-----\n") + .append(xMsgStart.toString()); } if (!(overflowedClassNameMentionsList.isEmpty())) { - msg += "\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n" - + String.join("\n", overflowedClassNameMentionsList); + actionableMsg + .append("\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n") + .append(String.join("\n", overflowedClassNameMentionsList)); } if (xMsgStart.length() > 0) { - msg += "\n-----\n"; + actionableMsg.append("\n-----\n"); } // Make a full note in server log - METHOD_TOO_LARGE_LOGGER.log(Level.FINER, "CpsFlowExecution.reportSuspectedMethodTooLarge: full original Throwable message:\n" + xStr); + METHOD_TOO_LARGE_LOGGER.log(Level.FINER, + "CpsFlowExecution.reportSuspectedMethodTooLarge: full original Throwable message:\n" + + xStr); if (ecCount > 1) { // Not squashing with explicit MethodTooLargeException // re-thrown below, in this codepath we have other errors. - return new RuntimeException(msg, x); + return new RuntimeException(actionableMsg.toString(), x); } // ecCount == 1 exactly, this is the only problem we saw. @@ -854,13 +863,18 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // build console, but let the full context be found in // server log with some dedication. Note it is seen at // a different logging verbosity level. - METHOD_TOO_LARGE_LOGGER.log(Level.FINE, "CpsFlowExecution.reportSuspectedMethodTooLarge: detected details of MethodTooLargeException:\n" + mtlEx.getMessage()); - - //return new RuntimeException(msg, mtlEx); - mtlEx = new RuntimeException(msg + - "\nComplete details can be seen in server log at FINE/FINER level " + - "(Jenkins admin access for " + METHOD_TOO_LARGE_LOGGER.getName() + " is required)", - null); + METHOD_TOO_LARGE_LOGGER.log(Level.FINE, + "CpsFlowExecution.reportSuspectedMethodTooLarge: detected details of MethodTooLargeException:\n" + + mtlEx.getMessage()); + + actionableMsg + .append("\nComplete details can be seen in server log at FINE/FINER level ") + .append("(Jenkins admin access for ") + .append(METHOD_TOO_LARGE_LOGGER.getName()) + .append(" is required)"); + + //return new RuntimeException(actionableMsg.toString(), mtlEx); + mtlEx = new RuntimeException(actionableMsg.toString(), null); // Avoid having a stack trace leading to this pretty log-printer in the build log StackTraceElement[] emptyStack = new StackTraceElement[0]; From 4d39666fab9060d07ca3e095f43179d2d97552a0 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 11:41:36 +0200 Subject: [PATCH 32/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): refactor overflowedClassNameBreadcrumbs as a StringBuilder Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 193b2d224..6216399b2 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -727,7 +727,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // Use the short "base name" string of the detected class name // for subsequent matching of bread-crumbs: String overflowedClassNameShort = null; - List overflowedClassNameMentionsList = new ArrayList(); + StringBuilder overflowedClassNameBreadcrumbs = new StringBuilder(); // For this matcher, caught patterns of interest include: // * alphanumeric-only token: step (global variable) from a Jenkins shared library @@ -780,7 +780,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // did not have a reference to this script/step/class // from the start of x.getMessage() effectively. if (!(xMsgStart.toString().contains(overflowedClassName))) - overflowedClassNameMentionsList.add(l); + overflowedClassNameBreadcrumbs.append(l).append("\n"); String[] overflowedClassNameSplit = overflowedClassName.split("/"); if (overflowedClassNameSplit.length > 1) { @@ -800,7 +800,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { Matcher matcher = CLASSNAME_MENTIONS_PATTERN.matcher(l); if (matcher.find()) { - overflowedClassNameMentionsList.add(l); + overflowedClassNameBreadcrumbs.append(l).append("\n"); } } } @@ -838,10 +838,10 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { .append(":\n-----\n") .append(xMsgStart.toString()); } - if (!(overflowedClassNameMentionsList.isEmpty())) { + if (overflowedClassNameBreadcrumbs.length() > 0) { actionableMsg .append("\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n") - .append(String.join("\n", overflowedClassNameMentionsList)); + .append(overflowedClassNameBreadcrumbs); } if (xMsgStart.length() > 0) { actionableMsg.append("\n-----\n"); From 4f5ab706bc0f539f373e773e52f67872629854f8 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 12:17:41 +0200 Subject: [PATCH 33/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): match by *short* overflowedClassName for overflowedClassNameBreadcrumbs Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 6216399b2..5ef90e438 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -776,12 +776,6 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { try { overflowedClassName = matcher.group(1); - // Only report it in potential bread-crumb log if we - // did not have a reference to this script/step/class - // from the start of x.getMessage() effectively. - if (!(xMsgStart.toString().contains(overflowedClassName))) - overflowedClassNameBreadcrumbs.append(l).append("\n"); - String[] overflowedClassNameSplit = overflowedClassName.split("/"); if (overflowedClassNameSplit.length > 1) { overflowedClassNameShort = overflowedClassNameSplit[overflowedClassNameSplit.length - 1]; @@ -789,6 +783,14 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { overflowedClassNameShort = overflowedClassName; } + // Only report it in potential bread-crumb log if we + // did not have a reference to this script/step/class + // from the start of x.getMessage() effectively. + // Note this is not a log line where we have a source + // line number. + if (!(xMsgStart.toString().contains(overflowedClassNameShort))) + overflowedClassNameBreadcrumbs.append(l).append("\n"); + // Update the matching pattern in case we manage // to spot our problematic source in the stack trace CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|Script\\d+|" + overflowedClassNameShort + ".*|\\.groovy):\\d+\\).*$"); From 9491ce1059d0222d68d8b9465c0b230be93f6d7f Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 13:03:26 +0200 Subject: [PATCH 34/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): wrap original log excerpts - only add a newline before final wrapper if not present in wrapped text Signed-off-by: Jim Klimov --- .../org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 5ef90e438..3cd83fb76 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -846,7 +846,9 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { .append(overflowedClassNameBreadcrumbs); } if (xMsgStart.length() > 0) { - actionableMsg.append("\n-----\n"); + if (!(actionableMsg.substring(actionableMsg.length() - 1).equals("\n"))) + actionableMsg.append("\n"); + actionableMsg.append("-----\n"); } // Make a full note in server log From 99362bc51afe1516506b9061e648bee28dab4ceb Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 19:31:40 +0200 Subject: [PATCH 35/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): comment about "interaction" with ContinuationGroup.fixupStackTrace() Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 3cd83fb76..f86866050 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -729,6 +729,26 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { String overflowedClassNameShort = null; StringBuilder overflowedClassNameBreadcrumbs = new StringBuilder(); + // FIXME: After initial development and testing it was found that + // the part of the stack trace originally reported in the build + // log, with "bread-crumbs" through the WorkflowScript and maybe + // *.groovy files, with a "at ___cps.transform___(Native Method)" + // (synthesized entry via Continuable.SEPARATOR_STACK_ELEMENT) + // was constructed by ContinuationGroup.fixupStackTrace() as + // called from PropertyishBlock.ContinuationImpl.get() in the + // groovy-cps library (see sources nearby in this project). + // Such patched-up Throwable combines the "real" exception + // call stack of broken code with that of the asynchronous + // CPS caller, and is then injected into the particular env's + // "ExceptionHandler" to eventually end up in the build log. + // This here log trimmer/parser should probably be refactored + // into a method or even class (PrettyMethodTooLargeException) + // in *that* library to directly impact the "get()" exception + // behavior for global variables, ultimately, and so to benefit + // slightly from this bit of tracing at that point (two methods + // which call it now should not anymore, to pass all needed + // stack info for mangling to that new decision point). + // For this matcher, caught patterns of interest include: // * alphanumeric-only token: step (global variable) from a Jenkins shared library // groovyjarjarasm.asm.MethodTooLargeException: Method too large: cloudBranch.___cps___586328 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; @@ -880,7 +900,13 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { //return new RuntimeException(actionableMsg.toString(), mtlEx); mtlEx = new RuntimeException(actionableMsg.toString(), null); - // Avoid having a stack trace leading to this pretty log-printer in the build log + // Avoid having a huge stack trace leading to this pretty log-printer + // in the build log. + // Technically, ContinuationGroup.fixupStackTrace() uses common + // parts of the "real" and CPS-caller stack traces to inject the + // async call parts, or skips the hassle if the two stack trace + // lists have different "roots" (as non-trivially defined in + // ContinuationGroup.hasSameRoots() method). StackTraceElement[] emptyStack = new StackTraceElement[0]; mtlEx.setStackTrace(emptyStack); return mtlEx; From 343c5da760b11e93dde1fd219fcf8711a44dfadd Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 21:24:58 +0200 Subject: [PATCH 36/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): do not hijack mtlEx in the end, use a dedicated "RuntimeException rtex" object Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index f86866050..4f76dde86 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -898,7 +898,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { .append(" is required)"); //return new RuntimeException(actionableMsg.toString(), mtlEx); - mtlEx = new RuntimeException(actionableMsg.toString(), null); + RuntimeException rtex = new RuntimeException(actionableMsg.toString(), null); // Avoid having a huge stack trace leading to this pretty log-printer // in the build log. @@ -907,9 +907,14 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // async call parts, or skips the hassle if the two stack trace // lists have different "roots" (as non-trivially defined in // ContinuationGroup.hasSameRoots() method). - StackTraceElement[] emptyStack = new StackTraceElement[0]; - mtlEx.setStackTrace(emptyStack); - return mtlEx; + StackTraceElement[] shortStack = new StackTraceElement[0]; + rtex.setStackTrace(shortStack); + + // Considered passing original context, + // but it is shown by ultimate job log :( + //rtex.addSuppressed(mtlEx); + + return rtex; } private CpsScript parseScript() throws IOException { From 82edfc148b5a17a2273bdc5e7888106705616d8d Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 21:41:59 +0200 Subject: [PATCH 37/42] CpsFlowExecution: reportSuspectedMethodTooLarge(): avoid extra blank line Signed-off-by: Jim Klimov --- .../org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 4f76dde86..7b90fb560 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -868,7 +868,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { if (xMsgStart.length() > 0) { if (!(actionableMsg.substring(actionableMsg.length() - 1).equals("\n"))) actionableMsg.append("\n"); - actionableMsg.append("-----\n"); + actionableMsg.append("-----"); } // Make a full note in server log From 337cc4e3e5ba9dcef6efc8b863c9a69f9d400a12 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2024 22:12:48 +0200 Subject: [PATCH 38/42] CpsScriptTest: methodTooLargeExceptionRealistic(): expect updated wording for logged message Signed-off-by: Jim Klimov --- .../java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java index a13d0f2d6..2bb9223f5 100644 --- a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java +++ b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java @@ -168,7 +168,7 @@ public void evaluateShallSandbox() throws Exception { r.assertLogContains("MethodTooLargeException", b); // "Prettier" explanation added by CpsFlowExecution.parseScript(): - r.assertLogContains("FAILED to parse WorkflowScript due to MethodTooLargeException", b); + r.assertLogContains("FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException", b); /* // Report as of release 3880.vb_ef4b_5cfd270 (Feb 2024) From ede9176c0980ab9b2e987d1b2d9ca7297c287868 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 4 Oct 2025 17:37:05 +0200 Subject: [PATCH 39/42] CpsFlowExecution, CpsScriptTest: mvn spotless:apply Signed-off-by: Jim Klimov --- .../workflow/cps/CpsFlowExecution.java | 75 +++++++----- .../plugins/workflow/cps/CpsScriptTest.java | 111 +++++++++--------- 2 files changed, 102 insertions(+), 84 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 5a4514f86..2df4e1238 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -125,9 +125,9 @@ import jenkins.model.Jenkins; import jenkins.util.SystemProperties; import net.jcip.annotations.GuardedBy; +import org.codehaus.groovy.GroovyBugError; import org.codehaus.groovy.control.ErrorCollector; import org.codehaus.groovy.control.MultipleCompilationErrorsException; -import org.codehaus.groovy.GroovyBugError; import org.jboss.marshalling.Unmarshaller; import org.jboss.marshalling.reflect.SerializableClassRegistry; import org.jenkinsci.plugins.workflow.actions.ErrorAction; @@ -485,7 +485,8 @@ Timing time(TimingKind kind) { static final Logger TIMING_LOGGER = Logger.getLogger(CpsFlowExecution.class.getName() + ".timing"); - static final Logger METHOD_TOO_LARGE_LOGGER = Logger.getLogger(CpsFlowExecution.class.getName() + ".MethodTooLargeLogging"); + static final Logger METHOD_TOO_LARGE_LOGGER = + Logger.getLogger(CpsFlowExecution.class.getName() + ".MethodTooLargeLogging"); void logTimings() { if (TIMING_LOGGER.isLoggable(Level.FINE)) { @@ -691,12 +692,12 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { for (int i = 0; i < ecCount; i++) { Exception ex = ec.getException(i); - if (ex == null) - continue; + if (ex == null) continue; - METHOD_TOO_LARGE_LOGGER.log(Level.FINE, - "CpsFlowExecution.reportSuspectedMethodTooLarge: " + - "Collected Exception #" + i + ": " + ex.toString()); + METHOD_TOO_LARGE_LOGGER.log( + Level.FINE, + "CpsFlowExecution.reportSuspectedMethodTooLarge: " + "Collected Exception #" + i + ": " + + ex.toString()); if (ex.getClass().getSimpleName().equals("MethodTooLargeException")) { mtlEx = ex; break; @@ -785,15 +786,22 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // For this matcher, caught patterns of interest include: // * alphanumeric-only token: step (global variable) from a Jenkins shared library - // groovyjarjarasm.asm.MethodTooLargeException: Method too large: cloudBranch.___cps___586328 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + // groovyjarjarasm.asm.MethodTooLargeException: + // Method too large: cloudBranch.___cps___586328 + // ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; // * ... or a name generated by this plugin for pipeline script // (variants detailed below): - // groovyjarjarasm.asm.MethodTooLargeException: Method too large: WorkflowScript.___cps___20692 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + // groovyjarjarasm.asm.MethodTooLargeException: + // Method too large: WorkflowScript.___cps___20692 + // ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; // * alphanumeric token with slashes: class from a Jenkins shared library - // groovyjarjarasm.asm.MethodTooLargeException: Method too large: com/myproject/ci/BranchResync.___cps___679414 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + // groovyjarjarasm.asm.MethodTooLargeException: + // Method too large: com/myproject/ci/BranchResync.___cps___679414 + // ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; // Note that we do not include a "." character, so any ".run", ".pipeline()" // or ".groovy" suffix is not in overflowedClassName* strings. - final Pattern MTLE_CLASSNAME_PATTERN = Pattern.compile("^.*MethodTooLargeException.*: ([^\\s.]+)\\.___cps___\\d+.*$"); + final Pattern MTLE_CLASSNAME_PATTERN = + Pattern.compile("^.*MethodTooLargeException.*: ([^\\s.]+)\\.___cps___\\d+.*$"); // Collect text of just the start of original exception // (at least CpsCompilationErrorsException carries the @@ -809,7 +817,8 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // * "*.groovy:LINENUM": possible path through Jenkins shared library // (if pipeline and some steps/classes are okay, and call one too big) // * later would add overflowedClassNameShort if/when we detect one - Pattern CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|Script\\d+|\\.groovy):\\d+\\).*$"); + Pattern CLASSNAME_MENTIONS_PATTERN = + Pattern.compile("^\\s+at .*(WorkflowScript.*|Script\\d+|\\.groovy):\\d+\\).*$"); // Used in a few checks later for the "Script" case: Pattern CLASSNAME_SCRIPTNUM_PATTERN = Pattern.compile("^Script\\d+$"); @@ -832,7 +841,8 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { String[] overflowedClassNameSplit = overflowedClassName.split("/"); if (overflowedClassNameSplit.length > 1) { - overflowedClassNameShort = overflowedClassNameSplit[overflowedClassNameSplit.length - 1]; + overflowedClassNameShort = + overflowedClassNameSplit[overflowedClassNameSplit.length - 1]; } else { overflowedClassNameShort = overflowedClassName; } @@ -847,7 +857,8 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // Update the matching pattern in case we manage // to spot our problematic source in the stack trace - CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|Script\\d+|" + overflowedClassNameShort + ".*|\\.groovy):\\d+\\).*$"); + CLASSNAME_MENTIONS_PATTERN = Pattern.compile("^\\s+at .*(WorkflowScript.*|Script\\d+|" + + overflowedClassNameShort + ".*|\\.groovy):\\d+\\).*$"); continue; } catch (Throwable ignored) { } @@ -864,7 +875,8 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { String overflowedClassNameReport; if (overflowedClassName == null) { overflowedClassNameReport = "WorkflowScript (the pipeline script) or one of its constituents"; - } else if (overflowedClassName.equals("WorkflowScript") || CLASSNAME_SCRIPTNUM_PATTERN.matcher(overflowedClassName).find()) { + } else if (overflowedClassName.equals("WorkflowScript") + || CLASSNAME_SCRIPTNUM_PATTERN.matcher(overflowedClassName).find()) { overflowedClassNameReport = overflowedClassName + " (the pipeline script)"; } else if (overflowedClassName.contains("/")) { // quote the step/class name pretty: @@ -887,28 +899,30 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { actionableMsg.append(" (and other issues)"); } actionableMsg.append("; please refactor to simplify code structure"); - if (overflowedClassNameReport.contains("WorkflowScript") || CLASSNAME_SCRIPTNUM_PATTERN.matcher(overflowedClassName).find()) + if (overflowedClassNameReport.contains("WorkflowScript") + || CLASSNAME_SCRIPTNUM_PATTERN.matcher(overflowedClassName).find()) { actionableMsg.append(" and/or move logic to a Jenkins Shared Library"); + } if (xMsgStart.length() > 0) { - actionableMsg - .append(":\n-----\n") - .append(xMsgStart.toString()); + actionableMsg.append(":\n-----\n").append(xMsgStart.toString()); } if (overflowedClassNameBreadcrumbs.length() > 0) { actionableMsg - .append("\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n") + .append( + "\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n") .append(overflowedClassNameBreadcrumbs); } if (xMsgStart.length() > 0) { - if (!(actionableMsg.substring(actionableMsg.length() - 1).equals("\n"))) + if (!(actionableMsg.substring(actionableMsg.length() - 1).equals("\n"))) { actionableMsg.append("\n"); + } actionableMsg.append("-----"); } // Make a full note in server log - METHOD_TOO_LARGE_LOGGER.log(Level.FINER, - "CpsFlowExecution.reportSuspectedMethodTooLarge: full original Throwable message:\n" - + xStr); + METHOD_TOO_LARGE_LOGGER.log( + Level.FINER, + "CpsFlowExecution.reportSuspectedMethodTooLarge: full original Throwable message:\n" + xStr); if (ecCount > 1) { // Not squashing with explicit MethodTooLargeException @@ -921,7 +935,8 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // build console, but let the full context be found in // server log with some dedication. Note it is seen at // a different logging verbosity level. - METHOD_TOO_LARGE_LOGGER.log(Level.FINE, + METHOD_TOO_LARGE_LOGGER.log( + Level.FINE, "CpsFlowExecution.reportSuspectedMethodTooLarge: detected details of MethodTooLargeException:\n" + mtlEx.getMessage()); @@ -931,7 +946,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { .append(METHOD_TOO_LARGE_LOGGER.getName()) .append(" is required)"); - //return new RuntimeException(actionableMsg.toString(), mtlEx); + // return new RuntimeException(actionableMsg.toString(), mtlEx); RuntimeException rtex = new RuntimeException(actionableMsg.toString(), null); // Avoid having a huge stack trace leading to this pretty log-printer @@ -946,7 +961,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { // Considered passing original context, // but it is shown by ultimate job log :( - //rtex.addSuppressed(mtlEx); + // rtex.addSuppressed(mtlEx); return rtex; } @@ -974,10 +989,8 @@ private CpsScript parseScript() throws IOException { // wrapped into further exception, for actionable // logging in the job. Throwable t = CpsFlowExecution.reportSuspectedMethodTooLarge(x); - if (t instanceof RuntimeException) - throw (RuntimeException)t; - if (t instanceof Error) - throw (Error)t; + if (t instanceof RuntimeException) throw (RuntimeException) t; + if (t instanceof Error) throw (Error) t; // NOTE: In practice we should not get here, due // to practical type of "x" and what of it is diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java index 07cf84df6..3b56f4187 100644 --- a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java +++ b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScriptTest.java @@ -105,7 +105,8 @@ public void blockRun() throws Exception { "Scripts not permitted to use method groovy.lang.Script run java.io.File java.lang.String[]", b); } - @Test public void methodTooLargeExceptionFabricated() throws Exception { + @Test + public void methodTooLargeExceptionFabricated() throws Exception { // Fabricate a MethodTooLargeException which "normally" happens when evaluated // groovy script becomes a Java class too large for Java to handle internally. // In Jenkins practice this can happen not only due to large singular pipelines @@ -115,16 +116,19 @@ public void blockRun() throws Exception { WorkflowJob p = r.createProject(WorkflowJob.class); // sandbox == false to allow creation of the exception here: p.setDefinition(new CpsFlowDefinition( - "import groovyjarjarasm.asm.MethodTooLargeException;\n\n" + - "throw new MethodTooLargeException('className', 'methodName', 'methodDescriptor', 65535);" - , false)); + "import groovyjarjarasm.asm.MethodTooLargeException;\n\n" + + "throw new MethodTooLargeException('className', 'methodName', 'methodDescriptor', 65535);", + false)); WorkflowRun b = r.buildAndAssertStatus(Result.FAILURE, p); - r.assertLogContains("groovyjarjarasm.asm.MethodTooLargeException: Method too large: className.methodName methodDescriptor", b); + r.assertLogContains( + "groovyjarjarasm.asm.MethodTooLargeException: Method too large: className.methodName methodDescriptor", + b); r.assertLogContains("at WorkflowScript.run(WorkflowScript:3)", b); r.assertLogContains("at ___cps.transform___(Native Method)", b); } - @Test public void methodTooLargeExceptionRealistic() throws Exception { + @Test + public void methodTooLargeExceptionRealistic() throws Exception { // See comments above. Here we try to really induce a "method too large" // condition by abusing the nesting of exception-handling, too many stages // or methods, and whatever else we can throw at it. @@ -158,26 +162,27 @@ public void blockRun() throws Exception { } sbMethods.append("}\n"); - p.setDefinition(new CpsFlowDefinition(sbMethods.toString() + - "pipeline {\n" + - " agent none;\n" + - " stages {\n" + - " stage ('Test stage') {\n" + - " steps {\n" + - " script {\n" + - " echo 'BEGINNING TEST IN PIPELINE';\n" + - " method();\n" + - " echo 'ENDED TEST IN PIPELINE';\n" + - " }\n" + - " }\n" + - " }\n" + - sbStages.toString() + - " }\n" + - "}\n" + - "echo 'BEGINNING TEST OUT OF PIPELINE';\n" + - "method();\n" + - "echo 'ENDED TEST OUT OF PIPELINE';\n" - , true)); + p.setDefinition(new CpsFlowDefinition( + sbMethods.toString() + + "pipeline {\n" + + " agent none;\n" + + " stages {\n" + + " stage ('Test stage') {\n" + + " steps {\n" + + " script {\n" + + " echo 'BEGINNING TEST IN PIPELINE';\n" + + " method();\n" + + " echo 'ENDED TEST IN PIPELINE';\n" + + " }\n" + + " }\n" + + " }\n" + + sbStages.toString() + + " }\n" + + "}\n" + + "echo 'BEGINNING TEST OUT OF PIPELINE';\n" + + "method();\n" + + "echo 'ENDED TEST OUT OF PIPELINE';\n", + true)); WorkflowRun b = p.scheduleBuild2(0).get(); @@ -191,33 +196,33 @@ public void blockRun() throws Exception { // "Prettier" explanation added by CpsFlowExecution.parseScript(): r.assertLogContains("FAILED to parse WorkflowScript (the pipeline script) due to MethodTooLargeException", b); -/* - // Report as of release 3880.vb_ef4b_5cfd270 (Feb 2024) - // and same pattern seen since at least Jun 2022 (note - // that numbers after ___cps___ differ from job to job): - -org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed: -General error during class generation: Method too large: WorkflowScript.___cps___1 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; - -groovyjarjarasm.asm.MethodTooLargeException: Method too large: WorkflowScript.___cps___1 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; - at groovyjarjarasm.asm.MethodWriter.computeMethodInfoSize(MethodWriter.java:2087) - at groovyjarjarasm.asm.ClassWriter.toByteArray(ClassWriter.java:447) - at org.codehaus.groovy.control.CompilationUnit$17.call(CompilationUnit.java:850) - at org.codehaus.groovy.control.CompilationUnit.applyToPrimaryClassNodes(CompilationUnit.java:1087) - at org.codehaus.groovy.control.CompilationUnit.doPhaseOperation(CompilationUnit.java:624) - at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:602) - at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:579) - at groovy.lang.GroovyClassLoader.doParseClass(GroovyClassLoader.java:323) - at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:293) - at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox$Scope.parse(GroovySandbox.java:163) - at org.jenkinsci.plugins.workflow.cps.CpsGroovyShell.doParse(CpsGroovyShell.java:190) - at org.jenkinsci.plugins.workflow.cps.CpsGroovyShell.reparse(CpsGroovyShell.java:175) - at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.parseScript(CpsFlowExecution.java:637) - at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.start(CpsFlowExecution.java:583) - at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:335) - at hudson.model.ResourceController.execute(ResourceController.java:101) - at hudson.model.Executor.run(Executor.java:442) -*/ + /* + // Report as of release 3880.vb_ef4b_5cfd270 (Feb 2024) + // and same pattern seen since at least Jun 2022 (note + // that numbers after ___cps___ differ from job to job): + + org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed: + General error during class generation: Method too large: WorkflowScript.___cps___1 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + + groovyjarjarasm.asm.MethodTooLargeException: Method too large: WorkflowScript.___cps___1 ()Lcom/cloudbees/groovy/cps/impl/CpsFunction; + at groovyjarjarasm.asm.MethodWriter.computeMethodInfoSize(MethodWriter.java:2087) + at groovyjarjarasm.asm.ClassWriter.toByteArray(ClassWriter.java:447) + at org.codehaus.groovy.control.CompilationUnit$17.call(CompilationUnit.java:850) + at org.codehaus.groovy.control.CompilationUnit.applyToPrimaryClassNodes(CompilationUnit.java:1087) + at org.codehaus.groovy.control.CompilationUnit.doPhaseOperation(CompilationUnit.java:624) + at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:602) + at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:579) + at groovy.lang.GroovyClassLoader.doParseClass(GroovyClassLoader.java:323) + at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:293) + at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox$Scope.parse(GroovySandbox.java:163) + at org.jenkinsci.plugins.workflow.cps.CpsGroovyShell.doParse(CpsGroovyShell.java:190) + at org.jenkinsci.plugins.workflow.cps.CpsGroovyShell.reparse(CpsGroovyShell.java:175) + at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.parseScript(CpsFlowExecution.java:637) + at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.start(CpsFlowExecution.java:583) + at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:335) + at hudson.model.ResourceController.execute(ResourceController.java:101) + at hudson.model.Executor.run(Executor.java:442) + */ r.assertLogContains("Method too large: WorkflowScript.___cps___", b); r.assertLogContains("()Lcom/cloudbees/groovy/cps/impl/CpsFunction;", b); From 2766a10038a4f263ebd9858e3d00e3d195ea2a48 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 4 Oct 2025 17:52:08 +0200 Subject: [PATCH 40/42] CpsFlowExecution: rephrase Jenkins Shared Library/JSL (doc book term) to mention Pipeline Groovy library/PGL (plugin term) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @jglick> BTW the terminology in https://www.jenkins.io/doc/book/pipeline/shared-libraries/ is unfortunate. By definition a “library” is “shared”; why else would you create a library? The plugin itself uses the term Pipeline Groovy library. Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsFlowExecution.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index 2df4e1238..b6f214ef5 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -881,11 +881,11 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { } else if (overflowedClassName.contains("/")) { // quote the step/class name pretty: // FAILED to parse 'stepName' due to... - overflowedClassNameReport = "presumed JSL class '" + overflowedClassName + "'"; + overflowedClassNameReport = "presumed PGL (JSL) class '" + overflowedClassName + "'"; } else { // quote the step/class name pretty: // FAILED to parse 'stepName' due to... - overflowedClassNameReport = "presumed JSL step '" + overflowedClassName + "'"; + overflowedClassNameReport = "presumed PGL (JSL) step '" + overflowedClassName + "'"; } // Short message suffices, not much that a pipeline developer @@ -901,15 +901,16 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { actionableMsg.append("; please refactor to simplify code structure"); if (overflowedClassNameReport.contains("WorkflowScript") || CLASSNAME_SCRIPTNUM_PATTERN.matcher(overflowedClassName).find()) { - actionableMsg.append(" and/or move logic to a Jenkins Shared Library"); + actionableMsg.append(" and/or move logic to a Pipeline Groovy library" + + "(aka Jenkins Shared Library in some documentation)"); } if (xMsgStart.length() > 0) { actionableMsg.append(":\n-----\n").append(xMsgStart.toString()); } if (overflowedClassNameBreadcrumbs.length() > 0) { actionableMsg - .append( - "\nGroovy code trail (mentions of pipeline WorkflowScript and/or your JSL in larger stack trace):\n") + .append("\nGroovy code trail (mentions of pipeline WorkflowScript " + + "and/or your PGL (JSL) in larger stack trace):\n") .append(overflowedClassNameBreadcrumbs); } if (xMsgStart.length() > 0) { From ef62003363362e3d5c90199f2e714c98f5795961 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 4 Oct 2025 18:00:23 +0200 Subject: [PATCH 41/42] CpsFlowExecution:: simplify end-of-line regex Apply suggestion from @jglick Co-authored-by: Jesse Glick --- .../org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java index b6f214ef5..4e2704c51 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java @@ -680,7 +680,7 @@ protected static Throwable reportSuspectedMethodTooLarge(Throwable x) { Throwable mtlEx = null; int ecCount = 0; String xStr = Functions.printThrowable(x); // includes x.getMessage() contents - final Pattern LINE_SEP_PATTERN = Pattern.compile("\\R"); + final Pattern LINE_SEP_PATTERN = Pattern.compile("\r?\n"); String[] xLines = LINE_SEP_PATTERN.split(xStr); if (x.getClass().getSimpleName().equals("MethodTooLargeException")) { From a6a03efbbc0ac39f37962e624e98d6cfe7e51e24 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 11 Aug 2026 19:13:25 +0200 Subject: [PATCH 42/42] CpsScript: invokeMethod(): also handle reportSuspectedMethodTooLarge() [#817] Signed-off-by: Jim Klimov --- .../plugins/workflow/cps/CpsScript.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScript.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScript.java index cd98553d7..d17db42b3 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScript.java +++ b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScript.java @@ -112,9 +112,19 @@ public final Object invokeMethod(String name, Object args) { GlobalVariable v = GlobalVariable.byName(name, $buildNoException()); if (v != null) { try { - Object o = v.getValue(this); - return InvokerHelper.getMetaClass(o).invokeMethod(o, "call", args); - } catch (Exception x) { + try { + Object o = v.getValue(this); + return InvokerHelper.getMetaClass(o).invokeMethod(o, "call", args); + } catch (RuntimeException | Error x) { + // This method ends up throwing something (original + // or changed exception, depending on situation). + // Here we anticipate a MethodTooLargeException + // (or traces of its message stack), possibly + // wrapped into further exception, for actionable + // logging in the job. + throw CpsFlowExecution.reportSuspectedMethodTooLarge(x); + } + } catch (Throwable x) { throw new InvokerInvocationException(x); } }