core: remove potential this-escapes by extracting constructions of StatusException and StatusRuntimeException to builders

Author: tharakadesilva

api/src/main/java/io/grpc/InternalStatus.java

@@ -44,6 +44,10 @@ private InternalStatus() {}
   @Internal
   public static final StatusRuntimeException asRuntimeException(Status status,
       @Nullable Metadata trailers, boolean fillInStackTrace) {
-    return new StatusRuntimeException(status, trailers, fillInStackTrace);
+    return new StatusRuntimeExceptionBuilder()
+        .setStatus(status)
+        .setTrailers(trailers)
+        .setFillInStackTrace(fillInStackTrace)
+        .build();
   }
 }

api/src/main/java/io/grpc/Status.java

@@ -522,30 +522,30 @@ public boolean isOk() {
    * to recover this {@link Status} instance when the returned exception is in the causal chain.
    */
   public StatusRuntimeException asRuntimeException() {
-    return new StatusRuntimeException(this);
+    return new StatusRuntimeExceptionBuilder().setStatus(this).build();
   }
 
   /**
    * Same as {@link #asRuntimeException()} but includes the provided trailers in the returned
    * exception.
    */
   public StatusRuntimeException asRuntimeException(@Nullable Metadata trailers) {
-    return new StatusRuntimeException(this, trailers);
+    return new StatusRuntimeExceptionBuilder().setStatus(this).setTrailers(trailers).build();
   }
 
   /**
    * Convert this {@link Status} to an {@link Exception}. Use {@link #fromThrowable}
    * to recover this {@link Status} instance when the returned exception is in the causal chain.
    */
   public StatusException asException() {
-    return new StatusException(this);
+    return new StatusExceptionBuilder().setStatus(this).build();
   }
 
   /**
    * Same as {@link #asException()} but includes the provided trailers in the returned exception.
    */
   public StatusException asException(@Nullable Metadata trailers) {
-    return new StatusException(this, trailers);
+    return new StatusExceptionBuilder().setStatus(this).setTrailers(trailers).build();
   }
 
   /** A string representation of the status useful for debugging. */

api/src/main/java/io/grpc/StatusException.java

@@ -30,30 +30,16 @@ public class StatusException extends Exception {
   private final boolean fillInStackTrace;
 
   /**
-   * Constructs an exception with both a status.  See also {@link Status#asException()}.
+   * Constructs an exception with status, trailers, and whether to fill in the stack trace.
+   * See also {@link Status#asException()} and {@link Status#asException(Metadata)}.
    *
    * @since 1.0.0
    */
-  public StatusException(Status status) {
-    this(status, null);
-  }
-
-  /**
-   * Constructs an exception with both a status and trailers.  See also
-   * {@link Status#asException(Metadata)}.
-   *
-   * @since 1.0.0
-   */
-  public StatusException(Status status, @Nullable Metadata trailers) {
-    this(status, trailers, /*fillInStackTrace=*/ true);
-  }
-
   StatusException(Status status, @Nullable Metadata trailers, boolean fillInStackTrace) {
     super(Status.formatThrowableMessage(status), status.getCause());
     this.status = status;
     this.trailers = trailers;
     this.fillInStackTrace = fillInStackTrace;
-    fillInStackTrace();
   }
 
   @Override

api/src/main/java/io/grpc/StatusExceptionBuilder.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright 2021 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc;
+
+/**
+ * Builder for creating a {@link StatusException}.
+ *
+ * @since 1.62.0
+ */
+public class StatusExceptionBuilder {
+  private Status status;
+  private Metadata trailers = null;
+  private boolean fillInStackTrace = true;
+
+  /**
+   * Sets the status.
+   *
+   * @since 1.62.0
+   */
+  public StatusExceptionBuilder setStatus(final Status status) {
+    this.status = status;
+    return this;
+  }
+
+  /**
+   * Sets the trailers.
+   *
+   * @since 1.62.0
+   */
+  public StatusExceptionBuilder setTrailers(final Metadata trailers) {
+    this.trailers = trailers;
+    return this;
+  }
+
+  /**
+   * Sets whether to fill in the stack trace.
+   *
+   * @since 1.62.0
+   */
+  public StatusExceptionBuilder setFillInStackTrace(final boolean fillInStackTrace) {
+    this.fillInStackTrace = fillInStackTrace;
+    return this;
+  }
+
+  /**
+   * Builds the exception.
+   *
+   * @since 1.62.0
+   */
+  public StatusException build() {
+    final StatusException statusException = new StatusException(status, trailers, fillInStackTrace);
+    statusException.fillInStackTrace();
+    return statusException;
+  }
+}

api/src/main/java/io/grpc/StatusRuntimeException.java

@@ -32,30 +32,16 @@ public class StatusRuntimeException extends RuntimeException {
   private final boolean fillInStackTrace;
 
   /**
-   * Constructs the exception with both a status. See also {@link Status#asRuntimeException()}.
+   * Constructs an exception with status, trailers, and whether to fill in the stack trace.
+   * See also {@link Status#asRuntimeException()} and {@link Status#asRuntimeException(Metadata)}.
    *
    * @since 1.0.0
    */
-  public StatusRuntimeException(Status status) {
-    this(status, null);
-  }
-
-  /**
-   * Constructs the exception with both a status and trailers. See also {@link
-   * Status#asRuntimeException(Metadata)}.
-   *
-   * @since 1.0.0
-   */
-  public StatusRuntimeException(Status status, @Nullable Metadata trailers) {
-    this(status, trailers, /*fillInStackTrace=*/ true);
-  }
-
   StatusRuntimeException(Status status, @Nullable Metadata trailers, boolean fillInStackTrace) {
     super(Status.formatThrowableMessage(status), status.getCause());
     this.status = status;
     this.trailers = trailers;
     this.fillInStackTrace = fillInStackTrace;
-    fillInStackTrace();
   }
 
   @Override

api/src/main/java/io/grpc/StatusRuntimeExceptionBuilder.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright 2021 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc;
+
+/**
+ * Builder for creating a {@link StatusRuntimeException}.
+ *
+ * @since 1.62.0
+ */
+public class StatusRuntimeExceptionBuilder {
+  private Status status;
+  private Metadata trailers = null;
+  private boolean fillInStackTrace = true;
+
+  /**
+   * Sets the status.
+   *
+   * @since 1.62.0
+   */
+  public StatusRuntimeExceptionBuilder setStatus(final Status status) {
+    this.status = status;
+    return this;
+  }
+
+  /**
+   * Sets the trailers.
+   *
+   * @since 1.62.0
+   */
+  public StatusRuntimeExceptionBuilder setTrailers(final Metadata trailers) {
+    this.trailers = trailers;
+    return this;
+  }
+
+  /**
+   * Sets whether to fill in the stack trace.
+   *
+   * @since 1.62.0
+   */
+  public StatusRuntimeExceptionBuilder setFillInStackTrace(final boolean fillInStackTrace) {
+    this.fillInStackTrace = fillInStackTrace;
+    return this;
+  }
+
+  /**
+   * Builds the exception.
+   *
+   * @since 1.62.0
+   */
+  public StatusRuntimeException build() {
+    final StatusRuntimeException statusRuntimeException =
+        new StatusRuntimeException(status, trailers, fillInStackTrace);
+    statusRuntimeException.fillInStackTrace();
+    return statusRuntimeException;
+  }
+}

api/src/test/java/io/grpc/StatusExceptionTest.java

@@ -31,30 +31,33 @@ public class StatusExceptionTest {
   @Test
   public void internalCtorRemovesStack() {
     StackTraceElement[] trace =
-        new StatusException(Status.CANCELLED, null, false) {}.getStackTrace();
+        new StatusExceptionBuilder().setStatus(Status.CANCELLED).setTrailers(null)
+            .setFillInStackTrace(false).build().getStackTrace();
 
     assertThat(trace).isEmpty();
   }
 
   @Test
   public void normalCtorKeepsStack() {
     StackTraceElement[] trace =
-        new StatusException(Status.CANCELLED, null) {}.getStackTrace();
+        new StatusExceptionBuilder().setStatus(Status.CANCELLED).setTrailers(null)
+            .build().getStackTrace();
 
     assertThat(trace).isNotEmpty();
   }
 
   @Test
   public void extendPreservesStack() {
-    StackTraceElement[] trace = new StatusException(Status.CANCELLED) {}.getStackTrace();
+    StackTraceElement[] trace =
+        new StatusExceptionBuilder().setStatus(Status.CANCELLED).build().getStackTrace();
 
     assertThat(trace).isNotEmpty();
   }
 
   @Test
   public void extendAndOverridePreservesStack() {
     final StackTraceElement element = new StackTraceElement("a", "b", "c", 4);
-    StatusException exception = new StatusException(Status.CANCELLED, new Metadata()) {
+    StatusException exception = new StatusException(Status.CANCELLED, new Metadata(), true) {
 
       @Override
       public synchronized Throwable fillInStackTrace() {

api/src/test/java/io/grpc/StatusRuntimeExceptionTest.java

@@ -31,36 +31,40 @@ public class StatusRuntimeExceptionTest {
   @Test
   public void internalCtorRemovesStack() {
     StackTraceElement[] trace =
-        new StatusRuntimeException(Status.CANCELLED, null, false) {}.getStackTrace();
+        new StatusRuntimeExceptionBuilder().setStatus(Status.CANCELLED).setTrailers(null)
+            .setFillInStackTrace(false).build().getStackTrace();
 
     assertThat(trace).isEmpty();
   }
 
   @Test
   public void normalCtorKeepsStack() {
     StackTraceElement[] trace =
-        new StatusRuntimeException(Status.CANCELLED, null) {}.getStackTrace();
+        new StatusRuntimeExceptionBuilder().setStatus(Status.CANCELLED).setTrailers(null)
+            .build().getStackTrace();
 
     assertThat(trace).isNotEmpty();
   }
 
   @Test
   public void extendPreservesStack() {
-    StackTraceElement[] trace = new StatusRuntimeException(Status.CANCELLED) {}.getStackTrace();
+    StackTraceElement[] trace =
+        new StatusRuntimeExceptionBuilder().setStatus(Status.CANCELLED).build().getStackTrace();
 
     assertThat(trace).isNotEmpty();
   }
 
   @Test
   public void extendAndOverridePreservesStack() {
     final StackTraceElement element = new StackTraceElement("a", "b", "c", 4);
-    StatusRuntimeException error = new StatusRuntimeException(Status.CANCELLED, new Metadata()) {
-      @Override
-      public synchronized Throwable fillInStackTrace() {
-        setStackTrace(new StackTraceElement[]{element});
-        return this;
-      }
-    };
+    StatusRuntimeException error =
+        new StatusRuntimeException(Status.CANCELLED, new Metadata(), true) {
+          @Override
+          public synchronized Throwable fillInStackTrace() {
+            setStackTrace(new StackTraceElement[]{element});
+            return this;
+          }
+        };
     assertThat(error.getStackTrace()).asList().containsExactly(element);
   }
 }

core/src/test/java/io/grpc/internal/AbstractServerStreamTest.java

@@ -34,7 +34,7 @@
 import io.grpc.InternalStatus;
 import io.grpc.Metadata;
 import io.grpc.Status;
-import io.grpc.StatusRuntimeException;
+import io.grpc.StatusRuntimeExceptionBuilder;
 import io.grpc.internal.AbstractServerStream.TransportState;
 import io.grpc.internal.MessageFramerTest.ByteWritableBuffer;
 import java.io.ByteArrayInputStream;
@@ -127,8 +127,8 @@ public void messagesAvailable(StreamListener.MessageProducer producer) {
       @Override
       public void halfClosed() {
         if (streamListenerMessageQueue.isEmpty()) {
-          throw new StatusRuntimeException(Status.INTERNAL.withDescription(
-              "Half close without request"));
+          throw new StatusRuntimeExceptionBuilder().setStatus(Status.INTERNAL.withDescription(
+              "Half close without request")).build();
         }
       }
 

core/src/test/java/io/grpc/internal/DelayedClientCallTest.java

@@ -33,7 +33,7 @@
 import io.grpc.ForwardingTestUtil;
 import io.grpc.Metadata;
 import io.grpc.Status;
-import io.grpc.StatusException;
+import io.grpc.StatusExceptionBuilder;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.Arrays;
@@ -168,7 +168,8 @@ public void cancelThenSetCall() {
         callExecutor, fakeClock.getScheduledExecutorService(), null);
     delayedClientCall.start(listener, new Metadata());
     delayedClientCall.request(1);
-    delayedClientCall.cancel("cancel", new StatusException(Status.CANCELLED));
+    delayedClientCall.cancel("cancel",
+        new StatusExceptionBuilder().setStatus(Status.CANCELLED).build());
     Runnable r = delayedClientCall.setCall(mockRealCall);
     assertThat(r).isNull();
     verify(mockRealCall, never()).start(any(Listener.class), any(Metadata.class));
@@ -187,7 +188,8 @@ public void setCallThenCancel() {
     Runnable r = delayedClientCall.setCall(mockRealCall);
     assertThat(r).isNotNull();
     r.run();
-    delayedClientCall.cancel("cancel", new StatusException(Status.CANCELLED));
+    delayedClientCall.cancel("cancel",
+        new StatusExceptionBuilder().setStatus(Status.CANCELLED).build());
     @SuppressWarnings("unchecked")
     ArgumentCaptor<Listener<Integer>> listenerCaptor = ArgumentCaptor.forClass(Listener.class);
     verify(mockRealCall).start(listenerCaptor.capture(), any(Metadata.class));