Repository: jclouds Updated Branches: refs/heads/master 59a88ff89 -> 72f2652dc
Properly handler ProfitBricks service errors Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/72f2652d Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/72f2652d Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/72f2652d Branch: refs/heads/master Commit: 72f2652dca861a743103a8f6d8a3fe0cfdf5b00d Parents: 59a88ff Author: Ignasi Barrera <[email protected]> Authored: Thu Feb 25 12:23:10 2016 +0100 Committer: Ignasi Barrera <[email protected]> Committed: Wed Mar 2 14:51:29 2016 +0100 ---------------------------------------------------------------------- .../ProfitBricksComputeServiceAdapter.java | 13 ++- .../profitbricks/domain/ServiceFault.java | 89 +++++++++++--------- .../handlers/ProfitBricksHttpErrorHandler.java | 14 ++- ...usFromPayloadHttpCommandExecutorService.java | 11 ++- .../parser/ServiceFaultResponseHandler.java | 27 ++++-- .../parser/ServiceFaultResponseHandlerTest.java | 32 +++++-- .../src/test/resources/fault-500.xml | 9 ++ 7 files changed, 136 insertions(+), 59 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jclouds/blob/72f2652d/providers/profitbricks/src/main/java/org/jclouds/profitbricks/compute/ProfitBricksComputeServiceAdapter.java ---------------------------------------------------------------------- diff --git a/providers/profitbricks/src/main/java/org/jclouds/profitbricks/compute/ProfitBricksComputeServiceAdapter.java b/providers/profitbricks/src/main/java/org/jclouds/profitbricks/compute/ProfitBricksComputeServiceAdapter.java index fe02c41..2b4df3d 100644 --- a/providers/profitbricks/src/main/java/org/jclouds/profitbricks/compute/ProfitBricksComputeServiceAdapter.java +++ b/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); - if (image != null) { - logger.trace(">> found image [%s].", image.name()); - return image; + try { + Image image = api.imageApi().getImage(id); + if (image != null) { + 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); http://git-wip-us.apache.org/repos/asf/jclouds/blob/72f2652d/providers/profitbricks/src/main/java/org/jclouds/profitbricks/domain/ServiceFault.java ---------------------------------------------------------------------- diff --git a/providers/profitbricks/src/main/java/org/jclouds/profitbricks/domain/ServiceFault.java b/providers/profitbricks/src/main/java/org/jclouds/profitbricks/domain/ServiceFault.java index bcf9774..d429981 100644 --- a/providers/profitbricks/src/main/java/org/jclouds/profitbricks/domain/ServiceFault.java +++ b/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 { - - 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 abstract String faultCode(); + public abstract String faultString(); + @Nullable public abstract Details details(); + public static Builder builder() { return new AutoValue_ServiceFault.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 Builder faultCode(String faultCode); + public abstract Builder faultString(String faultString); + public abstract Builder details(Details details); 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(); + } } } http://git-wip-us.apache.org/repos/asf/jclouds/blob/72f2652d/providers/profitbricks/src/main/java/org/jclouds/profitbricks/handlers/ProfitBricksHttpErrorHandler.java ---------------------------------------------------------------------- diff --git a/providers/profitbricks/src/main/java/org/jclouds/profitbricks/handlers/ProfitBricksHttpErrorHandler.java b/providers/profitbricks/src/main/java/org/jclouds/profitbricks/handlers/ProfitBricksHttpErrorHandler.java index a4ecb50..eaf26fa 100644 --- a/providers/profitbricks/src/main/java/org/jclouds/profitbricks/handlers/ProfitBricksHttpErrorHandler.java +++ b/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); http://git-wip-us.apache.org/repos/asf/jclouds/blob/72f2652d/providers/profitbricks/src/main/java/org/jclouds/profitbricks/http/ResponseStatusFromPayloadHttpCommandExecutorService.java ---------------------------------------------------------------------- diff --git a/providers/profitbricks/src/main/java/org/jclouds/profitbricks/http/ResponseStatusFromPayloadHttpCommandExecutorService.java b/providers/profitbricks/src/main/java/org/jclouds/profitbricks/http/ResponseStatusFromPayloadHttpCommandExecutorService.java index 00ca238..795d460 100644 --- a/providers/profitbricks/src/main/java/org/jclouds/profitbricks/http/ResponseStatusFromPayloadHttpCommandExecutorService.java +++ b/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) - responseBuilder - .statusCode(fault.httpCode()) - .message(fault.message()); + if (fault != null) { + if (fault.details() != null) { + responseBuilder.statusCode(fault.details().httpCode()).message(fault.details().message()); + } else { + responseBuilder.message(fault.faultString()); + } + } } } catch (Exception ex) { // ignore http://git-wip-us.apache.org/repos/asf/jclouds/blob/72f2652d/providers/profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/ServiceFaultResponseHandler.java ---------------------------------------------------------------------- diff --git a/providers/profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/ServiceFaultResponseHandler.java b/providers/profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/ServiceFaultResponseHandler.java index f21c32e..8b790ed 100644 --- a/providers/profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/ServiceFaultResponseHandler.java +++ b/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)) - builder.faultCode(ServiceFault.FaultCode.fromValue(textToStringValue())); + if ("faultcode".equals(qName)) + 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(); } http://git-wip-us.apache.org/repos/asf/jclouds/blob/72f2652d/providers/profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/ServiceFaultResponseHandlerTest.java ---------------------------------------------------------------------- diff --git a/providers/profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/ServiceFaultResponseHandlerTest.java b/providers/profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/ServiceFaultResponseHandlerTest.java index 2b54dd5..f8c36f6 100644 --- a/providers/profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/ServiceFaultResponseHandlerTest.java +++ b/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() - .faultCode(ServiceFault.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(); + ServiceFault expected = ServiceFault + .builder() + .faultCode("S:Server") + .faultString( + "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") + .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); } http://git-wip-us.apache.org/repos/asf/jclouds/blob/72f2652d/providers/profitbricks/src/test/resources/fault-500.xml ---------------------------------------------------------------------- diff --git a/providers/profitbricks/src/test/resources/fault-500.xml b/providers/profitbricks/src/test/resources/fault-500.xml new file mode 100644 index 0000000..e996474 --- /dev/null +++ b/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> \ No newline at end of file
