apple / apple/swift-async-dns-resolver
Incorrect error handling for NXDOMAIN
- Dominant language
- Swift
- Stars
- 160
- Forks
- 33
- PR merge metrics
- No merged PRs in 30d
Description
When receiving the response NXDOMAIN, AsyncDNSResolver throws `.internalError`, requiring callers to code like this:
```
var records: [SRVRecord]
do {
records = try await Self.resolver.querySRV(name: srvName)
} catch let error as AsyncDNSResolver.Error where error.code == .internalError {
// Catch NXDOMAIN, which is erroneously reported as `.internalError`.
// NXDOMAIN and `results==[]` (commonly called "NODATA") both indicate affirmative nonexistence.
// Hopefully other common errors aren't `.internalError` as that may cause weird behavior here.
records = []
} catch let error {
// Catch timeouts and other transient network errors.
throw error
}
```
Adding an exception for `.nxDomain` might be a tempting solution, but it would break existing workarounds like the above. But it can't be conflated into `.internalError`; doing so as it does now is clearly a bug.
I suggest updating queryXXX calls to return `[]` when the response is coded NXDOMAIN. This makes sense because the distinction between the DNS entry existing only by different types (commonly "NODATA") vs not existing at all (NXDOMAIN) isn't a useful distinction, so they can be treated the same. Returning [] simplifies clients by not needing the first catch case shown above, and sidesteps potential compatibility issues for existing callers.
Contributor guide
Research direction
Start by tracing the queryXXX calls and how c-ares NXDOMAIN responses become AsyncDNSResolver.Error values. Check the existing handling for empty results and .internalError, then verify that NXDOMAIN is treated consistently with NODATA without breaking existing callers. Done means the calls return [] for NXDOMAIN and the behavior is covered by the relevant tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- swift
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100