From 9853a0c4639a13a523530f99730cc93d113151a8 Mon Sep 17 00:00:00 2001
From: Terry Wilson <terrymwilson@gmail.com>
Date: Mon, 12 Sep 2022 13:17:16 -0700
Subject: [PATCH] core: Don't delegate inappropriate ConfigSelector errors
 (#9536)

In case a control plane returns an "inappropriate" response code, it is converted to INTERNAL to highlight the bug in the control plane.

https://github.com/grpc/proposal/blob/master/A54-restrict-control-plane-status-codes.md
---
 .../io/grpc/internal/ManagedChannelImpl.java  |  3 +-
 .../ConfigSelectingClientCallTest.java        | 28 ++++++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
index 6e80eb3925..aefe76dcb5 100644
--- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
+++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
@@ -1199,7 +1199,8 @@ final class ManagedChannelImpl extends ManagedChannel implements
       InternalConfigSelector.Result result = configSelector.selectConfig(args);
       Status status = result.getStatus();
       if (!status.isOk()) {
-        executeCloseObserverInContext(observer, status);
+        executeCloseObserverInContext(observer,
+            GrpcUtil.replaceInappropriateControlPlaneStatus(status));
         delegate = (ClientCall<ReqT, RespT>) NOOP_CALL;
         return;
       }
diff --git a/core/src/test/java/io/grpc/internal/ConfigSelectingClientCallTest.java b/core/src/test/java/io/grpc/internal/ConfigSelectingClientCallTest.java
index 9b3f8ad3b2..b5b6fef3c9 100644
--- a/core/src/test/java/io/grpc/internal/ConfigSelectingClientCallTest.java
+++ b/core/src/test/java/io/grpc/internal/ConfigSelectingClientCallTest.java
@@ -121,6 +121,31 @@ public class ConfigSelectingClientCallTest {
     InternalConfigSelector configSelector = new InternalConfigSelector() {
       @Override
       public Result selectConfig(PickSubchannelArgs args) {
+        return Result.forError(Status.DEADLINE_EXCEEDED);
+      }
+    };
+
+    ClientCall<Void, Void> configSelectingClientCall = new ConfigSelectingClientCall<>(
+        configSelector,
+        channel,
+        MoreExecutors.directExecutor(),
+        method,
+        CallOptions.DEFAULT);
+    configSelectingClientCall.start(callListener, new Metadata());
+    ArgumentCaptor<Status> statusCaptor = ArgumentCaptor.forClass(null);
+    verify(callListener).onClose(statusCaptor.capture(), any(Metadata.class));
+    assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.DEADLINE_EXCEEDED);
+
+    // The call should not delegate to null and fail methods with NPE.
+    configSelectingClientCall.request(1);
+  }
+
+  @Test
+  public void selectionErrorPropagatedToListener_inappropriateStatus() {
+    InternalConfigSelector configSelector = new InternalConfigSelector() {
+      @Override
+      public Result selectConfig(PickSubchannelArgs args) {
+        // This status code is considered inappropriate to propagate from the control plane...
         return Result.forError(Status.FAILED_PRECONDITION);
       }
     };
@@ -134,7 +159,8 @@ public class ConfigSelectingClientCallTest {
     configSelectingClientCall.start(callListener, new Metadata());
     ArgumentCaptor<Status> statusCaptor = ArgumentCaptor.forClass(null);
     verify(callListener).onClose(statusCaptor.capture(), any(Metadata.class));
-    assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.FAILED_PRECONDITION);
+    // ... so it should be represented as an internal error to highlight the control plane bug.
+    assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.INTERNAL);
 
     // The call should not delegate to null and fail methods with NPE.
     configSelectingClientCall.request(1);