changeset 200:4d6991d2839d

Improve error handling in web-gateway services Reviewed-by: jkang Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-July/023989.html
author Christopher Koehler <chkoehle@redhat.com>
date Mon, 17 Jul 2017 15:12:53 -0400
parents 0fe5182f3b1b
children f65dffb9f0f0
files common/core/pom.xml common/core/src/main/java/com/redhat/thermostat/gateway/common/util/HttpResponseExceptionHandler.java common/core/src/main/java/com/redhat/thermostat/gateway/common/util/ServiceException.java common/core/src/test/java/com/redhat/thermostat/gateway/common/util/HttpResponseExceptionHandlerTest.java common/core/src/test/java/com/redhat/thermostat/gateway/common/util/ServiceExceptionTest.java services/jvm-gc/src/main/java/com/redhat/thermostat/service/jvm/gc/JvmGcHttpHandler.java
diffstat 6 files changed, 375 insertions(+), 4 deletions(-) [+]
line wrap: on
line diff
--- a/common/core/pom.xml	Mon Jul 17 11:22:45 2017 -0400
+++ b/common/core/pom.xml	Mon Jul 17 15:12:53 2017 -0400
@@ -87,6 +87,12 @@
             <version>${mockito-core.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.glassfish.jersey.core</groupId>
+            <artifactId>jersey-common</artifactId>
+            <version>${jersey.version}</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/common/core/src/main/java/com/redhat/thermostat/gateway/common/util/HttpResponseExceptionHandler.java	Mon Jul 17 15:12:53 2017 -0400
@@ -0,0 +1,87 @@
+/*
+ * Copyright 2012-2017 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.gateway.common.util;
+
+import javax.ws.rs.core.Response;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * Accepts exceptions after being told what response to make for the provided
+ * exceptions, and creates the appropriate response. It is designed such that
+ * even if you don't load it with exceptions, it will still handle all of the
+ * exceptions that are children (or the same class as) Exception.
+ */
+public class HttpResponseExceptionHandler {
+
+    private static final ServiceException DEFAULT_SERVICE_EXCEPTION = ServiceException.UNEXPECTED_ERROR;
+
+    private Map<Class<? extends Exception>, ServiceException> classToResponseMessage = new HashMap<>();
+
+    /**
+     * Adds an exception class to be tracked by this object. If the class was
+     * already defined, it will replace it.
+     * @param cls The class for which the response will be returned.
+     * @param serviceException The service exception type.
+     * @return Itself (for chaining).
+     * @throws NullPointerException If any argument is null.
+     */
+    public HttpResponseExceptionHandler add(Class<? extends Exception> cls, ServiceException serviceException) {
+        Objects.requireNonNull(cls);
+        Objects.requireNonNull(serviceException);
+        classToResponseMessage.put(cls, serviceException);
+        return this;
+    }
+
+    /**
+     * Generates the response from the exception. If the exception is not known
+     * to the class (because it was never specified by .add()) then it will
+     * return a generic INTERNAL_SERVER_ERROR status with no message attached.
+     * @param exception The exception to get the error message for.
+     * @return The response for the exception.
+     * @throws NullPointerException If the argument is null.
+     */
+    public Response generateResponseForException(Exception exception) {
+        Objects.requireNonNull(exception);
+        ServiceException serviceException = DEFAULT_SERVICE_EXCEPTION;
+        if (classToResponseMessage.containsKey(exception.getClass())) {
+            serviceException = classToResponseMessage.get(exception.getClass());
+        }
+        return serviceException.buildResponse();
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/common/core/src/main/java/com/redhat/thermostat/gateway/common/util/ServiceException.java	Mon Jul 17 15:12:53 2017 -0400
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2012-2017 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.gateway.common.util;
+
+import java.util.Objects;
+
+import javax.ws.rs.core.Response;
+
+public enum ServiceException {
+    DATABASE_UNAVAILABLE(503, "Unable to connect to database"),
+    EXPECTED_JSON_ARRAY(400, "Expected JSON array"),
+    MALFORMED_CLIENT_REQUEST(400, "Malformed client request"),
+    CANNOT_QUERY_REALMS_PROPERTY(400, "Cannot query realms property"),
+    UNEXPECTED_ERROR(500);
+
+    private static final String NO_REASON = "";
+    private int statusNumber;
+    private String errorMessage;
+
+    ServiceException(int statusNumber) {
+        this(statusNumber, NO_REASON);
+    }
+
+    ServiceException(int statusNumber, String errorMessage) {
+        this.statusNumber = statusNumber;
+        this.errorMessage = Objects.requireNonNull(errorMessage);
+    }
+
+    private boolean hasReason() {
+        return !this.errorMessage.equals(NO_REASON);
+    }
+
+    public Response buildResponse() {
+        Response.ResponseBuilder builder = Response.status(statusNumber);
+        if (hasReason()) {
+            builder.entity("{\"error\":\"" + errorMessage + "\"}");
+        }
+        return builder.build();
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/common/core/src/test/java/com/redhat/thermostat/gateway/common/util/HttpResponseExceptionHandlerTest.java	Mon Jul 17 15:12:53 2017 -0400
@@ -0,0 +1,120 @@
+/*
+ * Copyright 2012-2017 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.gateway.common.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import javax.ws.rs.core.Response;
+
+import org.junit.Test;
+
+public class HttpResponseExceptionHandlerTest {
+
+    private static void assertResponse(ServiceException serviceException, Response response) {
+        assertNotNull(response);
+        Response expectedResponse = serviceException.buildResponse();
+        assertEquals(expectedResponse.getStatus(), response.getStatus());
+        assertEquals(expectedResponse.getEntity(), response.getEntity());
+    }
+
+    @Test
+    public void testWithNoLoadedExceptions() {
+        HttpResponseExceptionHandler exceptionHandler = new HttpResponseExceptionHandler();
+        Response response = exceptionHandler.generateResponseForException(new Exception());
+        assertResponse(ServiceException.UNEXPECTED_ERROR, response);
+
+        response = exceptionHandler.generateResponseForException(new NullPointerException());
+        assertResponse(ServiceException.UNEXPECTED_ERROR, response);
+
+        response = exceptionHandler.generateResponseForException(new ClassCastException());
+        assertResponse(ServiceException.UNEXPECTED_ERROR, response);
+    }
+
+    @Test
+    public void testWithLoadedExceptions() {
+        HttpResponseExceptionHandler exceptionHandler = new HttpResponseExceptionHandler();
+        Response response = exceptionHandler.generateResponseForException(new ClassCastException());
+        assertResponse(ServiceException.UNEXPECTED_ERROR, response);
+
+        exceptionHandler.add(ClassCastException.class, ServiceException.EXPECTED_JSON_ARRAY);
+
+        response = exceptionHandler.generateResponseForException(new ClassCastException());
+        assertResponse(ServiceException.EXPECTED_JSON_ARRAY, response);
+
+        // Shouldn't affect the default one.
+        response = exceptionHandler.generateResponseForException(new NullPointerException());
+        assertResponse(ServiceException.UNEXPECTED_ERROR, response);
+    }
+
+    @Test
+    public void testChangeExceptions() {
+        HttpResponseExceptionHandler exceptionHandler = new HttpResponseExceptionHandler();
+        Response response = exceptionHandler.generateResponseForException(new ClassCastException());
+        assertResponse(ServiceException.UNEXPECTED_ERROR, response);
+
+        exceptionHandler.add(ClassCastException.class, ServiceException.EXPECTED_JSON_ARRAY);
+
+        response = exceptionHandler.generateResponseForException(new ClassCastException());
+        assertResponse(ServiceException.EXPECTED_JSON_ARRAY, response);
+
+        exceptionHandler.add(ClassCastException.class, ServiceException.DATABASE_UNAVAILABLE);
+
+        response = exceptionHandler.generateResponseForException(new ClassCastException());
+        assertResponse(ServiceException.DATABASE_UNAVAILABLE, response);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testNoNullsAllowedAddFirstArg() {
+        new HttpResponseExceptionHandler().add(null, ServiceException.UNEXPECTED_ERROR);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testNoNullsAllowedAddSecondArg() {
+        new HttpResponseExceptionHandler().add(Exception.class, null);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testNoNullsAllowedAddBothArgs() {
+        new HttpResponseExceptionHandler().add(null, null);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testNoNullsResponseNullException() {
+        new HttpResponseExceptionHandler().generateResponseForException(null);
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/common/core/src/test/java/com/redhat/thermostat/gateway/common/util/ServiceExceptionTest.java	Mon Jul 17 15:12:53 2017 -0400
@@ -0,0 +1,63 @@
+/*
+ * Copyright 2012-2017 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.gateway.common.util;
+
+import static com.redhat.thermostat.gateway.common.util.ServiceException.EXPECTED_JSON_ARRAY;
+import static com.redhat.thermostat.gateway.common.util.ServiceException.UNEXPECTED_ERROR;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import javax.ws.rs.core.Response;
+
+import org.junit.Test;
+
+public class ServiceExceptionTest {
+
+    @Test
+    public void buildsProperResponse() {
+        Response response = EXPECTED_JSON_ARRAY.buildResponse();
+        assertEquals(400, response.getStatus());
+        assertEquals("{\"error\":\"Expected JSON array\"}", (String) response.getEntity());
+    }
+
+    @Test
+    public void buildsProperDefaultResponse() {
+        Response response = UNEXPECTED_ERROR.buildResponse();
+        assertEquals(500, response.getStatus());
+        assertNull(response.getEntity());
+    }
+}
--- a/services/jvm-gc/src/main/java/com/redhat/thermostat/service/jvm/gc/JvmGcHttpHandler.java	Mon Jul 17 11:22:45 2017 -0400
+++ b/services/jvm-gc/src/main/java/com/redhat/thermostat/service/jvm/gc/JvmGcHttpHandler.java	Mon Jul 17 15:12:53 2017 -0400
@@ -36,6 +36,13 @@
 
 package com.redhat.thermostat.service.jvm.gc;
 
+import static com.redhat.thermostat.gateway.common.util.ServiceException.CANNOT_QUERY_REALMS_PROPERTY;
+import static com.redhat.thermostat.gateway.common.util.ServiceException.DATABASE_UNAVAILABLE;
+import static com.redhat.thermostat.gateway.common.util.ServiceException.EXPECTED_JSON_ARRAY;
+import static com.redhat.thermostat.gateway.common.util.ServiceException.MALFORMED_CLIENT_REQUEST;
+
+import java.io.IOException;
+
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.Consumes;
@@ -51,6 +58,8 @@
 import javax.ws.rs.core.Response;
 
 import com.mongodb.DBObject;
+import com.mongodb.MongoTimeoutException;
+import com.mongodb.MongoWriteException;
 import com.redhat.thermostat.gateway.common.core.auth.keycloak.RealmAuthorizer;
 import com.redhat.thermostat.gateway.common.mongodb.ThermostatMongoStorage;
 import com.redhat.thermostat.gateway.common.mongodb.executor.MongoDataResultContainer;
@@ -59,11 +68,23 @@
 import com.redhat.thermostat.gateway.common.mongodb.response.MongoMetaDataResponseBuilder;
 import com.redhat.thermostat.gateway.common.mongodb.response.MongoResponseBuilder;
 import com.redhat.thermostat.gateway.common.mongodb.servlet.ServletContextConstants;
+import com.redhat.thermostat.gateway.common.util.HttpResponseExceptionHandler;
+import org.bson.json.JsonParseException;
 
 @Path("/")
 public class JvmGcHttpHandler {
     private final MongoExecutor mongoExecutor = new MongoExecutor();
     private final String collectionName = "jvm-gc";
+    private final HttpResponseExceptionHandler exceptionHandler = new HttpResponseExceptionHandler();
+
+    public JvmGcHttpHandler() {
+        exceptionHandler.add(MongoWriteException.class, MALFORMED_CLIENT_REQUEST)
+                        .add(JsonParseException.class, MALFORMED_CLIENT_REQUEST)
+                        .add(UnsupportedOperationException.class, MALFORMED_CLIENT_REQUEST)
+                        .add(ClassCastException.class, EXPECTED_JSON_ARRAY)
+                        .add(MongoTimeoutException.class, DATABASE_UNAVAILABLE)
+                        .add(IOException.class, CANNOT_QUERY_REALMS_PROPERTY);
+    }
 
     @GET
     @Consumes({ "application/json" })
@@ -110,7 +131,7 @@
             }
             return Response.status(Response.Status.OK).entity(response.build()).build();
         } catch (Exception e) {
-            return Response.status(Response.Status.BAD_REQUEST).build();
+            return exceptionHandler.generateResponseForException(e);
         }
     }
 
@@ -138,7 +159,7 @@
 
             return Response.status(Response.Status.OK).build();
         } catch (Exception e) {
-            return Response.status(Response.Status.BAD_REQUEST).build();
+            return exceptionHandler.generateResponseForException(e);
         }
     }
 
@@ -164,7 +185,7 @@
             }
             return Response.status(Response.Status.OK).build();
         } catch (Exception e) {
-            return Response.status(Response.Status.BAD_REQUEST).build();
+            return exceptionHandler.generateResponseForException(e);
         }
     }
 
@@ -191,7 +212,7 @@
 
             return Response.status(Response.Status.OK).build();
         } catch (Exception e) {
-            return Response.status(Response.Status.BAD_REQUEST).build();
+            return exceptionHandler.generateResponseForException(e);
         }
     }
 }