Skip to content

Commit 929f8fe

Browse files
author
Robert Fink
authored
Indicate client-vs-server error types (#14)
1 parent b6ef095 commit 929f8fe

File tree

3 files changed

+46
-45
lines changed

3 files changed

+46
-45
lines changed

‎errors/src/main/java/com/palantir/remoting/api/errors/ErrorType.java‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
import org.immutables.value.Value;
2121

2222
/**
23-
* Represents errors by code and name. {@link ErrorType} instance are meant to be compile-time constants in
24-
* the sense that the name should not contain information that is available at runtime only.
23+
* Represents errors by code and name. {@link ErrorType} instance are meant to be compile-time constants in the sense
24+
* that the name should not contain information that is available at runtime only. By convention, and in alignment with
25+
* the HTTP specification, errors associated with a {@code 4xx} HTTP status code are client errors, and errors
26+
* associated with a {@code 5xx} status are server errors.
2527
*/
2628
@Value.Immutable
2729
@ImmutablesStyle
@@ -30,12 +32,12 @@ public abstract class ErrorType {
3032
private static final Pattern UPPER_CAMEL_CASE = Pattern.compile("([A-Z][a-z]+)+");
3133

3234
public enum Code {
33-
UNKNOWN(500),
3435
PERMISSION_DENIED(403),
3536
INVALID_ARGUMENT(400),
36-
FAILED_PRECONDITION(400),
37+
FAILED_PRECONDITION(500),
3738
INTERNAL(500),
38-
CUSTOM(null /* unused */);
39+
CUSTOM_CLIENT(400),
40+
CUSTOM_SERVER(500);
3941

4042
private final Integer httpErrorCode; // boxed so that we fail loudly if someone accesses CUSTOM.httpErrorCode
4143

@@ -44,7 +46,6 @@ public enum Code {
4446
}
4547
}
4648

47-
public static final ErrorType UNKNOWN = createInternal(Code.UNKNOWN, "Unknown");
4849
public static final ErrorType PERMISSION_DENIED = createInternal(Code.PERMISSION_DENIED, "PermissionDenied");
4950
public static final ErrorType INVALID_ARGUMENT = createInternal(Code.INVALID_ARGUMENT, "InvalidArgument");
5051
public static final ErrorType FAILED_PRECONDITION = createInternal(Code.FAILED_PRECONDITION, "FailedPrecondition");
@@ -70,27 +71,27 @@ final void check() {
7071
}
7172

7273
/**
73-
* Creates a new error type with the given name and HTTP error code, and error type{@link Code#CUSTOM}.
74-
* Allowed error codes are {@code 400 BAD REQUEST} and {@code 500 INTERNAL SERVER ERROR}.
74+
* Creates a new error type with code {@link Code#CUSTOM_CLIENT} and the given name.
7575
*/
76-
public static ErrorType custom(String name, int httpErrorCode) {
77-
if (httpErrorCode != 400 && httpErrorCode != 500) {
78-
throw new IllegalArgumentException("CUSTOM ErrorTypes must have HTTP error code 400 or 500");
79-
}
80-
return ImmutableErrorType.builder()
81-
.code(Code.CUSTOM)
82-
.name(name)
83-
.httpErrorCode(httpErrorCode)
84-
.build();
76+
public static ErrorType client(String name) {
77+
return createInternal(Code.CUSTOM_CLIENT, name);
78+
}
79+
80+
/**
81+
* Creates a new error type with code {@link Code#CUSTOM_SERVER} and the given name.
82+
*/
83+
public static ErrorType server(String name) {
84+
return createInternal(Code.CUSTOM_SERVER, name);
8585
}
8686

8787
/**
8888
* Constructs an {@link ErrorType} with the given error {@link Code} and name. Cannot use the {@link
89-
* Code#CUSTOM} error code, see {@link #custom} instead.
89+
* Code#CUSTOM_CLIENT} or {@link Code#CUSTOM_SERVER} error codes, see {@link #client} and {@link #server} instead.
9090
*/
91-
public static ErrorType of(Code code, String name) {
92-
if (code == Code.CUSTOM) {
93-
throw new IllegalArgumentException("Use the custom() method to construct ErrorTypes with code CUSTOM");
91+
public static ErrorType create(Code code, String name) {
92+
if (code == Code.CUSTOM_CLIENT || code == Code.CUSTOM_SERVER) {
93+
throw new IllegalArgumentException("Use the client() or server() methods to construct "
94+
+ "ErrorTypes with code CUSTOM_CLIENT or CUSTOM_SERVER");
9495
}
9596
return createInternal(code, name);
9697
}

‎errors/src/test/java/com/palantir/remoting/api/errors/ErrorTypeTest.java‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,56 +25,56 @@ public final class ErrorTypeTest {
2525

2626
@Test
2727
public void testNameMustBeCamelCase() throws Exception {
28-
assertThatThrownBy(() -> ErrorType.of(ErrorType.Code.FAILED_PRECONDITION, "foo"))
28+
assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.FAILED_PRECONDITION, "foo"))
2929
.isInstanceOf(IllegalArgumentException.class)
3030
.hasMessageStartingWith("ErrorType names must be UpperCamelCase: foo");
3131

32-
assertThatThrownBy(() -> ErrorType.custom("foo", 400))
32+
assertThatThrownBy(() -> ErrorType.client("foo"))
3333
.isInstanceOf(IllegalArgumentException.class)
3434
.hasMessageStartingWith("ErrorType names must be UpperCamelCase: foo");
35-
assertThatThrownBy(() -> ErrorType.custom("fooBar", 400))
35+
assertThatThrownBy(() -> ErrorType.client("fooBar"))
3636
.isInstanceOf(IllegalArgumentException.class)
3737
.hasMessageStartingWith("ErrorType names must be UpperCamelCase: fooBar");
38-
assertThatThrownBy(() -> ErrorType.custom("", 400))
38+
assertThatThrownBy(() -> ErrorType.client(""))
3939
.isInstanceOf(IllegalArgumentException.class)
4040
.hasMessageStartingWith("ErrorType names must be UpperCamelCase: ");
4141
}
4242

4343
@Test
4444
public void testDefaultErrorTypeHttpErrorCodes() throws Exception {
45-
assertThat(ErrorType.UNKNOWN.httpErrorCode()).isEqualTo(500);
4645
assertThat(ErrorType.PERMISSION_DENIED.httpErrorCode()).isEqualTo(403);
4746
assertThat(ErrorType.INVALID_ARGUMENT.httpErrorCode()).isEqualTo(400);
48-
assertThat(ErrorType.FAILED_PRECONDITION.httpErrorCode()).isEqualTo(400);
47+
assertThat(ErrorType.FAILED_PRECONDITION.httpErrorCode()).isEqualTo(500);
4948
assertThat(ErrorType.INTERNAL.httpErrorCode()).isEqualTo(500);
5049
}
5150

5251
@Test
5352
public void testCustomErrors() throws Exception {
54-
ErrorType custom400 = ErrorType.custom("MyDesc", 400);
55-
assertThat(custom400.code()).isEqualTo(ErrorType.Code.CUSTOM);
56-
assertThat(custom400.httpErrorCode()).isEqualTo(400);
57-
assertThat(custom400.name()).isEqualTo("MyDesc");
53+
ErrorType customClient = ErrorType.client("MyDesc");
54+
assertThat(customClient.code()).isEqualTo(ErrorType.Code.CUSTOM_CLIENT);
55+
assertThat(customClient.httpErrorCode()).isEqualTo(400);
56+
assertThat(customClient.name()).isEqualTo("MyDesc");
5857

59-
ErrorType custom500 = ErrorType.custom("MyDesc", 500);
60-
assertThat(custom500.code()).isEqualTo(ErrorType.Code.CUSTOM);
61-
assertThat(custom500.httpErrorCode()).isEqualTo(500);
62-
assertThat(custom500.name()).isEqualTo("MyDesc");
63-
64-
assertThatThrownBy(() -> ErrorType.custom("MyDesc", 403))
65-
.isInstanceOf(IllegalArgumentException.class)
66-
.hasMessage("CUSTOM ErrorTypes must have HTTP error code 400 or 500");
58+
ErrorType customServer = ErrorType.server("MyDesc");
59+
assertThat(customServer.code()).isEqualTo(ErrorType.Code.CUSTOM_SERVER);
60+
assertThat(customServer.httpErrorCode()).isEqualTo(500);
61+
assertThat(customServer.name()).isEqualTo("MyDesc");
6762
}
6863

6964
@Test
7065
public void testCanCreateNewErrorTypes() throws Exception {
71-
ErrorType error = ErrorType.of(ErrorType.Code.FAILED_PRECONDITION, "MyDesc");
66+
ErrorType error = ErrorType.create(ErrorType.Code.FAILED_PRECONDITION, "MyDesc");
7267
assertThat(error.code()).isEqualTo(ErrorType.Code.FAILED_PRECONDITION);
73-
assertThat(error.httpErrorCode()).isEqualTo(400);
68+
assertThat(error.httpErrorCode()).isEqualTo(500);
7469
assertThat(error.name()).isEqualTo("MyDesc");
7570

76-
assertThatThrownBy(() -> ErrorType.of(ErrorType.Code.CUSTOM, "MyDesc"))
71+
assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.CUSTOM_CLIENT, "MyDesc"))
72+
.isInstanceOf(IllegalArgumentException.class)
73+
.hasMessage("Use the client() or server() methods to construct ErrorTypes with code CUSTOM_CLIENT "
74+
+ "or CUSTOM_SERVER");
75+
assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.CUSTOM_SERVER, "MyDesc"))
7776
.isInstanceOf(IllegalArgumentException.class)
78-
.hasMessage("Use the custom() method to construct ErrorTypes with code CUSTOM");
77+
.hasMessage("Use the client() or server() methods to construct ErrorTypes with code CUSTOM_CLIENT "
78+
+ "or CUSTOM_SERVER");
7979
}
8080
}

‎errors/src/test/java/com/palantir/remoting/api/errors/ServiceExceptionTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@
2626

2727
public final class ServiceExceptionTest {
2828

29-
private static final ErrorType ERROR = ErrorType.custom("MyDesc", 400);
30-
private static final String EXPECTED_ERROR_MSG = "ServiceException: CUSTOM (MyDesc)";
29+
private static final ErrorType ERROR = ErrorType.client("MyDesc");
30+
private static final String EXPECTED_ERROR_MSG = "ServiceException: CUSTOM_CLIENT (MyDesc)";
3131

3232
@Test
3333
public void testExceptionMessageContainsNoArgs_safeLogMessageContainsSafeArgsOnly() {

0 commit comments

Comments
 (0)