Skip to content

Commit

Permalink
Consume http response in case of HTTP error (#555)
Browse files Browse the repository at this point in the history 
* Consume http response in case of HTTP error.

* Consume response body if health report returns error.

* Replace DefaultHttpClient with preconfigured http client.
Http client now stop sending request body if server returns one of 4XX code.
Remove ExtenderClient constructor which accepts http client as argument.
Add traceId to log file.
Improve log file content in case of non-compilation error.
  • Loading branch information
@ekharkunov
ekharkunov authored Nov 20, 2024
1 parent dfd38e6 commit bd2c778
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 42 deletions.
80 changes: 44 additions & 36 deletions client/src/main/java/com/defold/extender/client/ExtenderClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,22 @@

import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.StatusLine;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.client.methods.HttpRequestBase;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.impl.client.AbstractHttpClient;
import org.apache.http.message.AbstractHttpMessage;
import org.apache.http.message.BasicHeader;
import org.apache.http.entity.ByteArrayEntity;
import org.apache.http.entity.mime.MultipartEntity;
import org.apache.http.entity.mime.MultipartEntityBuilder;
import org.apache.http.entity.mime.content.ByteArrayBody;
import org.apache.http.impl.client.DefaultHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.util.EntityUtils;
import org.apache.http.client.CookieStore;
import org.apache.http.client.HttpClient;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.impl.client.BasicCookieStore;
import org.json.simple.JSONArray;
import org.json.simple.JSONObject;
Expand All @@ -24,6 +27,7 @@
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.io.UnsupportedEncodingException;
import java.math.BigInteger;
import java.security.MessageDigest;
Expand All @@ -50,7 +54,7 @@ public class ExtenderClient {
private long buildSleepTimeout;
private long buildResultWaitTimeout;
private List<BasicHeader> headers;
private AbstractHttpClient httpClient;
private HttpClient httpClient;

public static final String appManifestFilename = "app.manifest";
public static final String extensionFilename = "ext.manifest";
Expand All @@ -63,25 +67,18 @@ public class ExtenderClient {
* @param cacheDir A directory where the cache files are located (it must exist beforehand)
*/
public ExtenderClient(String extenderBaseUrl, File cacheDir) throws IOException {
this(new DefaultHttpClient(), extenderBaseUrl, cacheDir);
}

/**
* Creates an extender client
*
* @param httpClient The http client instance to use when communicating with the extender server
* @param extenderBaseUrl The build server url (e.g. https://build.defold.com)
* @param cacheDir A directory where the cache files are located (it must exist beforehand)
*/
public ExtenderClient(AbstractHttpClient httpClient, String extenderBaseUrl, File cacheDir) throws IOException {
this.extenderBaseUrl = extenderBaseUrl;
this.cache = new ExtenderClientCache(cacheDir);
this.httpCookies = new BasicCookieStore();
this.buildSleepTimeout = Long.parseLong(System.getProperty("com.defold.extender.client.build-sleep-timeout", "5000"));
this.buildResultWaitTimeout = Long.parseLong(System.getProperty("com.defold.extender.client.build-wait-timeout", "1200000"));
this.headers = new ArrayList<BasicHeader>();
this.httpClient = httpClient;
this.httpClient.setCookieStore(httpCookies);
RequestConfig requestConfig = RequestConfig.custom()
.setExpectContinueEnabled(true).build();
HttpClientBuilder clientBuilder = HttpClientBuilder.create()
.setDefaultRequestConfig(requestConfig)
.setDefaultCookieStore(httpCookies);
this.httpClient = clientBuilder.build();
}

private void log(String s, Object... args) {
Expand Down Expand Up @@ -190,6 +187,7 @@ String queryCache(List<ExtenderResource> sourceResources) throws ExtenderClientE
if (response.getStatusLine().getStatusCode() == HttpStatus.SC_OK) {
return EntityUtils.toString(response.getEntity()).replace("\\/", "/");
} else {
EntityUtils.consumeQuietly(response.getEntity());
return null; // Caching is not supported
}
} catch (Exception e) {
Expand Down Expand Up @@ -223,7 +221,7 @@ private static Set<String> getCachedFiles(String json) throws ExtenderClientExce
}


private void build_async(String platform, String sdkVersion, MultipartEntity entity, File destination, File log) throws ExtenderClientException {
private void build_async(String platform, String sdkVersion, HttpEntity entity, File destination, File log) throws ExtenderClientException {
try {
String url = String.format("%s/build_async/%s/%s", extenderBaseUrl, platform, sdkVersion);
HttpPost request = new HttpPost(url);
Expand All @@ -236,7 +234,8 @@ private void build_async(String platform, String sdkVersion, MultipartEntity ent

HttpResponse response = httpClient.execute(request);

final int statusCode = response.getStatusLine().getStatusCode();
StatusLine statusLine = response.getStatusLine();
final int statusCode = statusLine.getStatusCode();
if (statusCode == HttpStatus.SC_OK) {
String jobId = EntityUtils.toString(response.getEntity());
String traceId = response.getFirstHeader(TRACE_ID_HEADER_NAME).getValue();
Expand All @@ -256,7 +255,7 @@ private void build_async(String platform, String sdkVersion, MultipartEntity ent
}

if (jobStatus == 0) {
throw new TimeoutException(String.format("Job %s did not complete in time (timeout: %d ms)", jobId, buildResultWaitTimeout));
throw new TimeoutException(String.format("Job %s (traceId %s) did not complete in time (timeout: %d ms)", jobId, traceId == null ? "null" : traceId, buildResultWaitTimeout));
}

log("Checking job result for job %s", jobId);
Expand All @@ -267,12 +266,19 @@ private void build_async(String platform, String sdkVersion, MultipartEntity ent
response.getEntity().writeTo(new FileOutputStream(destination));
} else {
log("Job %s did not complete successfully. Writing log to %s", jobId, log);
response.getEntity().writeTo(new FileOutputStream(log));
throw new ExtenderClientException("Failed to build source.");
OutputStream os = new FileOutputStream(log);
os.write(String.format("Job id: %s; traceId: %s", jobId, traceId == null ? "null" : traceId).getBytes());
response.getEntity().writeTo(os);
os.close();
throw new ExtenderClientException(String.format("Failed to build source: jobId - %s, traceId - %s", jobId, traceId == null ? "null" : traceId));
}
} else {
log("Async build request failed with status code %d", statusCode);
response.getEntity().writeTo(new FileOutputStream(log));
String result = String.format("Async build request failed with status code %d %s", statusCode, statusLine.getReasonPhrase());
log(result);
OutputStream os = new FileOutputStream(log);
os.write(result.getBytes());
response.getEntity().writeTo(os);
os.close();
throw new ExtenderClientException("Failed to build source.");
}
} catch (Exception e) {
Expand All @@ -281,7 +287,7 @@ private void build_async(String platform, String sdkVersion, MultipartEntity ent

}

private void build_sync(String platform, String sdkVersion, MultipartEntity entity, File destination, File log) throws ExtenderClientException {
private void build_sync(String platform, String sdkVersion, HttpEntity entity, File destination, File log) throws ExtenderClientException {
try {
String url = String.format("%s/build/%s/%s", extenderBaseUrl, platform, sdkVersion);
HttpPost request = new HttpPost(url);
Expand All @@ -295,8 +301,12 @@ private void build_sync(String platform, String sdkVersion, MultipartEntity enti
if (response.getStatusLine().getStatusCode() == HttpStatus.SC_OK) {
response.getEntity().writeTo(new FileOutputStream(destination));
} else {
response.getEntity().writeTo(new FileOutputStream(log));
throw new ExtenderClientException("Failed to build source.");
String traceId = response.getFirstHeader(TRACE_ID_HEADER_NAME).getValue();
OutputStream os = new FileOutputStream(log);
os.write(String.format("TraceId: %s", traceId == null ? "null" : traceId).getBytes());
response.getEntity().writeTo(os);
os.close();
throw new ExtenderClientException(String.format("Failed to build source (traceId %s).", traceId == null ? "null" : traceId));
}
} catch (Exception e) {
throw new ExtenderClientException("Failed to communicate with Extender service.", e);
Expand Down Expand Up @@ -338,7 +348,8 @@ public void build(String platform, String sdkVersion, List<ExtenderResource> sou
return;
}

MultipartEntity entity = new MultipartEntity();
MultipartEntityBuilder entityBuilder = MultipartEntityBuilder.create();
entityBuilder.setStrictMode();

// Now, let's ask the server what files it already has
String cacheInfoName = "ne-cache-info.json";
Expand All @@ -348,7 +359,7 @@ public void build(String platform, String sdkVersion, List<ExtenderResource> sou
cachedFiles = getCachedFiles(cacheInfoJson);

// add the updated info to the file
entity.addPart(cacheInfoName, new ByteArrayBody(cacheInfoJson.getBytes(), cacheInfoName)); // Same as specified in DataStoreService.java
entityBuilder.addPart(cacheInfoName, new ByteArrayBody(cacheInfoJson.getBytes(), cacheInfoName)); // Same as specified in DataStoreService.java
}

for (ExtenderResource s : sourceResources) {
Expand All @@ -363,24 +374,20 @@ public void build(String platform, String sdkVersion, List<ExtenderResource> sou
} catch (IOException e) {
throw new ExtenderClientException("Error while getting content for " + s.getPath() + ": " + e.getMessage());
}
entity.addPart(s.getPath(), bin);
entityBuilder.addPart(s.getPath(), bin);
}

if (async) {
build_async(platform, sdkVersion, entity, destination, log);
build_async(platform, sdkVersion, entityBuilder.build(), destination, log);
}
else {
build_sync(platform, sdkVersion, entity, destination, log);
build_sync(platform, sdkVersion, entityBuilder.build(), destination, log);
}

// Store the new build
cache.put(platform, cacheKey, destination);
}

private static final String getRelativePath(File base, File path) {
return base.toURI().relativize(path.toURI()).getPath();
}

// Left for future debugging
public String httpRequestToString(HttpRequestBase request) {
StringBuilder sb = new StringBuilder();
Expand All @@ -398,6 +405,7 @@ public boolean health() throws IOException {
addAuthorizationHeader(request);
addHeaders(request);
HttpResponse response = httpClient.execute(request);
EntityUtils.consumeQuietly(response.getEntity());
if (response.getStatusLine().getStatusCode() == HttpStatus.SC_OK) {
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion client/src/main/java/com/defold/extender/client/ExtenderClientCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public File getCachedBuildFile(String platform) {
/** Gets the cache storage file
*/
public File getCacheFile() {
return new File(cacheDir.getAbsolutePath() + File.separator + this.cacheFile);
return new File(cacheDir.getAbsolutePath() + File.separator + ExtenderClientCache.cacheFile);
}

/** Calculates a key to identify a build
Expand Down
2 changes: 0 additions & 2 deletions client/src/main/java/com/defold/extender/client/FileExtenderResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
public class FileExtenderResource implements ExtenderResource {
private File file = null;
private String filePath;
private String fileAbsPath;

public FileExtenderResource(String filePath) {
this(new File(filePath));
Expand All @@ -21,7 +20,6 @@ public FileExtenderResource(String filePath, String zipPath) {
FileExtenderResource(File file) {
this.file = file;
this.filePath = file.getPath();
this.fileAbsPath = file.getAbsolutePath();
}

@Override
Expand Down
66 changes: 63 additions & 3 deletions client/src/test/java/com/defold/extender/client/ExtenderClientTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
package com.defold.extender.client;

import org.apache.commons.io.FileUtils;

import org.apache.http.StatusLine;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.entity.EntityBuilder;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.impl.client.DefaultHttpClient;

import org.junit.BeforeClass;
import org.junit.Test;
import org.mockito.Mockito;

import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.math.BigInteger;
import java.nio.file.Files;
import java.security.MessageDigest;
Expand Down Expand Up @@ -336,7 +342,7 @@ public void testClientHasExtensions() {
}

@Test
public void testClientHeaders() throws IOException {
public void testClientHeaders() throws Exception {
class MockHttpClient extends DefaultHttpClient {
public HttpUriRequest request;

Expand All @@ -358,7 +364,12 @@ public CloseableHttpResponse execute(HttpUriRequest request) {
final String HDR_VALUE_2 = "my custom header2";
MockHttpClient httpClient = new MockHttpClient();
File cacheDir = new File("build");
ExtenderClient extenderClient = new ExtenderClient(httpClient, "http://localhost", cacheDir);
Class<?> mockExtenderClientClass = Class.forName("com.defold.extender.client.ExtenderClient");
ExtenderClient extenderClient = (ExtenderClient) mockExtenderClientClass.getDeclaredConstructor(String.class, File.class).newInstance("http://localhost", cacheDir);
Field field = mockExtenderClientClass.getDeclaredField("httpClient");
field.setAccessible(true);
field.set(extenderClient, httpClient);

extenderClient.setHeader(HDR_NAME_1, HDR_VALUE_1);
extenderClient.setHeader(HDR_NAME_2, HDR_VALUE_2);
extenderClient.health();
Expand All @@ -373,6 +384,55 @@ public CloseableHttpResponse execute(HttpUriRequest request) {
}
}

@Test(expected = ExtenderClientException.class)
public void testClientHandleHTTPError() throws ClientProtocolException, IOException, ExtenderClientException, ClassNotFoundException, InstantiationException, IllegalAccessException, IllegalArgumentException, InvocationTargetException, NoSuchMethodException, SecurityException, NoSuchFieldException {
DefaultHttpClient httpClient = Mockito.mock(DefaultHttpClient.class);
CloseableHttpResponse httpResponse = Mockito.mock(CloseableHttpResponse.class);
StatusLine statusLine = Mockito.mock(StatusLine.class);

when(statusLine.getStatusCode()).thenReturn(401);
when(httpResponse.getStatusLine()).thenReturn(statusLine);
when(httpResponse.getEntity()).thenReturn(
EntityBuilder.create()
.setStream(new ByteArrayInputStream("Unauthorized".getBytes()))
.build()
);
when(httpClient.execute(Mockito.any(HttpGet.class))).thenReturn(httpResponse);
when(httpClient.execute(Mockito.any(HttpPost.class))).thenReturn(httpResponse);

File cacheDir = new File("build");
File targetDir = new File("output");
File log = new File("build.log");

File a = new File("build/a");
File b = new File("build/b");
File c = new File("build/c");
FileExtenderResource aRes = new FileExtenderResource(a);
FileExtenderResource bRes = new FileExtenderResource(b);
FileExtenderResource cRes = new FileExtenderResource(c);

a.deleteOnExit();
b.deleteOnExit();
c.deleteOnExit();

writeToFile("build/a", "a");
writeToFile("build/b", "b");
writeToFile("build/c", "c");

List<ExtenderResource> inputFiles = new ArrayList<>();
inputFiles.add(aRes);
inputFiles.add(bRes);
inputFiles.add(cRes);

Class<?> mockExtenderClientClass = Class.forName("com.defold.extender.client.ExtenderClient");
ExtenderClient extenderClient = (ExtenderClient) mockExtenderClientClass.getDeclaredConstructor(String.class, File.class).newInstance("http://localhost", cacheDir);
Field field = mockExtenderClientClass.getDeclaredField("httpClient");
field.setAccessible(true);
field.set(extenderClient, httpClient);

extenderClient.build("js-web", "aaaaaaaa", inputFiles, targetDir, log, true);
}

/*
Scans a directory and returns true if there are extensions available
*/
Expand Down
1 change: 1 addition & 0 deletions server/src/main/java/com/defold/extender/services/HealthReporterService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public String collectHealthReport(boolean isRemoteBuildEnabled, Map<String, Remo
updateOperationalStatus(platformOperationalStatus, platform, false);
}
} else {
EntityUtils.consumeQuietly(response.getEntity());
updateOperationalStatus(platformOperationalStatus, platform, false);
}
} catch(Exception exc) {
Expand Down

0 comments on commit bd2c778

Please sign in to comment.