From eee7030a287d2be127a94d09a970cf3a8a7834ba Mon Sep 17 00:00:00 2001
From: Michael Lumish <mlumish@google.com>
Date: Wed, 6 Nov 2024 16:24:39 -0800
Subject: [PATCH] grpc-js-xds: Fix a couple of bugs with the config tears
 change

---
 .../grpc-js-xds/src/xds-dependency-manager.ts     | 15 ++++++++++++---
 packages/grpc-js/src/backoff-timeout.ts           |  2 +-
 packages/grpc-js/src/load-balancer-pick-first.ts  |  7 +++++++
 packages/grpc-js/test/test-pick-first.ts          | 13 +++++++++++++
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/packages/grpc-js-xds/src/xds-dependency-manager.ts b/packages/grpc-js-xds/src/xds-dependency-manager.ts
index e455929f8..aee3186dd 100644
--- a/packages/grpc-js-xds/src/xds-dependency-manager.ts
+++ b/packages/grpc-js-xds/src/xds-dependency-manager.ts
@@ -653,8 +653,9 @@ export class XdsDependencyManager {
           this.subscribedClusters[clusterName] -= 1;
           if (this.subscribedClusters[clusterName] <= 0) {
             delete this.subscribedClusters[clusterName];
-            this.pruneOrphanClusters();
-            this.maybeSendUpdate();
+            if (this.pruneOrphanClusters()) {
+              this.maybeSendUpdate();
+            }
           }
         }
       }
@@ -682,7 +683,12 @@ export class XdsDependencyManager {
     delete this.clusterForest[clusterName];
   }
 
-  private pruneOrphanClusters() {
+  /**
+   * Prune any clusters that are not descendents of any root clusters,
+   * including subscribed clusters.
+   * @returns True if any clusters were pruned, false otherwise
+   */
+  private pruneOrphanClusters(): boolean {
     const toCheck = [...this.clusterRoots, ...Object.keys(this.subscribedClusters)];
     const visited = new Set<string>();
     while(toCheck.length > 0) {
@@ -695,11 +701,14 @@ export class XdsDependencyManager {
       }
       visited.add(next);
     }
+    let removedAnyClusters = false;
     for (const clusterName of Object.keys(this.clusterForest)) {
       if (!visited.has(clusterName)) {
+        removedAnyClusters = true;
         this.removeCluster(clusterName);
       }
     }
+    return removedAnyClusters;
   }
 
   private handleRouteConfig(routeConfig: RouteConfiguration__Output) {
diff --git a/packages/grpc-js/src/backoff-timeout.ts b/packages/grpc-js/src/backoff-timeout.ts
index 10d347e79..ae5a62cab 100644
--- a/packages/grpc-js/src/backoff-timeout.ts
+++ b/packages/grpc-js/src/backoff-timeout.ts
@@ -107,7 +107,7 @@ export class BackoffTimeout {
   private runTimer(delay: number) {
     this.endTime = this.startTime;
     this.endTime.setMilliseconds(
-      this.endTime.getMilliseconds() + this.nextDelay
+      this.endTime.getMilliseconds() + delay
     );
     clearTimeout(this.timerId);
     this.timerId = setTimeout(() => {
diff --git a/packages/grpc-js/src/load-balancer-pick-first.ts b/packages/grpc-js/src/load-balancer-pick-first.ts
index 475d796ef..7ecc87c4b 100644
--- a/packages/grpc-js/src/load-balancer-pick-first.ts
+++ b/packages/grpc-js/src/load-balancer-pick-first.ts
@@ -275,6 +275,13 @@ export class PickFirstLoadBalancer implements LoadBalancer {
           new PickFirstPicker(this.currentPick)
         );
       }
+    } else if (this.latestAddressList?.length === 0) {
+      this.updateState(
+        ConnectivityState.TRANSIENT_FAILURE,
+        new UnavailablePicker({
+          details: `No connection established. Last error: ${this.lastError}`,
+        })
+      );
     } else if (this.children.length === 0) {
       this.updateState(ConnectivityState.IDLE, new QueuePicker(this));
     } else {
diff --git a/packages/grpc-js/test/test-pick-first.ts b/packages/grpc-js/test/test-pick-first.ts
index 36b06d446..c79d3e32a 100644
--- a/packages/grpc-js/test/test-pick-first.ts
+++ b/packages/grpc-js/test/test-pick-first.ts
@@ -690,6 +690,19 @@ describe('pick_first load balancing policy', () => {
       });
     });
   });
+  it('Should report TRANSIENT_FAILURE with no addresses', done => {
+    const channelControlHelper = createChildChannelControlHelper(
+      baseChannelControlHelper,
+      {
+        updateState: updateStateCallBackForExpectedStateSequence(
+          [ConnectivityState.TRANSIENT_FAILURE],
+          done
+        ),
+      }
+    );
+    const pickFirst = new PickFirstLoadBalancer(channelControlHelper, creds, {});
+    pickFirst.updateAddressList([], config);
+  });
   describe('Address list randomization', () => {
     const shuffleConfig = new PickFirstLoadBalancingConfig(true);
     it('Should pick different subchannels after multiple updates', done => {