jenkinsci/github-checks-plugin

`java.util.UnknownFormatConversionException: Conversion = ';'` from GitHubChecksPublisher.publish

Closed this issue · 9 comments

jglick commented

Seen in https://ci.jenkins.io/job/Plugins/job/jjwt-api-plugin/job/PR-21/1/console building jenkinsci/jjwt-api-plugin#21:

 ​[GitHub Checks] GitHub check (name: Jenkins, status: COMPLETED) has been published
 ​java.util.UnknownFormatConversionException: Conversion = ';
 ​	at java.base/java.util.Formatter.checkText(Formatter.java:2732
 ​	at java.base/java.util.Formatter.parse(Formatter.java:2718
 ​	at java.base/java.util.Formatter.format(Formatter.java:2655
 ​	at java.base/java.util.Formatter.format(Formatter.java:2609
 ​	at java.base/java.lang.String.format(String.java:2897
 ​	at io.jenkins.plugins.util.PluginLogger.log(PluginLogger.java:47
 ​	at io.jenkins.plugins.checks.github.GitHubChecksPublisher.publish(GitHubChecksPublisher.java:92
 ​	at io.jenkins.plugins.checks.status.BuildStatusChecksPublisher.publish(BuildStatusChecksPublisher.java:63
 ​	at io.jenkins.plugins.checks.status.BuildStatusChecksPublisher.access$000(BuildStatusChecksPublisher.java:42
 ​	at io.jenkins.plugins.checks.status.BuildStatusChecksPublisher$JobCheckoutListener.lambda$onCheckout$0(BuildStatusChecksPublisher.java:174
 ​	at java.base/java.util.Optional.ifPresent(Optional.java:183
 ​	at io.jenkins.plugins.checks.status.BuildStatusChecksPublisher$JobCheckoutListener.onCheckout(BuildStatusChecksPublisher.java:174
 ​	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:150
 ​	at …
KalleOlaviNiemitalo commented

I see, java.util.logging.Logger has a log(Level level, String msg, Throwable thrown) method that takes a literal string, but PluginLogger has only log(String format, Object... args). GitHubChecksPublisher then calls both with the same string.

github-checks-plugin/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java

Lines 89 to 93 in 1c4caf9

catch (IOException e) {
String message = "Failed Publishing GitHub checks: ";
SYSTEM_LOGGER.log(Level.WARNING, (message + details).replaceAll("[\r\n]", ""), e);
buildLogger.log(message + e);
}

PluginLogger.log has a @FormatMethod annotation, though. Would that allow the bug to be detected in static analysis? FormatStringAnnotation: Invalid format string passed to formatting method

jglick commented

Why not just bypass PluginLogger in this case and print directly to the build log however you prefer?

KalleOlaviNiemitalo commented

AFAIK, this can be called from BuildStatusChecksPublisher.JobScheduledListener.onEnterWaiting before there is a Run and a build log.

timja commented

We don't have direct access to the task listener here -.-

Looks like more a bug with plugin-util-api-plugin @uhafner.

If there's no args being passed we shouldn't try and format it?

It's a shame we don't have the actual io exception either, i guess it's in the system log if anyone wants to dig it out

KalleOlaviNiemitalo commented

I expect buildLogger.log("%s", message + e) would suffice here.

  • 👍2
jglick commented

it's in the system log if anyone wants to dig it out

No idea. I do not have access. Whatever the original error was, it seems to have been transient, as a simple rebuild worked.

timja commented

I expect buildLogger.log("%s", message + e) would suffice here.

yes probably

uhafner commented

ErrorProne will find such bugs, yes. This is the reason for the @FormatMethod annotation. You are simply not allowed to use a non-constant string for the format (the same error will occur if you use String.format). Due to our complex build tool chain ErrorProne simply does not work in Jenkins plugin builds.

So I would consider this a bug in the caller of the log method.

  • 👍1