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
grpc · Sep 12, 2022 · 9853a0c · 9853a0c
1 parent bcf5cde
commit 9853a0c
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 2 deletions.
diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
@@ -1199,7 +1199,8 @@ public void start(Listener<RespT> observer, Metadata headers) {
       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
@@ -121,6 +121,31 @@ public void selectionErrorPropagatedToListener() {
     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 Result selectConfig(PickSubchannelArgs args) {
     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);