Don't throw on empty responses #28
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yim-lee
reviewed
Feb 19, 2024
| /// - name: The name to resolve. | ||
| /// | ||
| /// - Returns: ``PTRRecord`` for the given name. | ||
| /// - Returns: ``PTRRecord`` for the given name, `nil` if no record was found. |
There was a problem hiding this comment.
I guess in case of PTRRecord, names is empty if no records found?
There was a problem hiding this comment.
Oh yes, good catch, the doc is wrong here.
Motivation: The AsyncDNSResolver currently throws an error when there are no records associated with a name. However, most query responses are arrays or contain arrays. As such it's unclear to callers whether they should be catching an error of a particular type or checking for the presence of values some combination of the two. Modifications: - Modify the `DNSResolver` API to return `nil` where only a single value is expected (e.g. CNAME). - Update documentation on `DNSResolver` to clarify the expected return values when no values are resolved. - Update the c-ares backend to check for no data when parsing and return empty responses - Update the DNSSD backend to pass in `nil` data to the parser when the status is timeout - Remove the `noData` error code as it shouldn't be reachable anymore Result: Queries with no response either return empty or nil
|
@swift-server-bot test this please |
yim-lee
approved these changes
Mar 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation:
The AsyncDNSResolver currently throws an error when there are no records associated with a name. However, most query responses are arrays or contain arrays. As such it's unclear to callers whether they should be catching an error of a particular type or checking for the presence of values some combination of the two.
Modifications:
DNSResolverAPI to returnnilwhere only a single value is expected (e.g. CNAME).DNSResolverto clarify the expected return values when no values are resolved.nildata to the parser when the status is timeoutnoDataerror code as it shouldn't be reachable anymoreResult:
Queries with no response either return empty or nil