FlowInterruptedException::getMessage returns exception details

[damianszczepanik]

Class FlowInterruptedException does not meet getMessage() contract defined in Throwable class.

Constructor does not pass message to parent class so later getMessage() returns null value. This means that every generic code that uses getMessage() in try catch block returns empty value for this class making debugging harder.

Testing done

Unit test cases have been provided to prove the implementation.

Submitter checklist

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests that demonstrate the feature works or the issue is fixed

[jglick]

making debugging harder

I think this would only affect the Java debugger, since https://github.com/jenkinsci/workflow-job-plugin/blob/62ac59c112dd34b65b8c3c37f99844b5c9caae75/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java#L657-L658 would normally be calling

workflow-step-api-plugin/src/main/java/org/jenkinsci/plugins/workflow/steps/FlowInterruptedException.java

Lines 112 to 141 in 664bdaf

	/**
	* If a build catches this exception, it should use this method to report it.
	*/
	public void handle(Run<?,?> run, TaskListener listener) {
	Set<CauseOfInterruption> boundCauses = new HashSet<>();
	for (InterruptedBuildAction a : run.getActions(InterruptedBuildAction.class)) {
	boundCauses.addAll(a.getCauses());
	}
	Set<CauseOfInterruption> diff = new LinkedHashSet<>(causes);
	diff.removeAll(boundCauses);
	if (!diff.isEmpty()) {
	run.addAction(new InterruptedBuildAction(diff));
	for (CauseOfInterruption cause : diff) {
	cause.print(listener);
	}
	}
	print(getCause(), run, listener);
	for (Throwable t : getSuppressed()) {
	print(t, run, listener);
	}
	}
	private static void print(@CheckForNull Throwable t, Run<?,?> run, @NonNull TaskListener listener) {
	if (t instanceof AbortException) {
	listener.getLogger().println(t.getMessage());
	} else if (t instanceof FlowInterruptedException) {
	((FlowInterruptedException) t).handle(run, listener);
	} else if (t != null) {
	Functions.printStackTrace(t, listener.getLogger());
	}
	}

for user display. Given that, computing the message in the constructor would normally be wasteful, so it would be better to override getMessage.

[damianszczepanik]

Not sure what do you mean by Java debugging. The reason why I prepare this PR is that having groovy pipelines defined in Jenkins shared library I found that exception is triggered but nothing goes to output, console. This is the only exception that is problematic, others report correctly.

I'm ok with moving this to getMesage unless this will also satisfy toString methid.

Additionally: should we also include. Result?

[damianszczepanik]

Also I don't think this computing is very complex so I would rather calculate it once once and do not count this every time gerMeasage is invoked.

[jglick]

having groovy pipelines defined in Jenkins shared library I found that exception is triggered but nothing goes to output, console

Not sure what scenario you are referring to.

[damianszczepanik]

Moved to getMessage() method

[damianszczepanik]

@jglick PR is fixed, can you comment or merge ?