Merge branch 'restructure_http_error_handling' into 'master'

restructure HTTP error handling

See merge request sheepitrenderfarm/client!105

src/main/java/com/sheepit/client/Error.java

@@ -61,6 +61,7 @@ public class Error {
 		PATH_INVALID(102),
 		NO_WRITE_PERMISSION(103),
 		SERVER_DOWN(104),
+		ERROR_BAD_SERVER_RESPONSE(105),
 		;
 		
 		private final int id;
@@ -161,6 +162,8 @@ public class Error {
 		switch (in) {
 			case ERROR_BAD_UPLOAD_RESPONSE:
 				return "Corrupt response from the server when trying to upload data. The server might be overloaded or be encountering other issues. Will try again in a few minutes.";
+			case ERROR_BAD_SERVER_RESPONSE:
+				return "Unexpected response from server received. The server might be overloaded or encountering other issues.";
 			case NETWORK_ISSUE:
 				return "Could not connect to the server, please check your connection to the internet.";
 			case SERVER_DOWN:

src/main/java/com/sheepit/client/Server.java

@@ -223,7 +223,14 @@ public class Server extends Thread {
 			int r = response.code();
 			String contentType = response.body().contentType().toString();
 
-			if (r == HttpURLConnection.HTTP_OK && contentType.startsWith("text/xml")) {
+			if (r != HttpURLConnection.HTTP_OK || contentType.startsWith("text/xml") == false) {
+				return Error.Type.ERROR_BAD_SERVER_RESPONSE;
+			}
+			else if (r == HttpURLConnection.HTTP_NOT_FOUND) {
+				// most likely the server instance is down but not the frontend proxy (traefik)
+				return Error.Type.SERVER_DOWN;
+			}
+
 			String in = response.body().string();
 			serverConfig = new Persister().read(ServerConfig.class, in);
 
@@ -239,11 +246,6 @@ public class Server extends Thread {
 				this.user_config.setPassword(publickey);
 			}
 		}
-			else if (r == HttpURLConnection.HTTP_NOT_FOUND) {
-				// most likely the server instance is down but not the frontend proxy (traefik)
-				return Error.Type.SERVER_DOWN;
-			}
-		}
 		catch (ConnectException e) {
 			this.log.error("Server::getConfiguration error ConnectException " + e);
 			return Error.Type.NETWORK_ISSUE;
@@ -287,7 +289,8 @@ public class Server extends Thread {
 					HttpUrl.Builder urlBuilder = Objects.requireNonNull(HttpUrl.parse(this.getPage("speedtest-answer"))).newBuilder();
 					Response response = this.HTTPRequest(urlBuilder, RequestBody.create(MediaType.parse("application/xml"), writer.toString()));
 					if (response.code() != HttpURLConnection.HTTP_OK) {
-						throw new IOException("Server::getConfiguration Speedtest unexpected response");
+						this.log.error("Server::getConfiguration Speedtest unexpected response");
+						return Error.Type.ERROR_BAD_SERVER_RESPONSE;
 					}
 				}
 				catch (final Exception e) {
@@ -339,9 +342,19 @@ public class Server extends Thread {
 			Response response = this.HTTPRequest(urlBuilder, RequestBody.create(this.generateXMLForMD5cache(), MediaType.parse("application/xml")));
 			
 			int r = response.code();
-			String contentType = response.body().contentType().toString();
+			if (r == HttpURLConnection.HTTP_UNAVAILABLE || r == HttpURLConnection.HTTP_CLIENT_TIMEOUT) {
+				this.log.error("Server::requestJob server unavailable or down: " + response);
+				throw new FermeServerDown();
+			}
+			else if(response.body().contentType().toString().startsWith("text/xml") == false) {
+				this.log.error("Server::requestJob bad contentType received: " + response);
+				throw new FermeExceptionBadResponseFromServer();
+			}
+			else if (r != HttpURLConnection.HTTP_OK) {
+				this.log.error("Server::requestJob unexpected response" + response);
+				throw new FermeException();
+			}
 
-			if (r == HttpURLConnection.HTTP_OK && contentType.startsWith("text/xml")) {
 			String in = response.body().string();
 
 			JobInfos jobData = new Persister().read(JobInfos.class, in);
@@ -388,18 +401,6 @@ public class Server extends Thread {
 					jobData.getRenderTask().getName(), jobData.getRenderTask().getPassword(),
 					jobData.getRenderTask().getSynchronous_upload().equals("1"), jobData.getRenderTask().getRendererInfos().getUpdate_method());
 		}
-			else {
-				System.out.println("Server::requestJob url " + url_contents + " r " + r + " contentType " + contentType);
-				if (r == HttpURLConnection.HTTP_UNAVAILABLE || r == HttpURLConnection.HTTP_CLIENT_TIMEOUT) {
-					// most likely varnish is up but apache down
-					throw new FermeServerDown();
-				}
-				else if (r == HttpURLConnection.HTTP_OK && contentType.startsWith("text/html")) {
-					throw new FermeExceptionBadResponseFromServer();
-				}
-				System.out.println(response.body().string());
-			}
-		}
 		catch (FermeException e) {
 			throw e;
 		}
@@ -415,7 +416,6 @@ public class Server extends Thread {
 			e.printStackTrace(pw);
 			throw new FermeException("error requestJob: unknown exception " + e + " stacktrace: " + sw.toString());
 		}
-		throw new FermeException("error requestJob, end of function");
 	}
 	
 	public Response HTTPRequest(String url) throws IOException {
@@ -444,7 +444,7 @@ public class Server extends Thread {
 			response = httpClient.newCall(request).execute();
 			
 			if (!response.isSuccessful()) {
-				throw new IOException("Unexpected code " + response);
+				this.log.error("Received unsuccessful HTTP response " + response);
 			}
 			
 			this.lastRequestTime = new Date().getTime();