fix: handle catchError+sh(returnStatus:true)+error() via FlowGraph traversal

[lidiams96]

Summary

Fixes the regression introduced by #106 for the catchError + sh(returnStatus:true) + error() pipeline pattern, as discussed in #105.

The problem

In this pattern:

catchError(buildResult: 'FAILURE') {
    def exitCode = sh(returnStatus: true, script: '...')
    if (exitCode != 0) { error('Command failed') }
}

The FlowGraph structure is:

• sh step → has LogAction (the actual failure output), no ErrorAction (returnStatus:true suppresses it)
• error() step → has ErrorAction, no LogAction (only carries the message string)

Strategy 1 finds error() via ErrorAction but skips it because logAction == null. The sh step is invisible since it has no ErrorAction or WarningAction. The result falls back to run.getLog(maxLines).

The fix

When Strategy 1 finds a node with ErrorAction but no LogAction, instead of skipping it, findImmediateParentWithLog checks the immediate parent of the error() node. In Jenkins CPS, Groovy control-flow constructs (if, def, variable assignments) are transparent to the FlowGraph — only Jenkins steps create FlowNodes. This means the immediate parent of error() is the preceding sh step, which holds the actual failure output.

The method only proceeds when error() has exactly one parent, which rules out parallel-merge join points where the relevant log cannot be determined unambiguously.

private FlowNode findImmediateParentWithLog(FlowNode errorOrigin) {
    List<FlowNode> parents = errorOrigin.getParents();
    if (parents.size() == 1) {
        FlowNode parent = parents.get(0);
        if (parent.getAction(LogAction.class) != null) {
            return parent;
        }
    }
    return null;
}

This only activates when the normal path (origin has LogAction) does not work, as requested in #105.

Changes

• PipelineLogExtractor.java: new findImmediateParentWithLog method + one-line hook in getFailedStepLog
• PipelineLogExtractorTest.java: integration test covering buildResult: 'FAILURE' case

Test plan

• New integration test catchError_returnStatusPattern_immediateParentShLogExtracted passes
• Full test suite: 133 tests, 0 failures
• Validated against a production Jenkins pipeline with complex parallel failures and catchError+sh(returnStatus:true)+error() pattern

Relates to: #105

🤖 Generated with Claude Code

[panicking]

@lidiams96 you should wait the other merge request land. We should avoid multiple changes

[lidiams96]

@panicking Thanks for the review feedback on the BFS approach.

I've adopted your suggestion and simplified findImmediateParentWithLog to a single-parent check:

private FlowNode findImmediateParentWithLog(FlowNode errorOrigin) {
    List<FlowNode> parents = errorOrigin.getParents();
    if (parents.size() == 1) {
        FlowNode parent = parents.get(0);
        if (parent.getAction(LogAction.class) != null) {
            return parent;
        }
    }
    return null;
}

This is cleaner and avoids unnecessary BFS traversal. The size() == 1 guard correctly handles parallel merge join points. The integration test catchError_returnStatusPattern_immediateParentShLogExtracted passes with this simplified version, confirming that in CPS execution for sh(...) → if(exitCode!=0) { error() }, the immediate parent of error() is the sh step.

All 133 tests still pass.

[Copilot]

Pull request overview

Fixes a regression in Pipeline failure log extraction for the catchError + sh(returnStatus:true) + error() pattern by enhancing FlowGraph-based Strategy 1 so it can still locate the sh step log even when the failing error() node has no LogAction.

Changes:

• Add a Strategy 1 fallback that, when an ErrorAction origin has no LogAction, checks the origin’s immediate parent for a LogAction.
• Update Strategy 1 documentation to describe the additional fallback behavior.
• Add an integration test covering catchError(buildResult: 'FAILURE') with sh(returnStatus:true) followed by error().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/main/java/io/jenkins/plugins/explain_error/PipelineLogExtractor.java	Adds a FlowGraph fallback helper and hooks it into Strategy 1 to capture sh logs for the returnStatus:true + error() pattern.
src/test/java/io/jenkins/plugins/explain_error/PipelineLogExtractorTest.java	Adds an integration test ensuring the sh output is extracted for the regression scenario.

[panicking]

@lidiams96 are you addressing all the comments? I mean try to avoid to go up with node that are failing form waningclass and squash the commit because in the second you fix a problem in the first one.

[lidiams96]

@panicking Done — commits squashed into one.

Regarding the WarningAction concern: findImmediateParentWithLog is already guarded by the condition logAction == null && errorAction \!= null in getFailedStepLog. Nodes that come from the WarningAction path have errorAction == null, so the parent lookup is never triggered for them.

[panicking]

Ok let me test it

[lidiams96]

@panicking were you able to test it? Is there anything I can help with from my side?

[panicking]

@panicking were you able to test it? Is there anything I can help with from my side?

Tonight. I'm at embedded world

[shenxianpeng]

Tonight. I'm at embedded world

Hi @panicking, just checking if you had a chance to test this.