PickFirstLoadBalancer.updateAddressList silently ignores resolver errors while in TRANSIENT_FAILURE with existing subchannels #3116

Problem description

Location (grpc/grpc-node @ default branch 6741e13): packages/grpc-js/src/load-balancer-pick-first.ts:520

On latest HEAD, the resolver-error branch of updateAddressList only reports a new picker when there are no children at all:

// In load-balancer-pick-first.ts, updateAddressList:
if (!maybeEndpointList.ok) {
  if (this.children.length === 0 && this.currentPick === null) {
    this.channelControlHelper.updateState(
      ConnectivityState.TRANSIENT_FAILURE,
      new UnavailablePicker(maybeEndpointList.error),
      maybeEndpointList.error.details
    );
  }
  return true;
}

Problem: When pick_first is in sticky TRANSIENT_FAILURE (all subchannels have failed to connect), the failed subchannel children still exist in this.children, so this.children.length === 0 is false. The resolver error is therefore dropped and updateState is never called, leaving the picker reflecting the old connection error instead of the new resolver error. The condition should additionally allow a picker update when the balancer is already in TRANSIENT_FAILURE; it should ignore resolver errors only when the balancer is not in TRANSIENT_FAILURE and already has addresses from a previous good update.

Reproduction steps

1. Create a PickFirstLoadBalancer and send a valid address update with one endpoint.
2. Transition the subchannel to TRANSIENT_FAILURE so the balancer enters sticky TF mode.
3. Send a resolver error via updateAddressList(statusOrFromError({code: Status.UNAVAILABLE, details: 'Resolution failed'}), ...).
4. Observe that updateState on the ChannelControlHelper is not called — the picker still reflects the old connection error.

Test added on HEAD (packages/grpc-js/test/test-pick-first-resolver-error-in-tf.ts):

it('Should update the picker when a resolver error is received while in TRANSIENT_FAILURE', done => {
  let updateStateCallCount = 0;
  const channelControlHelper = createChildChannelControlHelper(
    baseChannelControlHelper,
    {
      updateState: (connectivityState: ConnectivityState, picker: Picker, errorMessage: string | null) => {
        updateStateCallCount++;
      },
    }
  );
  const pickFirst = new PickFirstLoadBalancer(channelControlHelper);
  pickFirst.updateAddressList(
    statusOrFromValue([{ addresses: [{ host: 'localhost', port: 1 }] }]),
    config,
    {},
    ''
  );
  process.nextTick(() => {
    subchannels[0].transitionToState(ConnectivityState.TRANSIENT_FAILURE);
    const countAfterTF = updateStateCallCount;
    assert.strictEqual(countAfterTF, 2, 'Expected 2 updateState calls before resolver error');

    pickFirst.updateAddressList(
      statusOrFromError({ code: Status.UNAVAILABLE, details: 'Resolution failed: test error' }),
      config,
      {},
      ''
    );

    assert.ok(
      updateStateCallCount > countAfterTF,
      `Resolver error while in TRANSIENT_FAILURE should trigger a picker update. ` +
      `Expected updateState to be called more than ${countAfterTF} times, but was called ${updateStateCallCount} times.`
    );
    done();
  });
});

Failing output on HEAD + test patch:

1) pick_first resolver error handling Should update the picker when a resolver error is received while in TRANSIENT_FAILURE:
    Uncaught AssertionError [ERR_ASSERTION]: Resolver error while in TRANSIENT_FAILURE should trigger a picker update. Expected updateState to be called more than 2 times, but was called 2 times.
    + expected - actual

The control run without the test patch exits 0 (same suite).

Environment

• Node version: v22.23.4
• Python (toolchain line): Python 3.9.2
• Commit: 6741e13f210f119d4d3da37691dd01055541663b (2026-10-20), default branch
• Package: grpc-js (packages/grpc-js)

Additional context

Already fixed in grpc/grpc-go

The analogous bug was addressed in grpc/grpc-go PR #7498 ("pickfirst: New pick first policy for dualstack"), merged 2024-10-10T04:03:47Z (merge commit 00b9e140ce7). The PR is a larger dualstack feature; the relevant part is the new pickfirstleaf policy's resolverErrorLocked, which only ignores resolver errors when the balancer is not in TransientFailure and already has addresses, and otherwise updates the picker with the resolver error.

How grpc-go fixed it (from balancer/pickfirst/pickfirstleaf/pickfirstleaf.go):

func (b *pickfirstBalancer) resolverErrorLocked(err error) {
	...
	// The picker will not change since the balancer does not currently
	// report an error. If the balancer hasn't received a single good resolver
	// update yet, transition to TRANSIENT_FAILURE.
	if b.state != connectivity.TransientFailure && b.addressList.size() > 0 {
		if b.logger.V(2) {
			b.logger.Infof("Ignoring resolver error because balancer is using a previous good update.")
		}
		return
	}

	b.cc.UpdateState(balancer.State{
		ConnectivityState: connectivity.TransientFailure,
		Picker:            &picker{err: fmt.Errorf("name resolver error: %v", err)},
	})
}

This policy has since become grpc-go's default pick_first, and the same check is still on grpc-go master today (balancer/pickfirst/pickfirst.go:197).

Suggested fix

Also update the picker when the balancer is already in TRANSIENT_FAILURE, matching grpc-go:

diff --git a/packages/grpc-js/src/load-balancer-pick-first.ts b/packages/grpc-js/src/load-balancer-pick-first.ts
--- a/packages/grpc-js/src/load-balancer-pick-first.ts
+++ b/packages/grpc-js/src/load-balancer-pick-first.ts
@@ -517,7 +517,14 @@ export class PickFirstLoadBalancer implements LoadBalancer {
       return false;
     }
     if (!maybeEndpointList.ok) {
-      if (this.children.length === 0 && this.currentPick === null) {
+      /* If the balancer is already in TRANSIENT_FAILURE, update the picker
+       * with the new resolver error. Otherwise, only update the picker if
+       * there are no existing addresses (i.e. the balancer has not yet
+       * received a good resolver update). */
+      if (
+        this.currentState === ConnectivityState.TRANSIENT_FAILURE ||
+        (this.children.length === 0 && this.currentPick === null)
+      ) {
         this.channelControlHelper.updateState(
           ConnectivityState.TRANSIENT_FAILURE,
           new UnavailablePicker(maybeEndpointList.error),

Verified on 6741e13 (Node v22.23.4): the test above fails without this patch and passes with it; no other test results change.