Skip to content

Commit

Permalink
Added a fix for issue Jenkins JENKINS-22325. The issue is that when u…
Browse files Browse the repository at this point in the history 
…sing the 'build' URL to trigger a job without parameters we don't jsut a JSOn response back. This makes sendHTTPCall fail when it tries to create a JSON object out of the response. The fix is to check if the response is empty and if so return null from sendHTTPCall. This means any call to sendHTTPCall will have to be checked for 'null responses.
  • Loading branch information
tim.brown5 committed Mar 28, 2014
1 parent f683d4e commit a9dda51
Showing 1 changed file with 39 additions and 21 deletions.
60 changes: 39 additions & 21 deletions src/main/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/RemoteBuildConfiguration.java
View file
Expand Up @@ -68,6 +68,7 @@ public class RemoteBuildConfiguration extends Builder {

private static String paramerizedBuildUrl = "/buildWithParameters";
private static String normalBuildUrl = "/build";
//private static String normalBuildUrl = "/buildWithParameters";
private static String buildTokenRootUrl = "/buildByToken";

private final boolean overrideAuth;
Expand Down Expand Up @@ -459,7 +460,8 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListener lis
this.failBuild(new Exception("No remote host is defined for this job."), listener);
return true;
}

String remoteServerURL = remoteServer.getAddress().toString();

List<String> cleanedParams = null;

if (this.loadParamsFromFile) {
Expand Down Expand Up @@ -489,24 +491,29 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListener lis
preCheckUrlString += "/lastBuild";
preCheckUrlString += "/api/json/";
JSONObject preCheckResponse = sendHTTPCall(preCheckUrlString, "GET", build, listener);

// check the latest build on the remote server to see if it's running - if so wait until it has stopped.
// if building is true then the build is running
// if result is null the build hasn't finished - but might not have started running.
while (preCheckResponse.getBoolean("building") == true || preCheckResponse.getString("result") == null) {
listener.getLogger().println("Remote build is currently running - waiting for it to finish.");
preCheckResponse = sendHTTPCall(preCheckUrlString, "POST", build, listener);
listener.getLogger().println("Waiting for " + this.pollInterval + " seconds until next retry.");

// Sleep for 'pollInterval' seconds.
// Sleep takes miliseconds so need to convert this.pollInterval to milisecopnds (x 1000)
try {
Thread.sleep(this.pollInterval * 1000);
} catch (InterruptedException e) {
this.failBuild(e, listener);

if ( preCheckResponse != null ) {
// check the latest build on the remote server to see if it's running - if so wait until it has stopped.
// if building is true then the build is running
// if result is null the build hasn't finished - but might not have started running.
while (preCheckResponse.getBoolean("building") == true || preCheckResponse.getString("result") == null) {
listener.getLogger().println("Remote build is currently running - waiting for it to finish.");
preCheckResponse = sendHTTPCall(preCheckUrlString, "POST", build, listener);
listener.getLogger().println("Waiting for " + this.pollInterval + " seconds until next retry.");

// Sleep for 'pollInterval' seconds.
// Sleep takes miliseconds so need to convert this.pollInterval to milisecopnds (x 1000)
try {
Thread.sleep(this.pollInterval * 1000);
} catch (InterruptedException e) {
this.failBuild(e, listener);
}
}
listener.getLogger().println("Remote job remote job " + jobName + " is not currenlty building.");
} else {
this.failBuild(new Exception("Got a blank response from Remote Jenkins Server [" + remoteServerURL + "], cannot continue."), listener);
}
listener.getLogger().println("Remote job remote job " + jobName + " is not currenlty building.");

} else {
listener.getLogger().println("Not checking if the remote job " + jobName + " is building.");
}
Expand All @@ -516,6 +523,10 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListener lis

//listener.getLogger().println("Getting ID of next job to build. URL: " + queryUrlString);
JSONObject queryResponseObject = sendHTTPCall(queryUrlString, "GET", build, listener);
if (queryResponseObject == null ) {
this.failBuild(new Exception("Got a blank response from Remote Jenkins Server [" + remoteServerURL + "], cannot continue."), listener);

This comment has been minimized.

Sign in to view

Copy link
@Pyrolistical

Pyrolistical Apr 3, 2014

this is incomplete. if you enter the if block, queryResponseObject is immediately used and therefore NPEs

This comment has been minimized.

Sign in to view

Copy link
@morficus

morficus Apr 7, 2014

Member

Can you elaborate on this please.
What settings do you have that this is causing an NPE?

This comment has been minimized.

Sign in to view

Copy link
@Pyrolistical

Pyrolistical Apr 7, 2014

You can't reproduce the NPE under normal operation, but when you are trying to get the next job build number but get an empty body, you will NPE.

Just read the code, it is plainly wrong. Look at line 526. Let's assume queryResponseObject is null, you enter the if block, then on line 530 you get on NPE on queryResponseObject.

Either you need to handle queryResponseObject null properly, or prove that it is never null and remove the if block.

This comment has been minimized.

Sign in to view

Copy link
@morficus

morficus Apr 7, 2014

Member

But calling failBuild() throws the proper exception and terminates the build. So if queryResponseObject is null, execution will terminate and never get to line 530.

Or maybe my memory is failing me on what failBuild() is doing :-p

This comment has been minimized.

Sign in to view

Copy link
@Pyrolistical

Pyrolistical Apr 8, 2014

Not if shouldNotFailBuild is true. failBuild() is a method in the same class.
}

int nextBuildNumber = queryResponseObject.getInt("nextBuildNumber");
listener.getLogger().println("This job is build #[" + Integer.toString(nextBuildNumber) + "] on the remote server.");

Expand All @@ -526,9 +537,11 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListener lis
}

listener.getLogger().println("Triggering remote job now.");
JSONObject responseObject = sendHTTPCall(triggerUrlString, "POST", build, listener);
sendHTTPCall(triggerUrlString, "POST", build, listener);

String jobURL = responseObject.getString("url");

// String jobURL = responseObject.getString("url");
String jobURL = remoteServerURL + "/job/" + this.encodeValue(job) + "/";

// This is only for Debug
// This output whether there is another job running on the remote host that this job had conflicted with.
Expand Down Expand Up @@ -689,13 +702,18 @@ public JSONObject sendHTTPCall(String urlString, String requestType, AbstractBui

while ((line = rd.readLine()) != null) {
response.append(line);

}
rd.close();

// JSONSerializer serializer = new JSONSerializer();
// need to parse the data we get back into struct
responseObject = (JSONObject) JSONSerializer.toJSON(response.toString());
//listener.getLogger().println("Called URL: '" + urlString + "', got response: '" + response.toString() + "'");
if ( response.toString().isEmpty() ) {
listener.getLogger().println("Remote Jenkins server returned empty response to trigger.");
return null;
} else {
responseObject = (JSONObject) JSONSerializer.toJSON(response.toString());
}

} catch (IOException e) {
// something failed with the connection, so throw an exception to mark the build as failed.
Expand Down

0 comments on commit a9dda51

Please sign in to comment.