Skip to content

fix: set code ENOTFOUND on requests blocked by restrictedAddresses - #1572

Closed
Kart03 wants to merge 4 commits into
developfrom
fix/restricted-address-error-code
Closed

Kart03 wants to merge 4 commits into
developfrom
fix/restricted-address-error-code

Conversation

@Kart03

@Kart03 Kart03 commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Context

A request blocked by network.restrictedAddresses fails with an Error whose message is NETERR: getaddrinfo ENOTFOUND <host>, the same wording as a real DNS failure, but with no code. A real DNS failure from Node carries code: 'ENOTFOUND'. Consumers that name or group errors by code therefore see a bare Error for blocked addresses and can't treat them like other host-not-found failures.

Summary of the change

The error is now built in one place, restrictedAddressError(hostname) in lib/requester/core.js, and sets code = 'ENOTFOUND'. It replaces the five places that built it by hand: the pre-request hostname check in requester.js, the two DNS lookup paths, the *.localhost check, and the redirect listener. The message is unchanged, so anything matching on it keeps working.

I used ENOTFOUND rather than a new code so the error matches what its message already says and exposes nothing new about why the request was blocked.

Breaking changes

None. error.code goes from undefined to 'ENOTFOUND' on these errors only. Code that checks error.code === undefined to spot blocked addresses would change. Not checked for in dependents.

Dependencies

None.

Test plan

Result
npx mocha test/unit/requester-core.test.js 101 passing
npm run test-integration passes, including the new code assertion on a blocked address

The new assertions are on existing tests: the redirect listener (unit) and a request to a restricted IP (integration). Not checked: lint. The repo's ESLint config conflicts with a parent config in the worktree I ran it from.

Additional notes

Changelog entry added under unreleased.

Generated with Claude Code

The error had the message of a DNS failure but no code, so consumers that
name errors by code saw a bare Error.

Co-Authored-By: Claude <noreply@anthropic.com>
@Kart03
Kart03 marked this pull request as ready for review October 26, 2026 13:08
Co-Authored-By: Claude <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.84%. Comparing base (6b4c942) to head (7cb0d83).

❌ Your project check has failed because the head coverage (34.09%) is below the target coverage (35.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1572      +/-   ##
===========================================
+ Coverage    77.72%   77.84%   +0.11%     
===========================================
  Files           51       51              
  Lines         4369     4373       +4     
  Branches      1234     1234              
===========================================
+ Hits          3396     3404       +8     
+ Misses         730      725       -5     
- Partials       243      244       +1     
Flag Coverage Δ
integration 69.01% <90.00%> (+0.02%) ⬆️
legacy 34.09% <20.00%> (-0.01%) ⬇️
unit 47.29% <70.00%> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

Kart03 and others added 2 commits October 26, 2026 18:52
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@appurva21

Copy link
Copy Markdown
Member

I don’t think we should make this change.

  • A blocked address and a real DNS failure are different failures, and giving both code: 'ENOTFOUND' could be misleading for certain consumers.
  • The change also only partly matches Node: Node’s DNS error has no NETERR: prefix and carries errno, syscall and hostname, which this error lacks, and Node never returns ENOTFOUND for an IP literal. The NETERR: prefix already lets consumers tell blocked requests apart, so it’s up to each consumer to decide how to handle them.

@Kart03

Kart03 commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

Agreed, thanks. A blocked address isn't a DNS failure, and the NETERR: prefix already marks it. Closing this; I'll classify it on our side using that prefix.

@Kart03 Kart03 closed this Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants