Skip to content

Commit acbbf86

Browse files
api: Fix name resolver bridge listener handling for address resolution errors (#12441)
Fixes #12444 This PR addresses a bug in the `NameResolver.Listener` to `NameResolver.Listener2` bridge affecting custom NameResolver implementations using Listener. The bridge in `NameResolver.start(Listener)` at https://github.com/grpc/grpc-java/blob/master/api/src/main/java/io/grpc/NameResolver.java#L100 unconditionally calls `getValue()` on the `StatusOr`, throwing `java.lang.IllegalStateException: No value present.` when the result contains an error. This was identified when upgrading from gRPC `v1.63.3` to `v1.75.0`. The bug occurs due to `DnsNameResolver`'s error handling changes between versions: - `v1.63.3`: Errors reported via `Listener.onError()` (https://github.com/grpc/grpc-java/blob/v1.63.x/core/src/main/java/io/grpc/internal/DnsNameResolver.java#L319) - `v1.75.0`: Errors passed via `Listener2.onResult2()` with a ResolutionResult containing either addresses OR an error (https://github.com/grpc/grpc-java/blob/master/core/src/main/java/io/grpc/internal/DnsNameResolver.java#L322) This PR updates the bridge to check whether `ResolutionResult` contains addresses or an error. It passes the error via `onError` and addresses via `onAddresses`. **Reproducing the Issue** The `startOnOldListener_resolverReportsError` test reproduces a similar issue. It creates a custom `NameResolver` that reports errors through the `ResolutionResult` like the `DNSNameResolver` in `v1.75.0`, passes an old Listener to `resolver.start`, which triggers the bridge code path. Without the fix, the bridge calls `getValue()` on the error containing `StatusOr`, throwing `IllegalStateException: No value present`. With the fix, the bridge checks `hasValue()` first and correctly routes to `listener.onError()` when appropriate. This ensures backward compatibility for `Listener` implementations when resolvers report errors via `ResolutionResult`.
1 parent 9b424b3 commit acbbf86

File tree

2 files changed

+58
-2
lines changed

2 files changed

+58
-2
lines changed

api/src/main/java/io/grpc/NameResolver.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,14 @@ public void onError(Status error) {
9797

9898
@Override
9999
public void onResult(ResolutionResult resolutionResult) {
100-
listener.onAddresses(resolutionResult.getAddressesOrError().getValue(),
101-
resolutionResult.getAttributes());
100+
StatusOr<List<EquivalentAddressGroup>> addressesOrError =
101+
resolutionResult.getAddressesOrError();
102+
if (addressesOrError.hasValue()) {
103+
listener.onAddresses(addressesOrError.getValue(),
104+
resolutionResult.getAttributes());
105+
} else {
106+
listener.onError(addressesOrError.getStatus());
107+
}
102108
}
103109
});
104110
}

api/src/test/java/io/grpc/NameResolverTest.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,56 @@ public void resolutionResult_hashCode() {
192192
Objects.hashCode(StatusOr.fromValue(ADDRESSES), ATTRIBUTES, CONFIG));
193193
}
194194

195+
@Test
196+
public void startOnOldListener_resolverReportsError() {
197+
final boolean[] onErrorCalled = new boolean[1];
198+
final Status[] receivedError = new Status[1];
199+
200+
NameResolver resolver = new NameResolver() {
201+
@Override
202+
public String getServiceAuthority() {
203+
return "example.com";
204+
}
205+
206+
@Override
207+
public void shutdown() {
208+
}
209+
210+
@Override
211+
public void start(Listener2 listener2) {
212+
ResolutionResult errorResult = ResolutionResult.newBuilder()
213+
.setAddressesOrError(StatusOr.fromStatus(
214+
Status.UNAVAILABLE
215+
.withDescription("DNS resolution failed with UNAVAILABLE")))
216+
.build();
217+
218+
listener2.onResult(errorResult);
219+
}
220+
};
221+
222+
NameResolver.Listener listener = new NameResolver.Listener() {
223+
@Override
224+
public void onAddresses(
225+
List<EquivalentAddressGroup> servers,
226+
Attributes attributes) {
227+
throw new AssertionError("Called onAddresses on error");
228+
}
229+
230+
@Override
231+
public void onError(Status error) {
232+
onErrorCalled[0] = true;
233+
receivedError[0] = error;
234+
}
235+
};
236+
237+
resolver.start(listener);
238+
239+
assertThat(onErrorCalled[0]).isTrue();
240+
assertThat(receivedError[0].getCode()).isEqualTo(Status.Code.UNAVAILABLE);
241+
assertThat(receivedError[0].getDescription()).isEqualTo(
242+
"DNS resolution failed with UNAVAILABLE");
243+
}
244+
195245
private static class FakeSocketAddress extends SocketAddress {
196246
final String name;
197247

0 commit comments

Comments
 (0)