[fix] SerializableError#asConjure() handles both remoting2 and remoting3 JSON keys (#124)

Author: iamdanfox

errors/src/main/java/com/palantir/remoting/api/errors/SerializableError.java

@@ -119,16 +119,16 @@ public final com.palantir.conjure.java.api.errors.SerializableError asConjure()
         com.palantir.conjure.java.api.errors.SerializableError.Builder builder =
                 com.palantir.conjure.java.api.errors.SerializableError.builder();
 
-        if (!getExceptionClass().isPresent()) {
-            builder.errorCode(errorCode());
-            builder.errorName(errorName());
-        } else {
+        if (getExceptionClass().isPresent()) {
             // this is for back-compat, old remoting2 exceptions have 'exceptionClass' and 'message' fields.
             builder.exceptionClass(getExceptionClass().get());
             builder.message(getMessage().get());
         }
 
+        builder.errorName(errorName());
+        builder.errorCode(errorCode());
         builder.errorInstanceId(errorInstanceId());
+        builder.parameters(parameters());
 
         return builder.build();
     }

errors/src/test/java/com/palantir/remoting/api/errors/SerializableErrorTest.java

@@ -34,6 +34,10 @@ public final class SerializableErrorTest {
             .build();
     private static final ObjectMapper mapper = ObjectMappers.newServerObjectMapper();
 
+    private static SerializableError deserialize(String serialized) throws IOException {
+        return mapper.readValue(serialized, SerializableError.class);
+    }
+
     @Test
     public void forException_should_keep_both_safe_and_unsafe_args() {
         ErrorType error = ErrorType.FAILED_PRECONDITION;
@@ -109,15 +113,59 @@ public void testDeserializationFailsWhenNeitherErrorNameNorMessageIsSet() throws
     }
 
     @Test
-    public void asConjure_converts_json_nicely() throws IOException {
+    public void asConjure_converts_remoting3_json() throws IOException {
         assertThat(deserialize("{\"errorCode\":\"code\",\"errorName\":\"name\"}").asConjure())
                 .isEqualTo(com.palantir.conjure.java.api.errors.SerializableError.builder()
                         .errorCode("code")
                         .errorName("name")
                         .build());
     }
 
-    private static SerializableError deserialize(String serialized) throws IOException {
-        return mapper.readValue(serialized, SerializableError.class);
+    @Test
+    public void asConjure_converts_remoting2_json() throws IOException {
+        assertThat(deserialize("{\"exceptionClass\":\"java.lang.IllegalStateException\","
+                + "\"message\":\"Human readable message\"}").asConjure())
+                .isEqualTo(com.palantir.conjure.java.api.errors.SerializableError.builder()
+                        .exceptionClass("java.lang.IllegalStateException")
+                        .message("Human readable message")
+                        .build());
+    }
+
+    @Test
+    public void asConjure_converts_json_with_both_remoting2_and_remoting3_fields() throws IOException {
+        assertThat(deserialize("{\"errorCode\":\"PERMISSION_DENIED\",\"errorName\":\"Product:SomethingBroke\","
+                + "\"exceptionClass\":\"java.lang.IllegalStateException\",\"message\":\"Human readable message\","
+                + "\"errorInstanceId\":\"7d345390-e41d-49dd-8dcb-38dd716c0347\",\"parameters\":{\"a\":\"1\"}}")
+                .asConjure())
+                .isEqualTo(com.palantir.conjure.java.api.errors.SerializableError.builder()
+                        .errorCode("PERMISSION_DENIED")
+                        .errorName("Product:SomethingBroke")
+                        .errorInstanceId("7d345390-e41d-49dd-8dcb-38dd716c0347")
+                        .putParameters("a", "1")
+                        .build());
+    }
+
+    @Test
+    public void all_fields_are_preserved_when_present() throws IOException {
+        String string = "{\n"
+                + "    \"errorCode\":\"CONFLICT\",\n"
+                + "    \"errorName\":\"Product:TransactionConflict\",\n"
+                + "    \"errorInstanceId\":\"7d345390-e41d-49dd-8dcb-38dd716c0347\",\n"
+                + "    \"parameters\":{},\n"
+                + "    \"exceptionClass\":\"CONFLICT\",\n"
+                + "    \"message\":\"Refer to the server logs with this errorInstanceId: "
+                + "7d345390-e41d-49dd-8dcb-38dd716c0347\"\n"
+                + "}";
+
+        com.palantir.conjure.java.api.errors.SerializableError expected =
+                com.palantir.conjure.java.api.errors.SerializableError.builder()
+                .errorCode("CONFLICT")
+                .errorName("Product:TransactionConflict")
+                .errorInstanceId("7d345390-e41d-49dd-8dcb-38dd716c0347")
+                .message("Refer to the server logs with this errorInstanceId: 7d345390-e41d-49dd-8dcb-38dd716c0347")
+                .exceptionClass("CONFLICT")
+                .build();
+
+        assertThat(deserialize(string).asConjure()).isEqualTo(expected);
     }
 }