fix(resolver): handle None result.service from getnameinfo() by rnagulapalle · Pull Request #12392 · aio-libs/aiohttp

rnagulapalle · 2026-04-19T02:50:40Z

What do these changes do?

Handle the rare async resolver path where aiodns / pycares returns a link-local IPv6 getnameinfo() result with service=None.

The resolver now falls back to the numeric port from the original address tuple instead of passing None to int().

Are there changes in behavior for the user?

Yes. Link-local IPv6 resolution through AsyncResolver no longer raises when result.service is None; it keeps the original port value instead.

Is it a substantial burden for the maintainers to support this?

No. The change is limited to the existing link-local IPv6 resolver branch and mirrors the fallback behavior already used by the non-link-local IPv6 and IPv4 paths, which keep address[1] as the port.

Related issue number

No linked issue.

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes
If you provide code modification, please add yourself to CONTRIBUTORS.txt
- The format is .
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES/ folder
- name it <issue_or_pr_num>.<type>.rst (e.g. 588.bugfix.rst)
- if you don't have an issue number, change it to the pull request
  number after creating the PR
  - .bugfix: A bug fix for something the maintainers deemed an
    improper undesired behavior that got corrected to match
    pre-agreed expectations.
  - .feature: A new behavior, public APIs. That sort of stuff.
  - .deprecation: A declaration of future API removals and breaking
    changes in behavior.
  - .breaking: When something public is removed in a breaking way.
    Could be deprecated in an earlier release.
  - .doc: Notable updates to the documentation structure or build
    process.
  - .packaging: Notes for downstreams about unobvious side effects
    and tooling. Changes in the test invocation considerations and
    runtime assumptions.
  - .contrib: Stuff that affects the contributor experience. e.g.
    Running tests, building the docs, setting up the development
    environment.
  - .misc: Changes that are hard to assign to any of the above
    categories.
- Make sure to use full sentences with correct case and punctuation.

Test plan

python3 -m py_compile aiohttp/resolver.py tests/test_resolver.py

getnameinfo() returns a NameinfoResult where service is typed as str | None. Passing None directly to int() fails mypy's arg-type check. Fall back to the original numeric port when service is None. Fixes mypy error: Argument 1 to "int" has incompatible type "str | None"; expected "str | Buffer | SupportsInt | ..."

for more information, see https://pre-commit.ci

rnagulapalle · 2026-04-19T02:54:09Z

Closing — will be re-raised by Phalanx CI Fixer autonomously.

codecov · 2026-04-19T02:57:03Z

Codecov Report

❌ Patch coverage is 29.62963% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.29%. Comparing base (8059959) to head (ca363eb).
⚠️ Report is 34 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
tests/test_resolver.py	28.00%	18 Missing ⚠️
aiohttp/resolver.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #12392       +/-   ##
===========================================
- Coverage   98.94%   47.29%   -51.66%     
===========================================
  Files         131      131               
  Lines       46683    46706       +23     
  Branches     2421     2421               
===========================================
- Hits        46189    22088    -24101     
- Misses        369    23996    +23627     
- Partials      125      622      +497

Flag	Coverage Δ
Autobahn	`22.45% <25.92%> (+<0.01%)`	⬆️
CI-GHA	`?`
OS-Linux	`?`
OS-Windows	`?`
OS-macOS	`?`
Py-3.10.11	`?`
Py-3.10.20	`?`
Py-3.11.15	`?`
Py-3.11.9	`?`
Py-3.12.10	`?`
Py-3.12.13	`?`
Py-3.13.13	`?`
Py-3.14.4	`?`
Py-3.14.5	`?`
Py-3.14.5t	`?`
Py-pypy3.11.15-7.3.22	`?`
VM-macos	`?`
VM-ubuntu	`?`
VM-windows	`?`
cython-coverage	`37.89% <3.70%> (-0.02%)`	⬇️

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

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

codspeed-hq · 2026-04-19T03:04:32Z

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 69 skipped benchmarks¹

_{Comparing rnagulapalle:fix/resolver-service-none-mypy (ca363eb) with master (edc1cc2)²}

69 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on master (8059959) during the generation of this report, so edc1cc2 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

webknjaz · 2026-04-30T15:30:04Z

@rnagulapalle could you use our PR template instead of replacing it?

Dreamsorcerer · 2026-05-04T00:09:36Z

+                        int(result.service)
+                        if result.service is not None
+                        else address[1]


Is there a situation where result.service != address[1]?

Resolve the resolver test conflict while keeping the upstream DNSError regression coverage. Co-authored-by: Cursor <cursoragent@cursor.com>

