[fix][break] fix #148: ServiceException.getMessage includes unsafe args (#158)

This change provides parity with safe-logging Safe*Exception
types, see palantir/safe-logging#76

By providing all argument data in the exception message we
provide full context to logging systems which do not understand
safe-logging types.

Author: carterkozak

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

@@ -31,7 +31,7 @@ public final class ServiceException extends RuntimeException implements SafeLogg
     private final List<Arg<?>> args;  // unmodifiable
 
     private final String errorInstanceId = UUID.randomUUID().toString();
-    private final String safeMessage;
+    private final String unsafeMessage;
     private final String noArgsMessage;
 
     /**
@@ -45,12 +45,12 @@ public ServiceException(ErrorType errorType, Arg<?>... parameters) {
     /** As above, but additionally records the cause of this exception. */
     public ServiceException(ErrorType errorType, @Nullable Throwable cause, Arg<?>... args) {
         // TODO(rfink): Memoize formatting?
-        super(renderSafeMessage(errorType, args), cause);
+        super(cause);
 
         this.errorType = errorType;
         // Note that instantiators cannot mutate List<> args since it comes through copyToList in all code paths.
         this.args = copyToUnmodifiableList(args);
-        this.safeMessage = renderSafeMessage(errorType, args);
+        this.unsafeMessage = renderUnsafeMessage(errorType, args);
         this.noArgsMessage = renderNoArgsMessage(errorType);
     }
 
@@ -66,8 +66,8 @@ public String getErrorInstanceId() {
 
     @Override
     public String getMessage() {
-        // Including safe args here since any logger not configured with safe-logging will log this message.
-        return safeMessage;
+        // Including all args here since any logger not configured with safe-logging will log this message.
+        return unsafeMessage;
     }
 
     @Override
@@ -97,7 +97,7 @@ private static <T> List<T> copyToUnmodifiableList(T[] elements) {
         return Collections.unmodifiableList(list);
     }
 
-    private static String renderSafeMessage(ErrorType errorType, Arg<?>... args) {
+    private static String renderUnsafeMessage(ErrorType errorType, Arg<?>... args) {
         String message = renderNoArgsMessage(errorType);
 
         if (args.length == 0) {
@@ -106,15 +106,13 @@ private static String renderSafeMessage(ErrorType errorType, Arg<?>... args) {
 
         StringBuilder builder = new StringBuilder();
         builder.append(message).append(": {");
-        int consumedArgs = 0;
-        for (Arg<?> arg : args) {
-            if (arg.isSafeForLogging()) {
-                if (consumedArgs++ > 0) {
-                    builder.append(", ");
-                }
-
-                builder.append(arg.getName()).append("=").append(arg.getValue());
+        for (int i = 0; i < args.length; i++) {
+            Arg<?> arg = args[i];
+            if (i > 0) {
+                builder.append(", ");
             }
+
+            builder.append(arg.getName()).append("=").append(arg.getValue());
         }
         builder.append("}");
 

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

@@ -39,7 +39,7 @@ public void testExceptionMessageContainsNoArgs_safeLogMessageContainsSafeArgsOnl
         ServiceException ex = new ServiceException(ERROR, args);
 
         assertThat(ex.getLogMessage()).isEqualTo(EXPECTED_ERROR_MSG);
-        assertThat(ex.getMessage()).isEqualTo(EXPECTED_ERROR_MSG + ": {arg1=foo}");
+        assertThat(ex.getMessage()).isEqualTo(EXPECTED_ERROR_MSG + ": {arg1=foo, arg2=2, arg3=null}");
     }
 
     @Test
@@ -51,7 +51,7 @@ public void testExceptionMessageWithDuplicateKeys() {
     @Test
     public void testExceptionMessageWithUnsafeArgs() {
         ServiceException ex = new ServiceException(ERROR, UnsafeArg.of("arg1", 1), SafeArg.of("arg2", 2));
-        assertThat(ex.getMessage()).isEqualTo(EXPECTED_ERROR_MSG + ": {arg2=2}");
+        assertThat(ex.getMessage()).isEqualTo(EXPECTED_ERROR_MSG + ": {arg1=1, arg2=2}");
     }
 
     @Test