Skip to content

feat: add proxy DNS resolver and reorder defaults#32

Merged
RishabhS7 merged 1 commit into
masterfrom
feat/proxy-dns-resolver
May 11, 2026
Merged

feat: add proxy DNS resolver and reorder defaults#32
RishabhS7 merged 1 commit into
masterfrom
feat/proxy-dns-resolver

Conversation

@rongquan1
Copy link
Copy Markdown

@rongquan1 rongquan1 commented May 11, 2026

Summary by CodeRabbit

  • New Features

    • Added a new DNS-over-HTTPS proxy resolver to the default DNS resolver chain, expanding DNS query options.
  • Tests

    • Added test coverage for the new proxy resolver, including validation and error handling scenarios.

Review Change Stack

Adds proxyDnsResolver targeting https://dns.opencerts.io/resolve, a
server-side DoH proxy that forwards to Google/Cloudflare upstream.
Useful as a fallback in regions where direct Google/Cloudflare DoH
access is blocked.

Moves aliDnsResolver to the end of the fallback chain because it
truncates large TXT responses (verified live against
example.tradetrust.io: Ali returned 11 of 58 records while Google,
Cloudflare, and the proxy each returned all 58).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

📝 Walkthrough

Walkthrough

A new DNS-over-HTTPS proxy resolver (proxyDnsResolver) is added to query TXT records via dns.opencerts.io. The resolver is exported through the module barrel, integrated into the library's default resolver chain, and validated with comprehensive test coverage including error scenarios.

Changes

Proxy DNS Resolver Addition

Layer / File(s) Summary
Proxy DNS Resolver Implementation
src/util/dns-resolvers/proxy-dns-resolver.ts
New proxyDnsResolver function accepts a domain, validates it is provided, constructs a query URL to https://dns.opencerts.io/resolve with TXT record type, performs a fetch, rejects non-2xx responses by HTTP status, and parses the JSON response with error handling before returning the result as IDNSQueryResponse.
Resolver Module Export
src/util/dns-resolvers/index.ts
Barrel re-export added for the new proxy-dns-resolver module.
Default Resolver Integration
src/index.ts
Import added for proxyDnsResolver and it is appended to the exported defaultDnsResolvers array, making it available as a default resolver option for DNS lookups.
Test Coverage
src/util/dns-resolvers/dns-resolvers.test.ts
Test suite added that mocks the proxy endpoint, validates request query parameters (name and type=TXT), verifies error handling for non-2xx HTTP responses and empty domain input, and confirms resolver output conforms to expected shape.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • TradeTrust/dnsprove#30: Both PRs modify the DNS resolver surface by adding new resolver modules and updating exports in the dns-resolvers package.

Suggested reviewers

  • Isaac-kps
  • isaackps

Poem

🐰 A proxy resolver hops into view,
DNS queries now route through dns.opencerts too,
With validation and errors all caught with care,
The default resolvers now have more flair!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately reflects the main changes: adding a new proxy DNS resolver and updating the default resolvers.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/proxy-dns-resolver

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/index.ts (1)

27-32: ⚡ Quick win

Consider a contract test for default resolver order.

Since this PR intentionally changes fallback order, add one focused test that asserts defaultDnsResolvers sequence to prevent accidental reordering in future refactors.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/index.ts` around lines 27 - 32, Add a focused contract test that asserts
the exact order of the exported defaultDnsResolvers array to prevent accidental
reorderings: write a unit test that imports defaultDnsResolvers and verifies its
sequence equals [googleDnsResolver, cloudflareDnsResolver, proxyDnsResolver,
aliDnsResolver] (or compares names/identifiers if direct object equality is
brittle); name the test to indicate it guards resolver fallback order and keep
it small and isolated so future refactors will fail if the order changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/util/dns-resolvers/proxy-dns-resolver.ts`:
- Around line 7-9: The current validation only checks falsy values so
whitespace-only strings pass; trim the incoming domain string before validation
(e.g., let trimmed = domain.trim() or assign back to domain) and then throw the
Error if the trimmed value is empty. Update the validation in
proxy-dns-resolver.ts (the domain parameter check) to use the trimmed value so
inputs like "   " are rejected before sending upstream.
- Around line 14-18: In queryDns (proxy-dns-resolver.ts) add a request timeout
and stricter domain validation: before building the URL, validate the domain by
trimming (e.g., if (!domain || domain.trim().length === 0) throw new Error(...))
so whitespace-only strings are rejected; when calling fetch(url) use an
AbortSignal with AbortSignal.timeout(TIMEOUT_MS) (Node 18+), pass the signal
option to fetch, and handle the abort/timeout by catching the thrown
DOMException/AbortError and rethrowing a clear error (or propagate) so a hung
proxy won't block the resolver chain; keep the timeout value as a configurable
constant near queryDns.

---

Nitpick comments:
In `@src/index.ts`:
- Around line 27-32: Add a focused contract test that asserts the exact order of
the exported defaultDnsResolvers array to prevent accidental reorderings: write
a unit test that imports defaultDnsResolvers and verifies its sequence equals
[googleDnsResolver, cloudflareDnsResolver, proxyDnsResolver, aliDnsResolver] (or
compares names/identifiers if direct object equality is brittle); name the test
to indicate it guards resolver fallback order and keep it small and isolated so
future refactors will fail if the order changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ea4ddf77-8d9f-409e-a414-0e029def52d3

📥 Commits

Reviewing files that changed from the base of the PR and between 138a395 and 9301a5c.

📒 Files selected for processing (4)
  • src/index.ts
  • src/util/dns-resolvers/dns-resolvers.test.ts
  • src/util/dns-resolvers/index.ts
  • src/util/dns-resolvers/proxy-dns-resolver.ts

Comment thread src/util/dns-resolvers/proxy-dns-resolver.ts
Comment thread src/util/dns-resolvers/proxy-dns-resolver.ts
@rongquan1 rongquan1 requested a review from RishabhS7 May 11, 2026 09:41
@RishabhS7 RishabhS7 merged commit 0935846 into master May 11, 2026
11 checks passed
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 2.21.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants