You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@guacamole.apache.org by jm...@apache.org on 2017/01/04 03:59:56 UTC
[3/5] incubator-guacamole-client git commit: GUACAMOLE-36: Generalize
and simplify handling of REST API errors.
GUACAMOLE-36: Generalize and simplify handling of REST API errors.
Project: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/commit/30179c40
Tree: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/tree/30179c40
Diff: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/diff/30179c40
Branch: refs/heads/master
Commit: 30179c405fe2dc856c608df8226e53d81702b6eb
Parents: f891f69
Author: Michael Jumper <mj...@apache.org>
Authored: Mon Aug 15 22:24:04 2016 -0700
Committer: Michael Jumper <mj...@apache.org>
Committed: Tue Jan 3 19:42:24 2017 -0800
----------------------------------------------------------------------
.../org/apache/guacamole/rest/APIError.java | 152 +++++++++----------
.../org/apache/guacamole/rest/APIException.java | 100 ++----------
.../guacamole/rest/RESTExceptionWrapper.java | 116 ++------------
.../APIConnectionRecordSortPredicate.java | 7 +-
4 files changed, 111 insertions(+), 264 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/30179c40/guacamole/src/main/java/org/apache/guacamole/rest/APIError.java
----------------------------------------------------------------------
diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/APIError.java b/guacamole/src/main/java/org/apache/guacamole/rest/APIError.java
index c6a2c63..7727b85 100644
--- a/guacamole/src/main/java/org/apache/guacamole/rest/APIError.java
+++ b/guacamole/src/main/java/org/apache/guacamole/rest/APIError.java
@@ -20,8 +20,15 @@
package org.apache.guacamole.rest;
import java.util.Collection;
-import javax.ws.rs.core.Response;
+import org.apache.guacamole.GuacamoleClientException;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleResourceNotFoundException;
+import org.apache.guacamole.GuacamoleSecurityException;
import org.apache.guacamole.form.Field;
+import org.apache.guacamole.net.auth.credentials.GuacamoleCredentialsException;
+import org.apache.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException;
+import org.apache.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException;
+import org.apache.guacamole.tunnel.GuacamoleStreamException;
/**
* Describes an error that occurred within a REST endpoint.
@@ -60,124 +67,115 @@ public class APIError {
* The requested operation could not be performed because the request
* itself was malformed.
*/
- BAD_REQUEST(Response.Status.BAD_REQUEST),
+ BAD_REQUEST,
/**
* The credentials provided were invalid.
*/
- INVALID_CREDENTIALS(Response.Status.FORBIDDEN),
+ INVALID_CREDENTIALS,
/**
* The credentials provided were not necessarily invalid, but were not
* sufficient to determine validity.
*/
- INSUFFICIENT_CREDENTIALS(Response.Status.FORBIDDEN),
+ INSUFFICIENT_CREDENTIALS,
/**
* An internal server error has occurred.
*/
- INTERNAL_ERROR(Response.Status.INTERNAL_SERVER_ERROR),
+ INTERNAL_ERROR,
/**
* An object related to the request does not exist.
*/
- NOT_FOUND(Response.Status.NOT_FOUND),
+ NOT_FOUND,
/**
* Permission was denied to perform the requested operation.
*/
- PERMISSION_DENIED(Response.Status.FORBIDDEN),
+ PERMISSION_DENIED,
/**
* An error occurred within an intercepted stream, terminating that
* stream. The Guacamole protocol status code of that error can be
* retrieved with getStatusCode().
*/
- STREAM_ERROR(Response.Status.BAD_REQUEST);
+ STREAM_ERROR;
/**
- * The HTTP status associated with this error type.
- */
- private final Response.Status status;
-
- /**
- * Defines a new error type associated with the given HTTP status.
+ * Returns the REST API error type which corresponds to the type of the
+ * given exception.
*
- * @param status
- * The HTTP status to associate with the error type.
- */
- Type(Response.Status status) {
- this.status = status;
- }
-
- /**
- * Returns the HTTP status associated with this error type.
+ * @param exception
+ * The exception to use to derive the API error type.
*
* @return
- * The HTTP status associated with this error type.
+ * The API error type which corresponds to the type of the given
+ * exception.
*/
- public Response.Status getStatus() {
- return status;
+ public static Type fromGuacamoleException(GuacamoleException exception) {
+
+ // Additional credentials are needed
+ if (exception instanceof GuacamoleInsufficientCredentialsException)
+ return INSUFFICIENT_CREDENTIALS;
+
+ // The provided credentials are wrong
+ if (exception instanceof GuacamoleInvalidCredentialsException)
+ return INVALID_CREDENTIALS;
+
+ // Generic permission denied
+ if (exception instanceof GuacamoleSecurityException)
+ return PERMISSION_DENIED;
+
+ // Arbitrary resource not found
+ if (exception instanceof GuacamoleResourceNotFoundException)
+ return NOT_FOUND;
+
+ // Arbitrary bad requests
+ if (exception instanceof GuacamoleClientException)
+ return BAD_REQUEST;
+
+ // Errors from intercepted streams
+ if (exception instanceof GuacamoleStreamException)
+ return STREAM_ERROR;
+
+ // All other errors
+ return INTERNAL_ERROR;
+
}
}
/**
- * Creates a new APIError of type STREAM_ERROR and having the given
- * Guacamole protocol status code and human-readable message. The status
- * code and message should be taken directly from the "ack" instruction
- * causing the error.
+ * Creates a new APIError which exposes the details of the given
+ * GuacamoleException.
*
- * @param statusCode
- * The Guacamole protocol status code describing the error that
- * occurred within the intercepted stream.
- *
- * @param message
- * An arbitrary human-readable message describing the error that
- * occurred.
+ * @param exception
+ * The GuacamoleException from which the details of the new APIError
+ * should be derived.
*/
- public APIError(int statusCode, String message) {
- this.type = Type.STREAM_ERROR;
- this.message = message;
- this.statusCode = statusCode;
- this.expected = null;
- }
+ public APIError(GuacamoleException exception) {
- /**
- * Create a new APIError with the specified error message.
- *
- * @param type
- * The type of error that occurred.
- *
- * @param message
- * The error message.
- */
- public APIError(Type type, String message) {
- this.type = type;
- this.message = message;
- this.statusCode = null;
- this.expected = null;
- }
+ // Build base REST service error
+ this.type = Type.fromGuacamoleException(exception);
+ this.message = exception.getMessage();
+
+ // Add expected credentials if applicable
+ if (exception instanceof GuacamoleCredentialsException) {
+ GuacamoleCredentialsException credentialsException = (GuacamoleCredentialsException) exception;
+ this.expected = credentialsException.getCredentialsInfo().getFields();
+ }
+ else
+ this.expected = null;
+
+ // Add stream status code if applicable
+ if (exception instanceof GuacamoleStreamException) {
+ GuacamoleStreamException streamException = (GuacamoleStreamException) exception;
+ this.statusCode = streamException.getStatus().getGuacamoleStatusCode();
+ }
+ else
+ this.statusCode = null;
- /**
- * Create a new APIError with the specified error message and parameter
- * information.
- *
- * @param type
- * The type of error that occurred.
- *
- * @param message
- * The error message.
- *
- * @param expected
- * All parameters expected in the original request, or now required as
- * a result of the original request, as a collection of fields.
- */
- public APIError(Type type, String message, Collection<Field> expected) {
- this.type = type;
- this.message = message;
- this.statusCode = null;
- this.expected = expected;
}
/**
http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/30179c40/guacamole/src/main/java/org/apache/guacamole/rest/APIException.java
----------------------------------------------------------------------
diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/APIException.java b/guacamole/src/main/java/org/apache/guacamole/rest/APIException.java
index ea4cf5d..0e1a4f5 100644
--- a/guacamole/src/main/java/org/apache/guacamole/rest/APIException.java
+++ b/guacamole/src/main/java/org/apache/guacamole/rest/APIException.java
@@ -19,17 +19,16 @@
package org.apache.guacamole.rest;
-import java.util.Collection;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
-import org.apache.guacamole.form.Field;
-import org.apache.guacamole.protocol.GuacamoleStatus;
+import org.apache.guacamole.GuacamoleException;
/**
- * An exception that will result in the given error error information being
- * returned from the API layer. All error messages have the same format which
- * is defined by APIError.
+ * An exception which exposes a given error within the API layer. When thrown
+ * within the context of the REST API, an appropriate HTTP status code will be
+ * set for the failing response, and the details of the error will be exposed in
+ * the body of the response as an APIError structure.
*
* @author James Muehlner
* @author Michael Jumper
@@ -37,88 +36,21 @@ import org.apache.guacamole.protocol.GuacamoleStatus;
public class APIException extends WebApplicationException {
/**
- * Construct a new APIException with the given error. All information
- * associated with this new exception will be extracted from the given
- * APIError.
- *
- * @param error
- * The error that occurred.
- */
- public APIException(APIError error) {
- super(Response.status(error.getType().getStatus())
- .type(MediaType.APPLICATION_JSON)
- .entity(error)
- .build());
- }
-
- /**
- * Creates a new APIException with the given type and message. The
- * corresponding APIError will be created from the provided information.
- *
- * @param type
- * The type of error that occurred.
- *
- * @param message
- * A human-readable message describing the error.
- */
- public APIException(APIError.Type type, String message) {
- this(new APIError(type, message));
- }
-
- /**
- * Creates a new APIException which represents an error that occurred within
- * an intercepted Guacamole stream. The nature of that error will be
- * described by a given status code, which should be the status code
- * provided by the "ack" instruction that reported the error.
+ * Construct a new APIException based on the given GuacamoleException and
+ * HTTP status. The details of the GuacamoleException relevant to the REST
+ * API will be exposed via an APIError.
*
* @param status
- * The Guacamole protocol status code describing the error that
- * occurred within the intercepted stream.
+ * The HTTP status which corresponds to the GuacamoleException.
*
- * @param message
- * An arbitrary human-readable message describing the error that
- * occurred.
+ * @param exception
+ * The GuacamoleException that occurred.
*/
- public APIException(int status, String message) {
- this(new APIError(status, message));
- }
-
- /**
- * Creates a new APIException which represents an error that occurred within
- * an intercepted Guacamole stream. The nature of that error will be
- * described by a given Guacamole protocol status, which should be the
- * status associated with the code provided by the "ack" instruction that
- * reported the error.
- *
- * @param status
- * The Guacamole protocol status describing the error that occurred
- * within the intercepted stream.
- *
- * @param message
- * An arbitrary human-readable message describing the error that
- * occurred.
- */
- public APIException(GuacamoleStatus status, String message) {
- this(status.getGuacamoleStatusCode(), message);
- }
-
- /**
- * Creates a new APIException with the given type, message, and parameter
- * information. The corresponding APIError will be created from the
- * provided information.
- *
- * @param type
- * The type of error that occurred.
- *
- * @param message
- * A human-readable message describing the error.
- *
- * @param expected
- * All parameters expected in the original request, or now required as
- * a result of the original request, as a collection of fields.
- */
- public APIException(APIError.Type type, String message, Collection<Field> expected) {
- this(new APIError(type, message, expected));
+ public APIException(Response.Status status, GuacamoleException exception) {
+ super(Response.status(status)
+ .type(MediaType.APPLICATION_JSON)
+ .entity(new APIError(exception))
+ .build());
}
}
http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/30179c40/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionWrapper.java
----------------------------------------------------------------------
diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionWrapper.java b/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionWrapper.java
index f4de406..8d6839f 100644
--- a/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionWrapper.java
+++ b/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionWrapper.java
@@ -24,6 +24,8 @@ import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import javax.ws.rs.FormParam;
import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
import org.apache.guacamole.GuacamoleClientException;
@@ -31,10 +33,7 @@ import org.apache.guacamole.GuacamoleException;
import org.apache.guacamole.GuacamoleResourceNotFoundException;
import org.apache.guacamole.GuacamoleSecurityException;
import org.apache.guacamole.GuacamoleUnauthorizedException;
-import org.apache.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException;
-import org.apache.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException;
import org.apache.guacamole.rest.auth.AuthenticationService;
-import org.apache.guacamole.tunnel.GuacamoleStreamException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -148,7 +147,7 @@ public class RESTExceptionWrapper implements MethodInterceptor {
}
@Override
- public Object invoke(MethodInvocation invocation) throws Throwable {
+ public Object invoke(MethodInvocation invocation) throws WebApplicationException {
try {
@@ -172,108 +171,27 @@ public class RESTExceptionWrapper implements MethodInterceptor {
}
- // Rethrow unchecked exceptions such that they are properly wrapped
- catch (RuntimeException e) {
- throw new GuacamoleException(e.getMessage(), e);
- }
-
}
- // Additional credentials are needed
- catch (GuacamoleInsufficientCredentialsException e) {
-
- // Generate default message
- String message = e.getMessage();
- if (message == null)
- message = "Permission denied.";
-
- throw new APIException(
- APIError.Type.INSUFFICIENT_CREDENTIALS,
- message,
- e.getCredentialsInfo().getFields()
- );
- }
-
- // The provided credentials are wrong
- catch (GuacamoleInvalidCredentialsException e) {
-
- // Generate default message
- String message = e.getMessage();
- if (message == null)
- message = "Permission denied.";
-
- throw new APIException(
- APIError.Type.INVALID_CREDENTIALS,
- message,
- e.getCredentialsInfo().getFields()
- );
- }
-
- // Generic permission denied
+ // Translate GuacamoleException subclasses to HTTP error codes
catch (GuacamoleSecurityException e) {
-
- // Generate default message
- String message = e.getMessage();
- if (message == null)
- message = "Permission denied.";
-
- throw new APIException(
- APIError.Type.PERMISSION_DENIED,
- message
- );
-
+ throw new APIException(Response.Status.FORBIDDEN, e);
}
-
- // Arbitrary resource not found
catch (GuacamoleResourceNotFoundException e) {
-
- // Generate default message
- String message = e.getMessage();
- if (message == null)
- message = "Not found.";
-
- throw new APIException(
- APIError.Type.NOT_FOUND,
- message
- );
-
+ throw new APIException(Response.Status.NOT_FOUND, e);
}
-
- // Arbitrary bad requests
catch (GuacamoleClientException e) {
-
- // Generate default message
- String message = e.getMessage();
- if (message == null)
- message = "Invalid request.";
-
- throw new APIException(
- APIError.Type.BAD_REQUEST,
- message
- );
-
+ throw new APIException(Response.Status.BAD_REQUEST, e);
}
-
- // Errors from intercepted streams
- catch (GuacamoleStreamException e) {
-
- // Generate default message
- String message = e.getMessage();
- if (message == null)
- message = "Error reported by stream.";
-
- throw new APIException(
- e.getStatus(),
- message
- );
-
+ catch (GuacamoleException e) {
+ throw new APIException(Response.Status.INTERNAL_SERVER_ERROR, e);
}
- // All other errors
- catch (GuacamoleException e) {
+ // Rethrow unchecked exceptions such that they are properly wrapped
+ catch (Throwable t) {
- // Log all reasonable details of exception
- String message = e.getMessage();
+ // Log all reasonable details of error
+ String message = t.getMessage();
if (message != null)
logger.error("Unexpected internal error: {}", message);
else
@@ -282,12 +200,10 @@ public class RESTExceptionWrapper implements MethodInterceptor {
+ "details.");
// Ensure internal errors are fully logged at the debug level
- logger.debug("Unexpected exception in REST endpoint.", e);
+ logger.debug("Unexpected error in REST endpoint.", t);
- throw new APIException(
- APIError.Type.INTERNAL_ERROR,
- "Unexpected server error."
- );
+ throw new APIException(Response.Status.INTERNAL_SERVER_ERROR,
+ new GuacamoleException("Unexpected internal error.", t));
}
http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/30179c40/guacamole/src/main/java/org/apache/guacamole/rest/history/APIConnectionRecordSortPredicate.java
----------------------------------------------------------------------
diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/history/APIConnectionRecordSortPredicate.java b/guacamole/src/main/java/org/apache/guacamole/rest/history/APIConnectionRecordSortPredicate.java
index b685087..a881f60 100644
--- a/guacamole/src/main/java/org/apache/guacamole/rest/history/APIConnectionRecordSortPredicate.java
+++ b/guacamole/src/main/java/org/apache/guacamole/rest/history/APIConnectionRecordSortPredicate.java
@@ -19,8 +19,9 @@
package org.apache.guacamole.rest.history;
+import javax.ws.rs.core.Response;
+import org.apache.guacamole.GuacamoleClientException;
import org.apache.guacamole.net.auth.ConnectionRecordSet;
-import org.apache.guacamole.rest.APIError;
import org.apache.guacamole.rest.APIException;
/**
@@ -111,8 +112,8 @@ public class APIConnectionRecordSortPredicate {
// Bail out if sort property is not valid
catch (IllegalArgumentException e) {
throw new APIException(
- APIError.Type.BAD_REQUEST,
- String.format("Invalid sort property: \"%s\"", value)
+ Response.Status.BAD_REQUEST,
+ new GuacamoleClientException(String.format("Invalid sort property: \"%s\"", value))
);
}