Update the name resolver interface #2603
Open
+637
−445
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds
NIOStreamingResolver
as a replacement forResolver
. This resolver is phrased in terms of resolving names to socket addresses, whithout any implication that DNS will be used. It also removes constraints on how results should be delivered, allowing an implementation to stream results as they arrive, without having to wait for a complete response.Motivation:
Hello, I'm currently in need of a custom resolver for my HTTP client. But before opening a PR to add a
Resolver
to AsyncHTTPClient, I'd like to address the issues exposed by @Lukasa in #1487:So I've implemented the proposed interface.
Modifications:
This PR adds the proposed
NIOStreamingResolver
protocol and its associatedNIONameResolutionSession
.HappyEyeballsConnector
andGetaddrinfoResolver
are updated to uses this new interface.The
Resolver
interface is deprecated. TheNIOResolverToStreamingResolver
adapter is used to maintain compatibility with the existing implementations.Name resolution tests are left mostly unchanged for the moment.
Result:
Resolver
protocol will get a deprecation warning.Resolver
protocol may observe a minor change: if the connection to the target socket address succeeds or fails instantly (as sometimes happens when usingEmbeddedChannel
instead of the real network), thecancelQueries
method may be called even though both A and AAAA queries have already completed. This is because the resolver delivering results and the resolver completing are now two separate events, andHappyEyeballsConnector
may terminate and clean up its resources between the two events. This can be seen intestIPv6OnlyResolution
andtestAQueryReturningFirstThenAAAAReturns
. However it is unlikely to have an impact in the real world as thecancelQueries
method is probably always ignored.Unresolved:
NIONameResolutionSession.Hints
type as neitherHappyEyeballsConnector
norGetaddrinfoResolver
would currently use it. Do we want it now? Or should we wait until we have a real use for it?I haven't added new tests yet, nor modified existing ones (beyond what was strictly necessary to make them pass). I'd like to make sure every one is happy with the new design first.HappyEyeballsConnector
. But I think we can address that later.NIOStreamingResolver
Sendable?