Mercurial > hg > thermostat-ng > web-gateway
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); } } }