CpsFlowExecution: parseScript(): log "Method Too Large" situations more readably #817
Conversation
|
Out of curiosity, what output does this give you and how is it helpful? |
plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java
Outdated
Show resolved
Hide resolved
plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java
Outdated
Show resolved
Hide resolved
…to avoid catching by full name Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
…argeExceptionRealistic() tests Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
…sException as a carrier of MethodTooLargeException; re-throw with just a compact message to appear in the build log Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
|
Tests helped me realize that the PR updated with handling of both eventualities, and if the only exception in view is the MTL - re-throw with a short message avoiding a wall of text in pipeline log. From self-test console (showing both server log Still got a bit of stack trace from generic handling, but it is reasonably short - the ultimate pipeline devs won't have to scroll up a couple of screenfuls just to learn that their script was too long or complex. Now that the message is visible to end users, I am open to suggestions how to make it more actionable, and/or if the e.g. to explain about script length or its coding complexity and what to do about it (nesting, amount of stages, separation of methods into JSL classes, etc.); maybe point to a knowledge-base URL for a better maintainable article on the subject (don't want to replace one wall of text in the logs with another, right?) |
plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java
Outdated
Show resolved
Hide resolved
plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java
Outdated
Show resolved
Hide resolved
…unt to satisfy different CI platforms Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
…t importing the class Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
…uggestions for pipeline devs Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
… expected matching line Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
Until today, it was helpful to myself and a few colleagues who may/can/do follow traces of the Jenkins server logs when developing pipelines (testing in private Jenkins instances and those wrapped by IDEs notwithstanding). After the review comments some months ago (thanks, and sorry for not noticing) and a coding effort today, this should be better visible and more "actionable" to a more general population - now shown as a smaller wall of text right in the pipeline build logs: ...which is IMHO a bit more comprehensible than the original (especially the first time you see it): ...and requires less scrolling to get to the crux of the issue =D |
…ethods (class complexity) from maxTryCatch (nesting depth) Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
There was a problem hiding this comment.
The logic here is pretty convoluted and seems excessive when we do not really need to make things particularly pretty, we just need to offer some clue beyond what there is today. Would suffice to do something along the lines of
if (Functions.printThrowable(x).containsString("MethodTooLargeException")) {
throw new IllegalArgumentException("…your hint here…", x);
} else {
throw x;
}| } catch (RuntimeException | Error x) { | ||
| } catch (Exception x) { |
There was a problem hiding this comment.
This loses handling of Error, especially LinkageError.
| // Make a note in server log | ||
| LOGGER.log(Level.SEVERE, msg); | ||
|
|
There was a problem hiding this comment.
Please do not; this is up to the owner of the project to fix, not the server admin.
| // Make a note in server log | |
| LOGGER.log(Level.SEVERE, msg); |
A small almost-cosmetic change which makes life easier for my colleagues when they develop (and debug failures of) Jenkins scripted pipelines.