rnagulapalle · 2026-05-16T19:37:23Z

Updated the PR description to follow the repository template. Thanks for the pointer.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes two bugs in AsyncResolver: an IndexError when handling aiodns.error.DNSError with a single argument, and handling of None for result.service returned by getnameinfo() for link-local IPv6 addresses.

Changes:

Corrected the length check from >= 1 to >= 2 when accessing exc.args[1] in DNS error handling.
Added a fallback to address[1] for the port when result.service is None in the link-local IPv6 path.
Added tests for the single-arg DNSError case and for extracting port from getnameinfo(), and updated the test fixture to accept a configurable service.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
aiohttp/resolver.py	Off-by-one fix in DNSError handling and `None`-safe port extraction from `getnameinfo()`.
tests/test_resolver.py	New tests for the bugfixes and updated fake `getnameinfo` helper to accept a `service` argument.
CHANGES/12253.bugfix.rst	News fragment describing both fixes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    def __init__(self, host: str, service: str = "0") -> None:
        self.node = host
-        self.service = None
+        self.service = service


        except aiodns.error.DNSError as exc:
-            msg = exc.args[1] if len(exc.args) >= 1 else "DNS lookup failed"
+            msg = exc.args[1] if len(exc.args) >= 2 else "DNS lookup failed"
            raise OSError(None, msg) from exc


Dreamsorcerer · 2026-05-19T19:12:58Z

Updated the PR description to follow the repository template. Thanks for the pointer.

Could you also answer the inline question? I'm unclear if there should be an if condition there at all.

N3XT3R1337 and others added 2 commits March 18, 2026 11:43

fix: improve type safety and error handling

fb31388

rnagulapalle requested review from asvetlov and webknjaz as code owners April 19, 2026 02:50

psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Apr 19, 2026

[pre-commit.ci] auto fixes from pre-commit.com hooks

4459e6e

for more information, see https://pre-commit.ci

rnagulapalle closed this Apr 19, 2026

rnagulapalle reopened this Apr 19, 2026

Dreamsorcerer reviewed May 4, 2026

View reviewed changes

Merge master into resolver service fix.

b8fb625

Resolve the resolver test conflict while keeping the upstream DNSError regression coverage. Co-authored-by: Cursor <cursoragent@cursor.com>

Dreamsorcerer reviewed May 13, 2026

View reviewed changes

Comment thread tests/test_resolver.py Outdated

Apply suggestion from @Dreamsorcerer

6e59c58

Dreamsorcerer added the pr-unfinished The PR is unfinished and may need a volunteer to complete it label May 13, 2026

github-actions Bot removed the pr-unfinished The PR is unfinished and may need a volunteer to complete it label May 16, 2026

Merge branch 'master' into fix/resolver-service-none-mypy

ca363eb

bdraco requested review from bdraco and Copilot May 16, 2026 20:31

Copilot AI reviewed May 16, 2026

View reviewed changes

Copilot started reviewing on behalf of bdraco May 16, 2026 20:49 View session

Dreamsorcerer added the pr-unfinished The PR is unfinished and may need a volunteer to complete it label May 19, 2026

github-actions Bot removed the pr-unfinished The PR is unfinished and may need a volunteer to complete it label May 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(resolver): handle None result.service from getnameinfo()#12392

fix(resolver): handle None result.service from getnameinfo()#12392
rnagulapalle wants to merge 6 commits into
aio-libs:masterfrom
rnagulapalle:fix/resolver-service-none-mypy

rnagulapalle commented Apr 19, 2026 •

edited

Loading

Uh oh!

rnagulapalle commented Apr 19, 2026

Uh oh!

codecov Bot commented Apr 19, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Apr 19, 2026 •

edited

Loading

Uh oh!

webknjaz commented Apr 30, 2026

Uh oh!

Dreamsorcerer May 4, 2026

Uh oh!

Uh oh!

rnagulapalle commented May 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Dreamsorcerer commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

rnagulapalle commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What do these changes do?

Are there changes in behavior for the user?

Is it a substantial burden for the maintainers to support this?

Related issue number

Checklist

Test plan

Uh oh!

rnagulapalle commented Apr 19, 2026

Uh oh!

codecov Bot commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq Bot commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

webknjaz commented Apr 30, 2026

Uh oh!

Dreamsorcerer May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rnagulapalle commented May 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Dreamsorcerer commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rnagulapalle commented Apr 19, 2026 •

edited

Loading

codecov Bot commented Apr 19, 2026 •

edited

Loading

codspeed-hq Bot commented Apr 19, 2026 •

edited

Loading