Properly handler ProfitBricks service errors

providers/profitbricks/src/main/java/org/jclouds/profitbricks/compute/ProfitBricksComputeServiceAdapter.java

@@ -302,11 +302,16 @@ public class ProfitBricksComputeServiceAdapter implements ComputeServiceAdapter<
    public Provisionable getImage(String id) {
       // try search images
       logger.trace("<< searching for image with id=%s", id);
-      Image image = api.imageApi().getImage(id);
+      try {
-      if (image != null) {
+         Image image = api.imageApi().getImage(id);
-         logger.trace(">> found image [%s].", image.name());
+         if (image != null) {
-         return image;
+            logger.trace(">> found image [%s].", image.name());
+            return image;
+         }
+      } catch (Exception ex) {
+         logger.warn(ex, ">> unexpected error getting image. Trying to get as a snapshot...");
       }
+      
       // try search snapshots
       logger.trace("<< not found from images. searching for snapshot with id=%s", id);
       Snapshot snapshot = api.snapshotApi().getSnapshot(id);

providers/profitbricks/src/main/java/org/jclouds/profitbricks/domain/ServiceFault.java

@@ -16,55 +16,68 @@
  */
 package org.jclouds.profitbricks.domain;
 
+import org.jclouds.javax.annotation.Nullable;
+
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Enums;
 
 @AutoValue
 public abstract class ServiceFault {
 
-   public enum FaultCode {
+   public abstract String faultCode();
-
+   public abstract String faultString();
-      BAD_REQUEST,
+   @Nullable public abstract Details details();
-      UNEXPECTED,
+   
-      UNAUTHORIZED,
-      RESOURCE_NOT_FOUND,
-      RESOURCE_DELETED,
-      PROVISIONING_IN_PROCESS,
-      PROVISIONING_NO_CHANGES,
-      OVER_LIMIT_SETTING,
-      SERVER_EXCEED_CAPACITY,
-      SERVICE_UNAVAILABLE,
-      UNRECOGNIZED;
-
-      public static FaultCode fromValue(String v) {
-         return Enums.getIfPresent(FaultCode.class, v).or(UNRECOGNIZED);
-      }
-   }
-
-   public abstract FaultCode faultCode();
-
-   public abstract int httpCode();
-
-   public abstract String message();
-
-   public abstract int requestId();
-
    public static Builder builder() {
       return new AutoValue_ServiceFault.Builder();
    }
-
+   
    @AutoValue.Builder
    public abstract static class Builder {
-
+      public abstract Builder faultCode(String faultCode);
-      public abstract Builder faultCode(FaultCode faultCode);
+      public abstract Builder faultString(String faultString);
-
+      public abstract Builder details(Details details);
-      public abstract Builder httpCode(int httpCode);
-
-      public abstract Builder message(String message);
-
-      public abstract Builder requestId(int requestId);
-
       public abstract ServiceFault build();
-
+   }
+   
+   @AutoValue
+   public abstract static class Details {
+   
+      public enum FaultCode {
+   
+         BAD_REQUEST,
+         UNEXPECTED,
+         UNAUTHORIZED,
+         RESOURCE_NOT_FOUND,
+         RESOURCE_DELETED,
+         PROVISIONING_IN_PROCESS,
+         PROVISIONING_NO_CHANGES,
+         OVER_LIMIT_SETTING,
+         SERVER_EXCEED_CAPACITY,
+         SERVICE_UNAVAILABLE,
+         UNRECOGNIZED;
+   
+         public static FaultCode fromValue(String v) {
+            return Enums.getIfPresent(FaultCode.class, v).or(UNRECOGNIZED);
+         }
+      }
+   
+      public abstract FaultCode faultCode();
+      public abstract int httpCode();
+      public abstract String message();
+      public abstract int requestId();
+   
+      public static Builder builder() {
+         return new AutoValue_ServiceFault_Details.Builder();
+      }
+   
+      @AutoValue.Builder
+      public abstract static class Builder {
+         public abstract Builder faultCode(FaultCode faultCode);
+         public abstract Builder httpCode(int httpCode);
+         public abstract Builder message(String message);
+         public abstract Builder requestId(int requestId);
+         public abstract Details build();
+      }
    }
 }

providers/profitbricks/src/main/java/org/jclouds/profitbricks/handlers/ProfitBricksHttpErrorHandler.java

@@ -16,6 +16,7 @@
  */
 package org.jclouds.profitbricks.handlers;
 
+import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
 import static org.jclouds.util.Closeables2.closeQuietly;
 
 import javax.inject.Singleton;
@@ -39,7 +40,15 @@ public class ProfitBricksHttpErrorHandler implements HttpErrorHandler {
 
    @Override
    public void handleError(final HttpCommand command, final HttpResponse response) {
-      Exception exception = null;
+   // it is important to always read fully and close streams
+      byte[] data = closeClientButKeepContentStream(response);
+      String message = data != null ? new String(data) : null;
+
+      Exception exception = message != null ? new HttpResponseException(command, response, message)
+            : new HttpResponseException(command, response);
+      message = message != null ? message : String.format("%s -> %s", command.getCurrentRequest().getRequestLine(),
+            response.getStatusLine());
+      
       try {
          switch (response.getStatusCode()) {
             case 400:
@@ -49,6 +58,9 @@ public class ProfitBricksHttpErrorHandler implements HttpErrorHandler {
             case 401:
                exception = new AuthorizationException("This request requires authentication.", exception);
                break;
+            case 403:
+               exception = new AuthorizationException(response.getMessage(), exception);
+               break;
             case 402:
             case 409:
                exception = new IllegalStateException(response.getMessage(), exception);

providers/profitbricks/src/main/java/org/jclouds/profitbricks/http/ResponseStatusFromPayloadHttpCommandExecutorService.java

@@ -94,10 +94,13 @@ public class ResponseStatusFromPayloadHttpCommandExecutorService extends JavaUrl
          try {
             if (isSoapPayload(in)) {
                ServiceFault fault = faultHandler.parse(in);
-               if (fault != null)
+               if (fault != null) {
-                  responseBuilder
+                  if (fault.details() != null) {
-                          .statusCode(fault.httpCode())
+                     responseBuilder.statusCode(fault.details().httpCode()).message(fault.details().message());
-                          .message(fault.message());
+                  } else {
+                     responseBuilder.message(fault.faultString());
+                  }
+               }
             }
          } catch (Exception ex) {
             // ignore

providers/profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/ServiceFaultResponseHandler.java

@@ -17,27 +17,40 @@
 package org.jclouds.profitbricks.http.parser;
 
 import org.jclouds.profitbricks.domain.ServiceFault;
+import org.xml.sax.Attributes;
 import org.xml.sax.SAXException;
 
 public class ServiceFaultResponseHandler extends BaseProfitBricksResponseHandler<ServiceFault> {
 
    private final ServiceFault.Builder builder;
+   private ServiceFault.Details.Builder detailsBuilder;
    private boolean done = false;
 
    ServiceFaultResponseHandler() {
       this.builder = ServiceFault.builder();
    }
+   
+   @Override
+   public void startElement(String uri, String localName, String qName, Attributes attributes) throws SAXException {
+      if ("detail".equals(qName)) {
+         detailsBuilder = ServiceFault.Details.builder();
+      }
+   }
 
    @Override
    protected void setPropertyOnEndTag(String qName) {
-      if ("faultCode".equals(qName))
+      if ("faultcode".equals(qName))
-         builder.faultCode(ServiceFault.FaultCode.fromValue(textToStringValue()));
+         builder.faultCode(textToStringValue());
+      else if ("faultstring".equals(qName))
+         builder.faultString(textToStringValue());
+      else if ("faultCode".equals(qName))
+         detailsBuilder.faultCode(ServiceFault.Details.FaultCode.fromValue(textToStringValue()));
       else if ("httpCode".equals(qName))
-         builder.httpCode(textToIntValue());
+         detailsBuilder.httpCode(textToIntValue());
       else if ("message".equals(qName))
-         builder.message(textToStringValue());
+         detailsBuilder.message(textToStringValue());
       else if ("requestId".equals(qName))
-         builder.requestId(textToIntValue());
+         detailsBuilder.requestId(textToIntValue());
    }
 
    @Override
@@ -45,8 +58,10 @@ public class ServiceFaultResponseHandler extends BaseProfitBricksResponseHandler
       if (done)
          return;
       setPropertyOnEndTag(qName);
-      if ("detail".equals(qName))
+      if ("S:Fault".equals(qName))
          done = true;
+      if ("detail".equals(qName))
+         builder.details(detailsBuilder.build());
       clearTextBuffer();
    }
 

providers/profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/ServiceFaultResponseHandlerTest.java

@@ -21,6 +21,7 @@ import static org.testng.Assert.assertNotNull;
 
 import org.jclouds.http.functions.ParseSax;
 import org.jclouds.profitbricks.domain.ServiceFault;
+import org.jclouds.profitbricks.domain.ServiceFault.Details;
 import org.testng.annotations.Test;
 
 @Test(groups = "unit", testName = "ServiceFaultResponseHandlerTest")
@@ -37,12 +38,31 @@ public class ServiceFaultResponseHandlerTest extends BaseResponseHandlerTest<Ser
       ServiceFault actual = parser.parse(payloadFromResource("/fault-404.xml"));
       assertNotNull(actual, "Parsed content returned null");
 
-      ServiceFault expected = ServiceFault.builder()
+      ServiceFault expected = ServiceFault
-              .faultCode(ServiceFault.FaultCode.RESOURCE_NOT_FOUND)
+            .builder()
-              .httpCode(404)
+            .faultCode("S:Server")
-              .message("The requested resource could not be found. Please refer to Request Id : 16370720. [VDC-6-404] The requested resource does not exist or already deleted by the users. ResourceId random-non-existing-id")
+            .faultString(
-              .requestId(16370720)
+                  "The requested resource could not be found. Please refer to Request Id : 16370720. [VDC-6-404] The requested resource does not exist or already deleted by the users. ResourceId random-non-existing-id")
-              .build();
+            .details(
+                  Details
+                        .builder()
+                        .faultCode(ServiceFault.Details.FaultCode.RESOURCE_NOT_FOUND)
+                        .httpCode(404)
+                        .message(
+                              "The requested resource could not be found. Please refer to Request Id : 16370720. [VDC-6-404] The requested resource does not exist or already deleted by the users. ResourceId random-non-existing-id")
+                        .requestId(16370720).build()).build();
+
+      assertEquals(expected, actual);
+   }
+
+   @Test
+   public void testParseSoapServiceFaultWithoutDetails() {
+      ParseSax<ServiceFault> parser = createParser();
+      ServiceFault actual = parser.parse(payloadFromResource("/fault-500.xml"));
+      assertNotNull(actual, "Parsed content returned null");
+
+      ServiceFault expected = ServiceFault.builder().faultCode("S:Server").faultString("javax.ejb.EJBException")
+            .build();
 
       assertEquals(expected, actual);
    }

providers/profitbricks/src/test/resources/fault-500.xml

@@ -0,0 +1,9 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<S:Envelope xmlns:S="http://schemas.xmlsoap.org/soap/envelope/">
+    <S:Body>
+        <S:Fault xmlns:ns4="http://www.w3.org/2003/05/soap-envelope">
+            <faultcode>S:Server</faultcode>
+            <faultstring>javax.ejb.EJBException</faultstring>
+        </S:Fault>
+    </S:Body>
+</S:Envelope>