feat(dns-cache): add tegg dns cache plugin#6018
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughA new ChangesDNS Cache Plugin
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #6018 +/- ##
========================================
Coverage 81.94% 81.95%
========================================
Files 677 678 +1
Lines 20652 20800 +148
Branches 4100 4147 +47
========================================
+ Hits 16923 17046 +123
- Misses 3215 3236 +21
- Partials 514 518 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new DNS cache plugin (@eggjs/tegg-dns-cache) for the tegg framework, providing DNS caching with LRU eviction, round-robin address rotation, support for both lookup and resolve modes, and integration with Egg's HTTP client. The review feedback highlights critical improvements for the DnsResolver implementation: implementing promise coalescing to prevent cache stampedes under high load, enforcing a minimum TTL of 1000ms to avoid rapid-fire background updates when TTL is zero, and correcting the behavior of options.all to return all resolved addresses in a rotated order rather than a single address wrapped in an array.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the tegg DNS cache plugin into the egg monorepo as @eggjs/dns-cache-plugin, adding a new opt-in tegg plugin package that injects a cached dns.lookup implementation into Egg’s httpclient, plus a Vitest-based test suite and ESM fixtures.
Changes:
- Add
@eggjs/dns-cache-pluginpackage (types, app lifecycle hook, config defaults, andDnsResolverimplementation). - Add Vitest tests + ESM fixtures to validate lookup/resolve modes, caching, and address rotation behavior.
- Add package metadata (exports/publishConfig) and TS project config for the new plugin workspace.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tegg/plugin/dns-cache/tsconfig.json | Adds TS project config for the new plugin workspace. |
| tegg/plugin/dns-cache/test/utils.ts | Adds local HTTP server helpers used by the test suites. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/package.json | Declares ESM fixture app for resolve-mode tests. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/config/plugin.ts | Enables tegg + dns-cache plugin in the resolve-mode fixture. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/config/config.default.ts | Fixture config for resolve mode and httpclient settings. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/app/router.ts | Minimal route setup for resolve-mode fixture. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/app/controller/home.ts | Fixture controller that proxies requests via ctx.curl. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/package.json | Declares ESM fixture app for lookup-mode tests. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/config/plugin.ts | Enables tegg + dns-cache plugin in the lookup-mode fixture. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/config/config.default.ts | Fixture config for lookup mode with lookup interval / rotation. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/app/router.ts | Minimal route setup for lookup-mode fixture. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/app/controller/home.ts | Fixture controller that proxies requests via ctx.curl. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/package.json | Declares ESM fixture app for httpclient-next lookup tests. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/config/plugin.ts | Enables tegg + dns-cache plugin in the httpclient-next fixture. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/config/config.default.ts | Fixture config enabling useHttpClientNext + dns-cache lookup mode. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/app/router.ts | Minimal route setup for httpclient-next fixture. |
| tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/app/controller/home.ts | Fixture controller that proxies requests via ctx.curl. |
| tegg/plugin/dns-cache/test/dns_cache_resolve.test.ts | Adds resolve-mode integration tests for dns-cache. |
| tegg/plugin/dns-cache/test/dns_cache_lookup.test.ts | Adds lookup-mode integration tests, including rotation and fetch coverage. |
| tegg/plugin/dns-cache/test/dns_cache_lookup_http_next.test.ts | Adds lookup-mode tests for useHttpClientNext. |
| tegg/plugin/dns-cache/src/types.ts | Adds Egg module augmentation for app.dnsResolver and config.dnsCache. |
| tegg/plugin/dns-cache/src/lib/DnsResolver.ts | Implements cached DNS lookup/resolve with optional address rotation. |
| tegg/plugin/dns-cache/src/index.ts | Exposes the plugin’s public exports and type augmentation. |
| tegg/plugin/dns-cache/src/config/config.default.ts | Provides default dnsCache configuration and inline docs. |
| tegg/plugin/dns-cache/src/app.ts | Wires resolver into Egg lifecycle + httpclient lookup hook. |
| tegg/plugin/dns-cache/README.md | Documents installation/configuration/usage of the plugin. |
| tegg/plugin/dns-cache/package.json | Defines package metadata, exports maps, deps, and engine constraints. |
| tegg/plugin/dns-cache/CHANGELOG.md | Adds historical changelog content for the migrated plugin. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@tegg/plugin/dns-cache/README.md`:
- Around line 24-30: The README examples for dns-cache use CommonJS and .js
config, which is inconsistent with tegg’s ESM/TypeScript conventions. Update the
configuration snippets to match the fixture patterns in this PR by using the
ESM/TypeScript config style in the relevant example blocks. Refer to the
dnsCache enablement example and any other related snippets in the README so they
all use the same tegg-compatible format.
- Line 1: The README still uses the old package name, so update the title and
any install/reference text in the dns-cache plugin docs to
`@eggjs/dns-cache-plugin`. Make the change in the README content that identifies
the package (including the heading and installation instructions) so users are
directed to the renamed package instead of `@eggjs/tegg-dns-cache`.
- Around line 107-108: The README example for resolver.getCacheRecord shows the
wrong object shape; update it to match the actual cache entry structure used by
DnsResolver.ts. Inspect getCacheRecord and the cache entry type so the example
reflects a record object with a records array of { address, family, ttl,
timestamp } entries plus currentIndex, instead of a single { ip, family, ttl }
object.
In `@tegg/plugin/dns-cache/src/lib/DnsResolver.ts`:
- Around line 156-173: TTL=0 DNS answers are being stored and then immediately
served from the cache in DnsResolver, which effectively turns “do not cache”
into stale-while-revalidate behavior. Update the cache write path in _updateDNS
(the logic that stores records and their timestamp/ttl) to skip inserting
entries with ttl === 0, or otherwise force a synchronous re-resolve before
returning in the cache-hit branch. Make sure the cache-hit handling around
_callbackWithRecord only serves entries that were actually cacheable, so
zero-TTL responses do not linger in the cache.
- Line 8: The DNS cache lookup implementation in DnsResolver is not behaving
like a native dns.lookup replacement, which breaks IPv6 and multi-address
callers when wired through app.config.httpclient.lookup. Update the resolver
logic so the lookup path in DnsResolver and the function assigned from
src/app.ts preserve the requested family, support all: true by returning all
resolved addresses, and avoid forcing IPv4-only behavior or hostname-only
caching that changes dns.lookup semantics.
In `@tegg/plugin/dns-cache/test/dns_cache_lookup.test.ts`:
- Around line 17-427: The current dns cache lookup tests only validate behavior
on a single MockApplication, so they miss resolver/cache cross-talk when two
apps start together. Add a new multi-app regression in this suite or a dedicated
concurrent test that creates two MockApplication instances at the same time and
exercises the dnsResolver/cache paths separately. Verify isolated state for each
app by using the existing app.dnsResolver, getCacheRecord/getDnsCache, and
curl/fetch behaviors to ensure one app’s lookup, cache updates, and rotation do
not affect the other.
- Around line 401-424: The fetch rotation test in dns_cache_lookup.test.ts is
using the wrong modulus in the assertion, so it never validates movement across
both cached addresses. Update the assertion in the "should rotate with fetch"
case to use the same cached entry count returned by the mocked
dnsResolver.lookup (two entries) when checking entry.currentIndex after the
second app.fetch call, so the test actually verifies rotation in
dnsResolver.getDnsCache().
In `@tegg/plugin/dns-cache/test/dns_cache_resolve.test.ts`:
- Around line 20-23: The regression test is still relying on public DNS servers
and an external hostname, so keep it fully local by replacing the network
dependency in dns_cache_resolve.test.ts with a fixture-controlled DNS target.
Update the setup around app.dnsResolver.setServers and the resolve assertions to
use a test-owned resolver/domain fixture instead of 223.5.5.5, 223.6.6.6, and
www.aliyun.com, and make the same change in the corresponding later block so the
success path only exercises the plugin logic.
- Around line 19-23: The resolver test setup in dns_cache_resolve.test.ts is
swallowing failures from app.dnsResolver.setServers(), which allows the suite to
keep running against the host resolver instead of the plugin path. Update the
test setup to fail fast in this block by removing the catch-and-log-only
behavior, or by rethrowing the caught error after logging, so the dns-cache
resolve tests only proceed when the DNS servers are successfully pinned.
In `@tegg/plugin/dns-cache/test/utils.ts`:
- Around line 6-18: `startLocalServer` currently only returns a URL and relies
on `process.once('exit')`, which leaves test servers open during the suite;
update this helper to also expose a disposer/close function tied to the created
`http.Server` so callers can shut it down explicitly. While doing so, wire
startup failure handling through the server’s `'error'` event in
`startLocalServer`/`listen` logic instead of expecting the listen callback to
receive an error, and reject the promise from that path so bind failures don’t
hang. Use the existing `localServer` setup in `startLocalServer` as the main
place to attach the cleanup and error handling.
🪄 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: f8712c1f-4368-4de7-9789-3006f925a74d
📒 Files selected for processing (28)
tegg/plugin/dns-cache/CHANGELOG.mdtegg/plugin/dns-cache/README.mdtegg/plugin/dns-cache/package.jsontegg/plugin/dns-cache/src/app.tstegg/plugin/dns-cache/src/config/config.default.tstegg/plugin/dns-cache/src/index.tstegg/plugin/dns-cache/src/lib/DnsResolver.tstegg/plugin/dns-cache/src/types.tstegg/plugin/dns-cache/test/dns_cache_lookup.test.tstegg/plugin/dns-cache/test/dns_cache_lookup_http_next.test.tstegg/plugin/dns-cache/test/dns_cache_resolve.test.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/app/controller/home.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/app/router.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/config/config.default.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/config/plugin.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/package.jsontegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/app/controller/home.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/app/router.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/config/config.default.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/config/plugin.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/package.jsontegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/app/controller/home.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/app/router.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/config/config.default.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/config/plugin.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/package.jsontegg/plugin/dns-cache/test/utils.tstegg/plugin/dns-cache/tsconfig.json
Migrate `@eggjs/tegg-dns-cache` from the standalone tegg repo into the monorepo as `@eggjs/dns-cache-plugin`, following the established tegg plugin migration pattern (ESM, `src/` layout, exports map + publishConfig, vitest + `@eggjs/mock`, isolatedDeclarations). It is an opt-in plugin (not added to egg's default plugin list), so the change is self-contained under `tegg/plugin/dns-cache/`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
elrrrrrrr
force-pushed
the
feat/tegg-dns-cache-plugin
branch
from
ef2c463 to
5247909
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tegg/plugin/dns-cache/CHANGELOG.md`:
- Around line 6-411: Add a migration note at the top of the changelog explaining
that the history was inherited from the old package and is now for
`@eggjs/dns-cache-plugin`. Update the repeated package references in the existing
entries, especially the “Version bump only for package” notes, so they clearly
reflect the new package name. Use the changelog heading/history block as the
place to make the rename explicit and avoid implying this file belongs to
`@eggjs/tegg-dns-cache`.
- Around line 388-399: The release sections in CHANGELOG.md have a heading
hierarchy jump because the “Features” subsections are marked as H3 directly
under the version headings. Update the headings in the affected release blocks
so they follow the existing document hierarchy consistently, using the same
heading level as other subsections in this changelog and keeping the version
headings as the unique anchors to locate the affected sections.
🪄 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: 12220b65-ed00-4a1b-9790-f0a931d8b831
📒 Files selected for processing (28)
tegg/plugin/dns-cache/CHANGELOG.mdtegg/plugin/dns-cache/README.mdtegg/plugin/dns-cache/package.jsontegg/plugin/dns-cache/src/app.tstegg/plugin/dns-cache/src/config/config.default.tstegg/plugin/dns-cache/src/index.tstegg/plugin/dns-cache/src/lib/DnsResolver.tstegg/plugin/dns-cache/src/types.tstegg/plugin/dns-cache/test/dns_cache_lookup.test.tstegg/plugin/dns-cache/test/dns_cache_lookup_http_next.test.tstegg/plugin/dns-cache/test/dns_cache_resolve.test.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/app/controller/home.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/app/router.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/config/config.default.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/config/plugin.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/package.jsontegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/app/controller/home.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/app/router.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/config/config.default.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/config/plugin.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/package.jsontegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/app/controller/home.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/app/router.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/config/config.default.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/config/plugin.tstegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/package.jsontegg/plugin/dns-cache/test/utils.tstegg/plugin/dns-cache/tsconfig.json
✅ Files skipped from review due to trivial changes (9)
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/package.json
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/app/router.ts
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/package.json
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/package.json
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/config/plugin.ts
- tegg/plugin/dns-cache/src/types.ts
- tegg/plugin/dns-cache/README.md
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/config/config.default.ts
- tegg/plugin/dns-cache/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/config/plugin.ts
- tegg/plugin/dns-cache/tsconfig.json
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/app/controller/home.ts
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/app/router.ts
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/config/config.default.ts
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup/app/router.ts
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/config/plugin.ts
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/config/config.default.ts
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_lookup_http_next/app/controller/home.ts
- tegg/plugin/dns-cache/test/fixtures/apps/dns_cache_resolve/app/controller/home.ts
- tegg/plugin/dns-cache/src/config/config.default.ts
- tegg/plugin/dns-cache/src/app.ts
- tegg/plugin/dns-cache/test/dns_cache_lookup.test.ts
- tegg/plugin/dns-cache/test/dns_cache_lookup_http_next.test.ts
- tegg/plugin/dns-cache/src/lib/DnsResolver.ts
Address review feedback: the README, the configWillLoad warning prefix, and the CHANGELOG still referenced the old `@eggjs/tegg-dns-cache` name. Update them to `@eggjs/dns-cache-plugin`, and reset the CHANGELOG to the standard monorepo header (the old per-version lerna history under the old name is dropped). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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
Migrate
@eggjs/tegg-dns-cachefrom the standalone tegg repo into this monorepo as@eggjs/dns-cache-plugin, one of the tegg packages not yet brought over.This is a faithful code move
The plugin logic is migrated as-is — the runtime behavior of
DnsResolveris intentionally identical to the upstream tegg implementation. Only the packaging was adapted to monorepo conventions (rename, ESM +src/layout, exports map,workspace:/catalog:deps, vitest +@eggjs/mock,isolatedDeclarations).Automated-review suggestions about the resolver's behavior — e.g. promise coalescing to avoid cache stampedes,
options.allreturning the full rotated address list, enforcing a minimum TTL — describe pre-existing behavior carried over verbatim. They're intentionally out of scope for this move and can be picked up in a separate follow-up.Scope
Self-contained under
tegg/plugin/dns-cache/:@eggjs/tegg-dns-cache→@eggjs/dns-cache-pluginsrc/layout;exports+publishConfig.exports;workspace:*/catalog:depsisolatedDeclarations/verbatimModuleSyntaxcompliantegg-mock→ vitest/@eggjs/mock; fixtures to ESMpackages/eggor any shared config. Apps enable it via their ownconfig/plugin.Test evidence
tsgo --noEmit(typecheck),tsdown(build /.d.tsemit), andoxlint --type-awareare clean.223.5.5.5, real-domain resolution — are gated behind!process.env.CI.)🤖 Generated with Claude Code