addressed review comments

DNVindhya · Oct 8, 2024 · 3916c48 · 3916c48
1 parent fae5955
commit 3916c48
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 11 deletions.
diff --git a/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java b/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java
@@ -408,7 +408,7 @@ private void handleRpcStreamClosed(Status status) {
               status.getCode(), status.getDescription(), status.getCause());
         } else {
           logger.log(XdsLogLevel.DEBUG,
-              "ADS stream closed by server after responses received status {0}: {1}. Cause: {2}");
+              "ADS stream closed by server after responses received.");
         }
       } else {
         // If the ADS stream is closed without ever having received a response from the server, then
@@ -417,14 +417,11 @@ private void handleRpcStreamClosed(Status status) {
           newStatus = Status.UNAVAILABLE.withDescription(
               "ADS stream failed, because connection was closed before receiving a response.");
         }
-      }
-
-      if (!newStatus.isOk() && !(newStatus.getDescription() != null
-          && newStatus.getDescription().contains(CLOSED_BY_SERVER_AFTER_RECEIVING_A_RESPONSE))) {
         logger.log(
             XdsLogLevel.ERROR, "ADS stream failed with status {0}: {1}. Cause: {2}",
             newStatus.getCode(), newStatus.getDescription(), newStatus.getCause());
       }
+
       closed = true;
       xdsResponseHandler.handleStreamClosed(newStatus);
       cleanUp();

diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java
@@ -3342,15 +3342,30 @@ public void streamClosedWithNoResponse() {
     verify(ldsResourceWatcher, Mockito.timeout(1000).times(1))
         .onError(errorCaptor.capture());
     verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE,
-        "ADS stream failed due to connectivity error");
+        "ADS stream failed, because connection was closed before receiving a response.");
     verify(rdsResourceWatcher).onError(errorCaptor.capture());
     verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE,
-        "ADS stream failed due to connectivity error");
-    ScheduledTask retryTask =
-        Iterables.getOnlyElement(fakeClock.getPendingTasks(RPC_RETRY_TASK_FILTER));
-    assertThat(retryTask.getDelay(TimeUnit.NANOSECONDS)).isEqualTo(10L);
+        "ADS stream failed, because connection was closed before receiving a response.");
+  }
 
-    verifyNoMoreInteractions(ldsResourceWatcher, rdsResourceWatcher);
+  @Test
+  public void streamClosedAfterSendingResponses() {
+    xdsClient.watchXdsResource(XdsListenerResource.getInstance(),LDS_RESOURCE, ldsResourceWatcher);
+    xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(),RDS_RESOURCE,
+        rdsResourceWatcher);
+    DiscoveryRpcCall call = resourceDiscoveryCalls.poll();
+    ScheduledTask ldsResourceTimeout =
+        Iterables.getOnlyElement(fakeClock.getPendingTasks(LDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER));
+    ScheduledTask rdsResourceTimeout =
+        Iterables.getOnlyElement(fakeClock.getPendingTasks(RDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER));
+    call.sendResponse(LDS, testListenerRds, VERSION_1, "0000");
+    assertThat(ldsResourceTimeout.isCancelled()).isTrue();
+    call.sendResponse(RDS, testRouteConfig, VERSION_1, "0000");
+    assertThat(rdsResourceTimeout.isCancelled()).isTrue();
+    // Management server closes the RPC stream after sending responses.
+    call.sendCompleted();
+    verify(ldsResourceWatcher, never()).onError(errorCaptor.capture());
+    verify(rdsResourceWatcher, never()).onError(errorCaptor.capture());
   }
 
   @Test