diff --git a/core/src/main/java/io/confluent/rest/exceptions/DebuggableExceptionMapper.java b/core/src/main/java/io/confluent/rest/exceptions/DebuggableExceptionMapper.java index d08bc51ccb..600b77ea63 100644 --- a/core/src/main/java/io/confluent/rest/exceptions/DebuggableExceptionMapper.java +++ b/core/src/main/java/io/confluent/rest/exceptions/DebuggableExceptionMapper.java @@ -52,8 +52,8 @@ public DebuggableExceptionMapper(RestConfig restConfig) { * @param exc Throwable that triggered this ExceptionMapper * @param status HTTP response status */ - public Response.ResponseBuilder createResponse(Throwable exc, Response.Status status, - String msg) { + public Response.ResponseBuilder createResponse(Throwable exc, int errorCode, + Response.Status status, String msg) { String readableMessage = msg; if (restConfig != null && restConfig.getBoolean(RestConfig.DEBUG_CONFIG)) { readableMessage += " " + exc.getClass().getName() + ": " + exc.getMessage(); @@ -68,7 +68,7 @@ public Response.ResponseBuilder createResponse(Throwable exc, Response.Status st // Ignore } } - final ErrorMessage message = new ErrorMessage(status.getStatusCode(), readableMessage); + final ErrorMessage message = new ErrorMessage(errorCode, readableMessage); return Response.status(status) .entity(message); diff --git a/core/src/main/java/io/confluent/rest/exceptions/GenericExceptionMapper.java b/core/src/main/java/io/confluent/rest/exceptions/GenericExceptionMapper.java index d3e3924598..6113cfe57e 100644 --- a/core/src/main/java/io/confluent/rest/exceptions/GenericExceptionMapper.java +++ b/core/src/main/java/io/confluent/rest/exceptions/GenericExceptionMapper.java @@ -32,7 +32,8 @@ public GenericExceptionMapper(RestConfig restConfig) { public Response toResponse(Throwable exc) { // There's no more specific information about the exception that can be passed back to the user, // so we can only use the generic message. Debug mode will append the exception info. - return createResponse(exc, Response.Status.INTERNAL_SERVER_ERROR, + return createResponse(exc, Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), + Response.Status.INTERNAL_SERVER_ERROR, Response.Status.INTERNAL_SERVER_ERROR.getReasonPhrase()).build(); } } diff --git a/core/src/main/java/io/confluent/rest/exceptions/RestException.java b/core/src/main/java/io/confluent/rest/exceptions/RestException.java new file mode 100644 index 0000000000..cd1b6d83ca --- /dev/null +++ b/core/src/main/java/io/confluent/rest/exceptions/RestException.java @@ -0,0 +1,49 @@ +/** + * Copyright 2015 Confluent Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ +package io.confluent.rest.exceptions; + +import javax.ws.rs.WebApplicationException; + +/** + * RestException is a subclass of WebApplicationException that always includes an error code and + * can be converted to the standard error format. It is automatically handled by the rest-utils + * exception mappers. For convenience, it provides subclasses for a few of the most common + * responses, e.g. NotFoundException. If you're using the standard error format, you + * should prefer these exceptions to the normal JAX-RS ones since they provide better error + * responses and force you to provide both the HTTP status code and a more specific error code. + */ +public class RestException extends WebApplicationException { + + private int errorCode; + + public RestException(final String message, final int status, final int errorCode) { + this(message, status, errorCode, (Throwable) null); + } + + public RestException(final String message, final int status, final int errorCode, + final Throwable cause) { + super(message, cause, status); + this.errorCode = errorCode; + } + + public int getStatus() { + return getResponse().getStatus(); + } + + public int getErrorCode() { + return errorCode; + } +} diff --git a/core/src/main/java/io/confluent/rest/exceptions/RestNotAuthorizedException.java b/core/src/main/java/io/confluent/rest/exceptions/RestNotAuthorizedException.java new file mode 100644 index 0000000000..d7fb7657ca --- /dev/null +++ b/core/src/main/java/io/confluent/rest/exceptions/RestNotAuthorizedException.java @@ -0,0 +1,30 @@ +/** + * Copyright 2015 Confluent Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +package io.confluent.rest.exceptions; + +import javax.ws.rs.core.Response; + +public class RestNotAuthorizedException extends RestException { + + public RestNotAuthorizedException(String message, int errorCode) { + super(message, Response.Status.UNAUTHORIZED.getStatusCode(), errorCode); + } + + public RestNotAuthorizedException(String message, int errorCode, Throwable cause) { + super(message, Response.Status.UNAUTHORIZED.getStatusCode(), errorCode, cause); + } +} \ No newline at end of file diff --git a/core/src/main/java/io/confluent/rest/exceptions/RestNotFoundException.java b/core/src/main/java/io/confluent/rest/exceptions/RestNotFoundException.java new file mode 100644 index 0000000000..0362e6de6f --- /dev/null +++ b/core/src/main/java/io/confluent/rest/exceptions/RestNotFoundException.java @@ -0,0 +1,30 @@ +/** + * Copyright 2015 Confluent Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +package io.confluent.rest.exceptions; + +import javax.ws.rs.core.Response; + +public class RestNotFoundException extends RestException { + + public RestNotFoundException(String message, int errorCode) { + super(message, Response.Status.NOT_FOUND.getStatusCode(), errorCode); + } + + public RestNotFoundException(String message, int errorCode, Throwable cause) { + super(message, Response.Status.NOT_FOUND.getStatusCode(), errorCode, cause); + } +} \ No newline at end of file diff --git a/core/src/main/java/io/confluent/rest/exceptions/WebApplicationExceptionMapper.java b/core/src/main/java/io/confluent/rest/exceptions/WebApplicationExceptionMapper.java index 03d4387143..99c222690a 100644 --- a/core/src/main/java/io/confluent/rest/exceptions/WebApplicationExceptionMapper.java +++ b/core/src/main/java/io/confluent/rest/exceptions/WebApplicationExceptionMapper.java @@ -43,7 +43,9 @@ public Response toResponse(WebApplicationException exc) { // The human-readable message for these can use the exception message directly. Since // WebApplicationExceptions are expected to be passed back to users, it will either contain a // situation-specific message or the HTTP status message - Response.ResponseBuilder response = createResponse(exc, status, exc.getMessage()); + int errorCode = (exc instanceof RestException) ? ((RestException)exc).getErrorCode() + : status.getStatusCode(); + Response.ResponseBuilder response = createResponse(exc, errorCode, status, exc.getMessage()); // Apparently, 415 Unsupported Media Type errors disable content negotiation in Jersey, which // causes use to return data without a content type. Work around this by detecting that specific diff --git a/core/src/test/java/io/confluent/rest/ExceptionHandlingTest.java b/core/src/test/java/io/confluent/rest/ExceptionHandlingTest.java new file mode 100644 index 0000000000..1ce139eef0 --- /dev/null +++ b/core/src/test/java/io/confluent/rest/ExceptionHandlingTest.java @@ -0,0 +1,129 @@ +/** + * Copyright 2015 Confluent Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +package io.confluent.rest; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.util.Properties; + +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.client.ClientBuilder; +import javax.ws.rs.core.Configurable; +import javax.ws.rs.core.Response; + +import io.confluent.rest.entities.ErrorMessage; +import io.confluent.rest.exceptions.RestNotFoundException; + +import static org.junit.Assert.assertEquals; + +/** + * Tests that a demo app catches exceptions correctly and returns errors in the expected format. + */ +public class ExceptionHandlingTest { + + RestConfig config; + ExceptionApplication app; + + @Before + public void setUp() throws Exception { + Properties props = new Properties(); + props.setProperty("debug", "false"); + config = new RestConfig(props); + app = new ExceptionApplication(config); + app.start(); + } + + @After + public void tearDown() throws Exception { + app.stop(); + app.join(); + } + + private void testAppException(String path, int expectedStatus, int expectedErrorCode, + String expectedMessage) { + Response response = ClientBuilder.newClient() + .target("http://localhost:" + config.getInt(RestConfig.PORT_CONFIG)) + .path(path) + .request() + .get(); + assertEquals(expectedStatus, response.getStatus()); + + ErrorMessage msg = response.readEntity(ErrorMessage.class); + assertEquals(expectedErrorCode, msg.getErrorCode()); + assertEquals(expectedMessage, msg.getMessage()); + } + + @Test + public void testRestException() { + testAppException("/restnotfound", 404, 4040, "Rest Not Found"); + } + + @Test + public void testNonRestException() { + // These just duplicate the HTTP status code but should carry the custom message through + testAppException("/notfound", 404, 404, "Generic Not Found"); + } + + @Test + public void testUnexpectedException() { + // Under non-debug mode, this uses a completely generic message since unexpected errors + // is the one case we want to be certain we don't leak extra info + testAppException("/unexpected", 500, 500, + Response.Status.INTERNAL_SERVER_ERROR.getReasonPhrase()); + } + + // Test app just has endpoints that trigger different types of exceptions. + private static class ExceptionApplication extends Application { + + ExceptionApplication(RestConfig props) { + super(props); + } + + @Override + public void setupResources(Configurable config, RestConfig appConfig) { + config.register(ExceptionResource.class); + } + } + + @Produces("application/json") + @Path("/") + public static class ExceptionResource { + + @GET + @Path("/restnotfound") + public String restNotFound() { + throw new RestNotFoundException("Rest Not Found", 4040); + } + + @GET + @Path("/notfound") + public String notFound() { + throw new javax.ws.rs.NotFoundException("Generic Not Found"); + } + + @GET + @Path("/unexpected") + public String unexpected() { + throw new RuntimeException("Internal server error."); + } + } + +} diff --git a/core/src/test/java/io/confluent/rest/WebApplicationExceptionMapperTest.java b/core/src/test/java/io/confluent/rest/WebApplicationExceptionMapperTest.java new file mode 100644 index 0000000000..b028925dd4 --- /dev/null +++ b/core/src/test/java/io/confluent/rest/WebApplicationExceptionMapperTest.java @@ -0,0 +1,62 @@ +/** + * Copyright 2015 Confluent Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +package io.confluent.rest; + +import org.junit.Before; +import org.junit.Test; + +import java.util.Properties; + +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.Response; + +import io.confluent.rest.entities.ErrorMessage; +import io.confluent.rest.exceptions.RestException; +import io.confluent.rest.exceptions.WebApplicationExceptionMapper; + +import static org.junit.Assert.*; + +public class WebApplicationExceptionMapperTest { + + private WebApplicationExceptionMapper mapper; + + @Before + public void setUp() { + Properties props = new Properties(); + props.setProperty("debug", "false"); + RestConfig config = new RestConfig(props); + mapper = new WebApplicationExceptionMapper(config); + } + + @Test + public void testRestException() { + Response response = mapper.toResponse(new RestException("msg", 400, 1000)); + assertEquals(400, response.getStatus()); + ErrorMessage out = (ErrorMessage)response.getEntity(); + assertEquals("msg", out.getMessage()); + assertEquals(1000, out.getErrorCode()); + } + + @Test + public void testNonRestWebApplicationException() { + Response response = mapper.toResponse(new WebApplicationException("msg", 400)); + assertEquals(400, response.getStatus()); + ErrorMessage out = (ErrorMessage)response.getEntity(); + assertEquals("msg", out.getMessage()); + assertEquals(400, out.getErrorCode()); + } +} \ No newline at end of file