Note: The failing test log shown here in github Actions is not failing in local even after clean run/build.

The purpose of gRFC A88 is to introduce onAmbientError to notify watchers of transient errors without them having to drop their cached data. If the underlying ControlPlaneClient hides the error and reports Status.OK, then XdsClientImpl would never know a transient failure occurred and would therefore never be able to call onAmbientError on the watchers. This would completely break this new mechanism.

Note: I added a TODO in connect_then_mainServerDown_fallbackServerUp in XdsClientFallbackTest. Unable to write this test. Any suggestions on this would be appreciated.

if (subscriber.hasResult() || !authoritiesForClosedCpc.contains(subscriber.authority)) {
    subscriber.onError(status, null);
    continue;
}

This above code block was the initial fix for the failing fallback test that was mentioned above.
But it introduced the flakiness in testTwoBadUrl() in XdsClientFallbackTest.java. The test became flaky because sometimes it might see the premature garbageUri1 error, and sometimes it might run long enough to also see the final garbageUri2 error, and ldsUpdateCaptor.getValue() would happen to hold the correct final value. The behavior was no longer deterministic.

The solution is to restructure the logic in the for loop to handle different subscriber states more clearly and deterministically. The updated logic is more clearer:

1. If a subscriber already has data, it should get an ambient error.
2. If a subscriber does not have data, the client should attempt to fall back.
3. Only if the subscriber has no data AND all fallback attempts have failed should a definitive error be reported.