SerializableError#params now contains unsafe args in addition to safe args (#65)

* SerializableError#params now contains unsafe args in addition to safe args

* Use for loop (200ns) instead of stream collect toMap (260ns)

Author: iamdanfox

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

@@ -107,17 +107,16 @@ String getMessage() {
 
     /**
      * Creates a {@link SerializableError} representation of this exception that derives from the error code and
-     * message, as well as the {@link Arg#isSafeForLogging safe} {@link ServiceException#args parameters}.
+     * message, as well as the {@link Arg#isSafeForLogging safe} and unsafe {@link ServiceException#args parameters}.
      */
     public static SerializableError forException(ServiceException exception) {
         Builder builder = new Builder()
                 .errorCode(exception.getErrorType().code().name())
                 .errorName(exception.getErrorType().name())
                 .errorInstanceId(exception.getErrorInstanceId());
+
         for (Arg<?> arg : exception.getArgs()) {
-            if (arg.isSafeForLogging()) {
-                builder.putParameters(arg.getName(), arg.getValue().toString());
-            }
+            builder.putParameters(arg.getName(), arg.getValue().toString());
         }
 
         return builder.build();

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

@@ -35,15 +35,35 @@ public final class SerializableErrorTest {
     private static final ObjectMapper mapper = ObjectMappers.newServerObjectMapper();
 
     @Test
-    public void testExceptionToError() {
+    public void forException_should_keep_both_safe_and_unsafe_args() {
         ErrorType error = ErrorType.FAILED_PRECONDITION;
-        ServiceException exception =
-                new ServiceException(error, SafeArg.of("safeKey", 42), UnsafeArg.of("foo", "bar"));
+        ServiceException exception = new ServiceException(error,
+                SafeArg.of("safeKey", 42),
+                UnsafeArg.of("sensitiveInfo", "some user-entered content"));
+
         SerializableError expected = new SerializableError.Builder()
                 .errorCode(error.code().name())
                 .errorName(error.name())
                 .errorInstanceId(exception.getErrorInstanceId())
                 .putParameters("safeKey", "42")
+                .putParameters("sensitiveInfo", "some user-entered content")
+                .build();
+        assertThat(SerializableError.forException(exception)).isEqualTo(expected);
+    }
+
+    @Test
+    public void forException_arg_key_collisions_just_use_the_last_one() {
+        ErrorType error = ErrorType.INTERNAL;
+        ServiceException exception = new ServiceException(
+                error,
+                SafeArg.of("collision", "first"),
+                UnsafeArg.of("collision", "second"));
+
+        SerializableError expected = new SerializableError.Builder()
+                .errorCode(error.code().name())
+                .errorName(error.name())
+                .errorInstanceId(exception.getErrorInstanceId())
+                .putParameters("collision", "second")
                 .build();
         assertThat(SerializableError.forException(exception)).isEqualTo(expected);
     }