diff --git a/errors/src/main/java/com/palantir/remoting/api/errors/ErrorType.java b/errors/src/main/java/com/palantir/remoting/api/errors/ErrorType.java index 2de96ec2d..415a49ce5 100644 --- a/errors/src/main/java/com/palantir/remoting/api/errors/ErrorType.java +++ b/errors/src/main/java/com/palantir/remoting/api/errors/ErrorType.java @@ -20,8 +20,10 @@ import org.immutables.value.Value; /** - * Represents errors by code and name. {@link ErrorType} instance are meant to be compile-time constants in - * the sense that the name should not contain information that is available at runtime only. + * Represents errors by code and name. {@link ErrorType} instance are meant to be compile-time constants in the sense + * that the name should not contain information that is available at runtime only. By convention, and in alignment with + * the HTTP specification, errors associated with a {@code 4xx} HTTP status code are client errors, and errors + * associated with a {@code 5xx} status are server errors. */ @Value.Immutable @ImmutablesStyle @@ -30,12 +32,12 @@ public abstract class ErrorType { private static final Pattern UPPER_CAMEL_CASE = Pattern.compile("([A-Z][a-z]+)+"); public enum Code { - UNKNOWN(500), PERMISSION_DENIED(403), INVALID_ARGUMENT(400), - FAILED_PRECONDITION(400), + FAILED_PRECONDITION(500), INTERNAL(500), - CUSTOM(null /* unused */); + CUSTOM_CLIENT(400), + CUSTOM_SERVER(500); private final Integer httpErrorCode; // boxed so that we fail loudly if someone accesses CUSTOM.httpErrorCode @@ -44,7 +46,6 @@ public enum Code { } } - public static final ErrorType UNKNOWN = createInternal(Code.UNKNOWN, "Unknown"); public static final ErrorType PERMISSION_DENIED = createInternal(Code.PERMISSION_DENIED, "PermissionDenied"); public static final ErrorType INVALID_ARGUMENT = createInternal(Code.INVALID_ARGUMENT, "InvalidArgument"); public static final ErrorType FAILED_PRECONDITION = createInternal(Code.FAILED_PRECONDITION, "FailedPrecondition"); @@ -70,27 +71,27 @@ final void check() { } /** - * Creates a new error type with the given name and HTTP error code, and error type{@link Code#CUSTOM}. - * Allowed error codes are {@code 400 BAD REQUEST} and {@code 500 INTERNAL SERVER ERROR}. + * Creates a new error type with code {@link Code#CUSTOM_CLIENT} and the given name. */ - public static ErrorType custom(String name, int httpErrorCode) { - if (httpErrorCode != 400 && httpErrorCode != 500) { - throw new IllegalArgumentException("CUSTOM ErrorTypes must have HTTP error code 400 or 500"); - } - return ImmutableErrorType.builder() - .code(Code.CUSTOM) - .name(name) - .httpErrorCode(httpErrorCode) - .build(); + public static ErrorType client(String name) { + return createInternal(Code.CUSTOM_CLIENT, name); + } + + /** + * Creates a new error type with code {@link Code#CUSTOM_SERVER} and the given name. + */ + public static ErrorType server(String name) { + return createInternal(Code.CUSTOM_SERVER, name); } /** * Constructs an {@link ErrorType} with the given error {@link Code} and name. Cannot use the {@link - * Code#CUSTOM} error code, see {@link #custom} instead. + * Code#CUSTOM_CLIENT} or {@link Code#CUSTOM_SERVER} error codes, see {@link #client} and {@link #server} instead. */ - public static ErrorType of(Code code, String name) { - if (code == Code.CUSTOM) { - throw new IllegalArgumentException("Use the custom() method to construct ErrorTypes with code CUSTOM"); + public static ErrorType create(Code code, String name) { + if (code == Code.CUSTOM_CLIENT || code == Code.CUSTOM_SERVER) { + throw new IllegalArgumentException("Use the client() or server() methods to construct " + + "ErrorTypes with code CUSTOM_CLIENT or CUSTOM_SERVER"); } return createInternal(code, name); } diff --git a/errors/src/test/java/com/palantir/remoting/api/errors/ErrorTypeTest.java b/errors/src/test/java/com/palantir/remoting/api/errors/ErrorTypeTest.java index 9761faab4..eeb16b261 100644 --- a/errors/src/test/java/com/palantir/remoting/api/errors/ErrorTypeTest.java +++ b/errors/src/test/java/com/palantir/remoting/api/errors/ErrorTypeTest.java @@ -25,56 +25,56 @@ public final class ErrorTypeTest { @Test public void testNameMustBeCamelCase() throws Exception { - assertThatThrownBy(() -> ErrorType.of(ErrorType.Code.FAILED_PRECONDITION, "foo")) + assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.FAILED_PRECONDITION, "foo")) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("ErrorType names must be UpperCamelCase: foo"); - assertThatThrownBy(() -> ErrorType.custom("foo", 400)) + assertThatThrownBy(() -> ErrorType.client("foo")) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("ErrorType names must be UpperCamelCase: foo"); - assertThatThrownBy(() -> ErrorType.custom("fooBar", 400)) + assertThatThrownBy(() -> ErrorType.client("fooBar")) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("ErrorType names must be UpperCamelCase: fooBar"); - assertThatThrownBy(() -> ErrorType.custom("", 400)) + assertThatThrownBy(() -> ErrorType.client("")) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("ErrorType names must be UpperCamelCase: "); } @Test public void testDefaultErrorTypeHttpErrorCodes() throws Exception { - assertThat(ErrorType.UNKNOWN.httpErrorCode()).isEqualTo(500); assertThat(ErrorType.PERMISSION_DENIED.httpErrorCode()).isEqualTo(403); assertThat(ErrorType.INVALID_ARGUMENT.httpErrorCode()).isEqualTo(400); - assertThat(ErrorType.FAILED_PRECONDITION.httpErrorCode()).isEqualTo(400); + assertThat(ErrorType.FAILED_PRECONDITION.httpErrorCode()).isEqualTo(500); assertThat(ErrorType.INTERNAL.httpErrorCode()).isEqualTo(500); } @Test public void testCustomErrors() throws Exception { - ErrorType custom400 = ErrorType.custom("MyDesc", 400); - assertThat(custom400.code()).isEqualTo(ErrorType.Code.CUSTOM); - assertThat(custom400.httpErrorCode()).isEqualTo(400); - assertThat(custom400.name()).isEqualTo("MyDesc"); + ErrorType customClient = ErrorType.client("MyDesc"); + assertThat(customClient.code()).isEqualTo(ErrorType.Code.CUSTOM_CLIENT); + assertThat(customClient.httpErrorCode()).isEqualTo(400); + assertThat(customClient.name()).isEqualTo("MyDesc"); - ErrorType custom500 = ErrorType.custom("MyDesc", 500); - assertThat(custom500.code()).isEqualTo(ErrorType.Code.CUSTOM); - assertThat(custom500.httpErrorCode()).isEqualTo(500); - assertThat(custom500.name()).isEqualTo("MyDesc"); - - assertThatThrownBy(() -> ErrorType.custom("MyDesc", 403)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("CUSTOM ErrorTypes must have HTTP error code 400 or 500"); + ErrorType customServer = ErrorType.server("MyDesc"); + assertThat(customServer.code()).isEqualTo(ErrorType.Code.CUSTOM_SERVER); + assertThat(customServer.httpErrorCode()).isEqualTo(500); + assertThat(customServer.name()).isEqualTo("MyDesc"); } @Test public void testCanCreateNewErrorTypes() throws Exception { - ErrorType error = ErrorType.of(ErrorType.Code.FAILED_PRECONDITION, "MyDesc"); + ErrorType error = ErrorType.create(ErrorType.Code.FAILED_PRECONDITION, "MyDesc"); assertThat(error.code()).isEqualTo(ErrorType.Code.FAILED_PRECONDITION); - assertThat(error.httpErrorCode()).isEqualTo(400); + assertThat(error.httpErrorCode()).isEqualTo(500); assertThat(error.name()).isEqualTo("MyDesc"); - assertThatThrownBy(() -> ErrorType.of(ErrorType.Code.CUSTOM, "MyDesc")) + assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.CUSTOM_CLIENT, "MyDesc")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Use the client() or server() methods to construct ErrorTypes with code CUSTOM_CLIENT " + + "or CUSTOM_SERVER"); + assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.CUSTOM_SERVER, "MyDesc")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Use the custom() method to construct ErrorTypes with code CUSTOM"); + .hasMessage("Use the client() or server() methods to construct ErrorTypes with code CUSTOM_CLIENT " + + "or CUSTOM_SERVER"); } } diff --git a/errors/src/test/java/com/palantir/remoting/api/errors/ServiceExceptionTest.java b/errors/src/test/java/com/palantir/remoting/api/errors/ServiceExceptionTest.java index 835f3a255..d89f2a316 100644 --- a/errors/src/test/java/com/palantir/remoting/api/errors/ServiceExceptionTest.java +++ b/errors/src/test/java/com/palantir/remoting/api/errors/ServiceExceptionTest.java @@ -26,8 +26,8 @@ public final class ServiceExceptionTest { - private static final ErrorType ERROR = ErrorType.custom("MyDesc", 400); - private static final String EXPECTED_ERROR_MSG = "ServiceException: CUSTOM (MyDesc)"; + private static final ErrorType ERROR = ErrorType.client("MyDesc"); + private static final String EXPECTED_ERROR_MSG = "ServiceException: CUSTOM_CLIENT (MyDesc)"; @Test public void testExceptionMessageContainsNoArgs_safeLogMessageContainsSafeArgsOnly() {