GUACAMOLE-36: Generalize and simplify handling of REST API errors.

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.
+         * Returns the REST API error type which corresponds to the type of the
-         */
+         * given exception.
-        private final Response.Status status;
-
-        /**
-         * Defines a new error type associated with the given HTTP status.
          *
-         * @param status
+         * @param exception
-         *     The HTTP status to associate with the error type.
+         *     The exception to use to derive the API error type.
-         */
-        Type(Response.Status status) {
-            this.status = status;
-        }
-
-        /**
-         * Returns the HTTP status associated with this 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() {
+        public static Type fromGuacamoleException(GuacamoleException exception) {
-            return status;
+
+            // 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
+     * Creates a new APIError which exposes the details of the given
-     * Guacamole protocol status code and human-readable message. The status
+     * GuacamoleException.
-     * code and message should be taken directly from the "ack" instruction
-     * causing the error.
      *
-     * @param statusCode
+     * @param exception
-     *     The Guacamole protocol status code describing the error that
+     *     The GuacamoleException from which the details of the new APIError
-     *     occurred within the intercepted stream.
+     *     should be derived.
-     *
-     * @param message
-     *     An arbitrary human-readable message describing the error that
-     *     occurred.
      */
-    public APIError(int statusCode, String message) {
+    public APIError(GuacamoleException exception) {
-        this.type       = Type.STREAM_ERROR;
-        this.message    = message;
-        this.statusCode = statusCode;
-        this.expected   = null;
-    }
 
-    /**
+        // Build base REST service error
-     * Create a new APIError with the specified error message.
+        this.type = Type.fromGuacamoleException(exception);
-     *
+        this.message = exception.getMessage();
-     * @param type
+
-     *     The type of error that occurred.
+        // Add expected credentials if applicable
-     *
+        if (exception instanceof GuacamoleCredentialsException) {
-     * @param message
+            GuacamoleCredentialsException credentialsException = (GuacamoleCredentialsException) exception;
-     *     The error message.
+            this.expected = credentialsException.getCredentialsInfo().getFields();
-     */
+        }
-    public APIError(Type type, String message) {
+        else
-        this.type       = type;
+            this.expected = null;
-        this.message    = message;
+
-        this.statusCode = null;
+        // Add stream status code if applicable
-        this.expected   = null;
+        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;
     }
 
     /**

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.GuacamoleException;
-import org.apache.guacamole.protocol.GuacamoleStatus;
 
 /**
- * An exception that will result in the given error error information being
+ * An exception which exposes a given error within the API layer. When thrown
- * returned from the API layer. All error messages have the same format which
+ * within the context of the REST API, an appropriate HTTP status code will be
- * is defined by APIError.
+ * 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
+     * Construct a new APIException based on the given GuacamoleException and
-     * associated with this new exception will be extracted from the given
+     * HTTP status. The details of the GuacamoleException relevant to the REST
-     * APIError.
+     * API will be exposed via an APIError.
      *
-     * @param error
+     * @param status
-     *     The error that occurred.
+     *     The HTTP status which corresponds to the GuacamoleException.
+     *
+     * @param exception
+     *     The GuacamoleException that occurred.
      */
-    public APIException(APIError error) {
+    public APIException(Response.Status status, GuacamoleException exception) {
-        super(Response.status(error.getType().getStatus())
+        super(Response.status(status)
                 .type(MediaType.APPLICATION_JSON)
-                .entity(error)
+                .entity(new APIError(exception))
                 .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.
-     *
-     * @param status
-     *     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.
-     */
-    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));
-    }
-
 }

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
+        // Translate GuacamoleException subclasses to HTTP error codes
-        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
         catch (GuacamoleSecurityException e) {
-
+            throw new APIException(Response.Status.FORBIDDEN, e);
-            // Generate default message
-            String message = e.getMessage();
-            if (message == null)
-                message = "Permission denied.";
-
-            throw new APIException(
-                APIError.Type.PERMISSION_DENIED,
-                message
-            );
-
         }
-
-        // Arbitrary resource not found
         catch (GuacamoleResourceNotFoundException e) {
-
+            throw new APIException(Response.Status.NOT_FOUND, e);
-            // Generate default message
-            String message = e.getMessage();
-            if (message == null)
-                message = "Not found.";
-
-            throw new APIException(
-                APIError.Type.NOT_FOUND,
-                message
-            );
-
         }
-        
-        // Arbitrary bad requests
         catch (GuacamoleClientException e) {
-
+            throw new APIException(Response.Status.BAD_REQUEST, e);
-            // Generate default message
-            String message = e.getMessage();
-            if (message == null)
-                message = "Invalid request.";
-
-            throw new APIException(
-                APIError.Type.BAD_REQUEST,
-                message
-            );
-
         }
-
-        // 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
-            );
-
-        }
-
-        // All other errors
         catch (GuacamoleException e) {
+            throw new APIException(Response.Status.INTERNAL_SERVER_ERROR, e);
+        }
 
-            // Log all reasonable details of exception
+        // Rethrow unchecked exceptions such that they are properly wrapped
-            String message = e.getMessage();
+        catch (Throwable t) {
+
+            // 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(
+            throw new APIException(Response.Status.INTERNAL_SERVER_ERROR,
-                APIError.Type.INTERNAL_ERROR,
+                    new GuacamoleException("Unexpected internal error.", t));
-                "Unexpected server error."
-            );
 
         }
 

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,
+                Response.Status.BAD_REQUEST,
-                String.format("Invalid sort property: \"%s\"", value)
+                new GuacamoleClientException(String.format("Invalid sort property: \"%s\"", value))
             );
         }