Skip to content

dns: better precision for Resolver timeout - #65343

Open
starrify wants to merge 3 commits into
nodejs:mainfrom
starrify:dns-sync-uv-loop-time-upon-start-timer-fix-65340
Open

dns: better precision for Resolver timeout#65343
starrify wants to merge 3 commits into
nodejs:mainfrom
starrify:dns-sync-uv-loop-time-upon-start-timer-fix-65340

Conversation

@starrify

@starrify starrify commented Aug 17, 2026

Copy link
Copy Markdown

Changes:

3cba1b1876c test: add test case for dns Resolver for maxTimeout with tries=1
ed5f5fc97ba dns: update uv event loop time in ChannelWrap::StartTimer
d26572d7654 dns: delay the AresTimeout timer by 1 ms

Fixes: #65340

Edit: Branch force-updated to fix a typo.

Refs: nodejs#65340
Signed-off-by: Peng-Yu Chen <pengyu@libstarrify.so>
Otherwise the event loop time may be late by a considerable amount (e.g.
10~30 ms) and the timer callback would fire earlier by that amount.
Timeout processing in c-ares would then fail to recognize a timeout
until the next firing of this timer.

Fixes: nodejs#65340
Signed-off-by: Peng-Yu Chen <pengyu@libstarrify.so>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. needs-ci PRs that need a full CI run. labels Aug 17, 2026
@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.12%. Comparing base (ad7a5b8) to head (e75ce7d).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #65343   +/-   ##
=======================================
  Coverage   90.11%   90.12%           
=======================================
  Files         752      752           
  Lines      251569   251570    +1     
  Branches    47268    47265    -3     
=======================================
+ Hits       226701   226726   +25     
+ Misses      16233    16170   -63     
- Partials     8635     8674   +39     
Files with missing lines Coverage Δ
src/cares_wrap.cc 63.12% <100.00%> (+0.02%) ⬆️

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@starrify
starrify force-pushed the dns-sync-uv-loop-time-upon-start-timer-fix-65340 branch from e75ce7d to 2000c78 Compare August 17, 2026 10:39
This is because ChannelWrap::StartTimer happens before
ares_calc_query_timeout usually by a few tens of microseconds but could
be longer. An additional allowance of 1 ms helps reduce the chance of
missing a timeout.

Fixes: nodejs#65340
Signed-off-by: Peng-Yu Chen <pengyu@libstarrify.so>
@starrify
starrify force-pushed the dns-sync-uv-loop-time-upon-start-timer-fix-65340 branch from 2000c78 to d26572d Compare August 17, 2026 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dns: Resolver timeout seemingly doubled upon tries=1

2 participants