Remove errorType from ServiceException args (#133)

Revert of #92 

Now that palantir/conjure-java-runtime#827 has merged and we are logging the error name in the exception mapper, we no longer need the injected arg here.

With the injected args, the `ServiceExceptionMapper` logs would have both a `errorName` param and a `throwable0_errorType` param which is confusing and unnecessary.

Also note that this parameter has never been logged properly (see #130), so this does not cause a regression in capability.

Author: pkoenig10

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

@@ -111,7 +111,7 @@ public static SerializableError forException(ServiceException exception) {
                 .errorName(exception.getErrorType().name())
                 .errorInstanceId(exception.getErrorInstanceId());
 
-        for (Arg<?> arg : exception.getParameters()) {
+        for (Arg<?> arg : exception.getArgs()) {
             builder.putParameters(arg.getName(), Objects.toString(arg.getValue()));
         }
 

errors/src/main/java/com/palantir/conjure/java/api/errors/ServiceException.java

@@ -17,7 +17,6 @@
 package com.palantir.conjure.java.api.errors;
 
 import com.palantir.logsafe.Arg;
-import com.palantir.logsafe.SafeArg;
 import com.palantir.logsafe.SafeLoggable;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -27,7 +26,6 @@
 
 /** A {@link ServiceException} thrown in server-side code to indicate server-side {@link ErrorType error states}. */
 public final class ServiceException extends RuntimeException implements SafeLoggable {
-    public static final String ERROR_TYPE_ARG_NAME = "errorType";
 
     private final ErrorType errorType;
     private final List<Arg<?>> args;  // unmodifiable
@@ -51,7 +49,7 @@ public ServiceException(ErrorType errorType, @Nullable Throwable cause, Arg<?>..
 
         this.errorType = errorType;
         // Note that instantiators cannot mutate List<> args since it comes through copyToList in all code paths.
-        this.args = collectArgs(errorType, args);
+        this.args = copyToUnmodifiableList(args);
         this.safeMessage = renderSafeMessage(errorType, args);
         this.noArgsMessage = renderNoArgsMessage(errorType);
     }
@@ -84,12 +82,19 @@ public List<Arg<?>> getArgs() {
     }
 
     /**
-     * Get Args of this service exception excluding injected/generated Args like errorType.
-     * @return parameters passed to this ServiceException at construction time
+     * Deprecated.
+     *
+     * @deprecated use {@link #getArgs}.
      */
+    @Deprecated
     public List<Arg<?>> getParameters() {
-        // errorType is always the first argument, so just slice it out
-        return args.subList(1, args.size());
+        return getArgs();
+    }
+
+    private static <T> List<T> copyToUnmodifiableList(T[] elements) {
+        List<T> list = new ArrayList<>(elements.length);
+        Collections.addAll(list, elements);
+        return Collections.unmodifiableList(list);
     }
 
     private static String renderSafeMessage(ErrorType errorType, Arg<?>... args) {
@@ -119,11 +124,4 @@ private static String renderSafeMessage(ErrorType errorType, Arg<?>... args) {
     private static String renderNoArgsMessage(ErrorType errorType) {
         return String.format("ServiceException: %s (%s)", errorType.code(), errorType.name());
     }
-
-    private static List<Arg<?>> collectArgs(ErrorType errorType, Arg<?>... args) {
-        List<Arg<?>> argList = new ArrayList<>(args.length + 1);
-        argList.add(SafeArg.of(ERROR_TYPE_ARG_NAME, errorType));
-        Collections.addAll(argList, args);
-        return Collections.unmodifiableList(argList);
-    }
 }

errors/src/test/java/com/palantir/conjure/java/api/errors/ServiceExceptionTest.java

@@ -18,17 +18,16 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-import com.google.common.collect.ImmutableList;
 import com.palantir.logsafe.Arg;
 import com.palantir.logsafe.SafeArg;
 import com.palantir.logsafe.UnsafeArg;
-import java.util.List;
 import java.util.UUID;
 import org.junit.Test;
 
 public final class ServiceExceptionTest {
 
-    private static final ErrorType ERROR = ErrorType.create(ErrorType.Code.CUSTOM_CLIENT, "Namespace:MyDesc");
+    private static final String ERROR_NAME = "Namespace:MyDesc";
+    private static final ErrorType ERROR = ErrorType.create(ErrorType.Code.CUSTOM_CLIENT, ERROR_NAME);
     private static final String EXPECTED_ERROR_MSG = "ServiceException: CUSTOM_CLIENT (Namespace:MyDesc)";
 
     @Test
@@ -41,32 +40,6 @@ public void testExceptionMessageContainsNoArgs_safeLogMessageContainsSafeArgsOnl
 
         assertThat(ex.getLogMessage()).isEqualTo(EXPECTED_ERROR_MSG);
         assertThat(ex.getMessage()).isEqualTo(EXPECTED_ERROR_MSG + ": {arg1=foo}");
-
-        List<Arg<?>> expectedArgs = ImmutableList.<Arg<?>>builder()
-                .add(SafeArg.of(ServiceException.ERROR_TYPE_ARG_NAME, ERROR))
-                .add(args)
-                .build();
-
-        assertThat(ex.getArgs()).isEqualTo(expectedArgs);
-    }
-
-    @Test
-    public void testExceptionMessageWithNameCollisionWithInjectedArgs() {
-        Arg<?>[] args = {
-                SafeArg.of(ServiceException.ERROR_TYPE_ARG_NAME, "foo"),
-                UnsafeArg.of("arg2", 2),
-                UnsafeArg.of("arg3", null)};
-        ServiceException ex = new ServiceException(ERROR, args);
-
-        assertThat(ex.getLogMessage()).isEqualTo(EXPECTED_ERROR_MSG);
-        assertThat(ex.getMessage()).isEqualTo(EXPECTED_ERROR_MSG + ": {errorType=foo}");
-
-        List<Arg<?>> expectedArgs = ImmutableList.<Arg<?>>builder()
-                .add(SafeArg.of(ServiceException.ERROR_TYPE_ARG_NAME, ERROR))
-                .add(args)
-                .build();
-
-        assertThat(ex.getArgs()).isEqualTo(expectedArgs);
     }
 
     @Test

test-utils/src/main/java/com/palantir/conjure/java/api/testing/ServiceExceptionAssert.java

@@ -41,7 +41,6 @@ public final ServiceExceptionAssert hasType(ErrorType type) {
     public final ServiceExceptionAssert hasArgs(Arg<?>... args) {
         isNotNull();
 
-        // fetch getParameters here to exclude any generated/injected args like 'errorType'
         AssertableArgs actualArgs = new AssertableArgs(actual.getParameters());
         AssertableArgs expectedArgs = new AssertableArgs(Arrays.asList(args));