Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Primary error message is not shown in error header, instead says error in 'error' step #124

Open
justinmchase opened this issue Sep 4, 2021 · 5 comments · May be fixed by #125
Open

Primary error message is not shown in error header, instead says error in 'error' step #124

justinmchase opened this issue Sep 4, 2021 · 5 comments · May be fixed by #125

Comments

@justinmchase
Copy link

Feature Request

When I report an error like this in one of my steps:

stage ("Submodule Check") {
  steps {
    script {
      error "Submodules are OK for branches but cannot be merged into main"
    }
  }
}

This plugin does not report that error cleanly in the check. I expect it to show the message I reported more prominently but instead it shows information about the error step itself.

For example right in the Pull Request instead of showing that error message it simply says error in 'error' step:

Screen Shot 2021-09-04 at 8 10 00 AM

Other github status checkers will show the actual error message in this part of the UI instead.

Additionaly when going to the error details it has the same message prominently in a header as well as some redundant information about an Error in the error step.

(red for bad, green for good)
Screen Shot 2021-09-04 at 8 11 19 AM

I feel like in the case of reporting an error via the error() step / function, the information about that step should be omitted and the error message itself should be what is reported.

@justinmchase justinmchase changed the title Show primary error message in error header Primary error message is not shown in error header, instead says error in 'error' step Sep 4, 2021
@justinmchase
Copy link
Author

Also, the unstable step has the same issue:

Screen Shot 2021-09-04 at 10 25 49 AM

Ideally it would show the actual message when in the unstable state as well.

@KalleOlaviNiemitalo
Copy link

The information about failing steps is formatted in FlowExecutionAnalyzer.java, so this issue belongs to jenkinsci/checks-api-plugin.

@timja timja transferred this issue from jenkinsci/github-checks-plugin Sep 4, 2021
@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Sep 4, 2021

Thank you, @timja.

UnstableStep adds a WarningAction.

ErrorStep just throws an AbortException. I guess that is then translated to ErrorAction somewhere in workflow-cps-plugin.

@KalleOlaviNiemitalo
Copy link

Should the formatting just special-case flowNode.getDisplayFunctionName() equal to "error" and "unstable", or is there a better way to decide whether the name and arguments of the step are uninteresting?

@KalleOlaviNiemitalo
Copy link

isStageNode already hardcodes the "stage" name, so hardcoding "error" and "unstable" as well would be consistent with that.

I imagine changes:

  • In getPotentialTitle, if flowNode is an "error" step, then get whereBuildFailed from the first line of ErrorAction.getDisplayName() instead of formatting.
  • In processErrorOrWarningRow, if flowNode is an "error" or "unstable" step, and there is only one argument, and its value matches errorAction.getDisplayName() or warningAction.getMessage(), then omit the step name and arguments from the output. Thus, if the "unstable" step somehow throws an actual error, then the step name and arguments would still be included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants