Re-raise OSError as ClientConnectionError when failing to explicitly close connector socket by bdraco · Pull Request #10551 · aio-libs/aiohttp

bdraco · 2025-03-14T23:59:23Z

What do these changes do?

This is a followup to #10464 to handle the case where socket.close() can also raise. This matches the logic we have in aiohappyeyeballs:

https://github.com/aio-libs/aiohappyeyeballs/blob/e3bd5bdf44f5d187802de6dcb08d27e1ca6da048/src/aiohappyeyeballs/impl.py#L227

We shouldn't raising OSError externally from this method as callers expect a ClientError

Are there changes in behavior for the user?

bugfix

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

no

Related issue number

fixes #10506

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 <Name> <Surname>.
- 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,
  for example:
```
Fixed issue with non-ascii contents in doctest text files
-- by :user:`contributor-gh-handle`.
```
  Use the past tense or the present tense a non-imperative mood,
  referring to what's changed compared to the last released version
  of this project.

…ose connector socket This is a followup to #10464 to handle the case where socket.close() can also raise. This matches the logic we have in aiohappyeyeballs: https://github.com/aio-libs/aiohappyeyeballs/blob/e3bd5bdf44f5d187802de6dcb08d27e1ca6da048/src/aiohappyeyeballs/impl.py#L227 fixes #10506

codspeed-hq · 2025-03-15T00:09:43Z

CodSpeed Performance Report

Merging #10551 will not alter performance

_{Comparing reraise_close_failure_as_client_error (a48e406) with master (45b861f)}

Summary

✅ 46 untouched benchmarks

codecov · 2025-03-15T00:12:56Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.69%. Comparing base (45b861f) to head (a48e406).
Report is 562 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #10551   +/-   ##
=======================================
  Coverage   98.69%   98.69%           
=======================================
  Files         122      122           
  Lines       37230    37241   +11     
  Branches     2064     2064           
=======================================
+ Hits        36745    36756   +11     
  Misses        338      338           
  Partials      147      147

Flag	Coverage Δ
CI-GHA	`98.57% <100.00%> (+<0.01%)`	⬆️
OS-Linux	`98.24% <100.00%> (+<0.01%)`	⬆️
OS-Windows	`96.18% <100.00%> (+<0.01%)`	⬆️
OS-macOS	`97.36% <100.00%> (+<0.01%)`	⬆️
Py-3.10.11	`97.26% <100.00%> (+<0.01%)`	⬆️
Py-3.10.16	`97.81% <100.00%> (+<0.01%)`	⬆️
Py-3.11.11	`97.89% <100.00%> (+<0.01%)`	⬆️
Py-3.11.9	`97.35% <100.00%> (+<0.01%)`	⬆️
Py-3.12.9	`98.35% <100.00%> (+<0.01%)`	⬆️
Py-3.13.2	`98.34% <100.00%> (+<0.01%)`	⬆️
Py-3.9.13	`97.15% <100.00%> (-0.01%)`	⬇️
Py-3.9.21	`97.68% <100.00%> (+<0.01%)`	⬆️
Py-pypy7.3.16	`83.84% <100.00%> (-5.39%)`	⬇️
VM-macos	`97.36% <100.00%> (+<0.01%)`	⬆️
VM-ubuntu	`98.24% <100.00%> (+<0.01%)`	⬆️
VM-windows	`96.18% <100.00%> (+<0.01%)`	⬆️

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.

patchback · 2025-03-16T00:33:25Z

Backport to 3.11: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.11/d067260df75e4d04a23a0481cf9cf8f7194c80f1/pr-10551

Backported as #10561

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

…close connector socket (#10551)  ## What do these changes do? This is a followup to #10464 to handle the case where `socket.close()` can also raise. This matches the logic we have in aiohappyeyeballs: https://github.com/aio-libs/aiohappyeyeballs/blob/e3bd5bdf44f5d187802de6dcb08d27e1ca6da048/src/aiohappyeyeballs/impl.py#L227 We shouldn't raising `OSError` externally from this method as callers expect a `ClientError` ## Are there changes in behavior for the user? bugfix ## Is it a substantial burden for the maintainers to support this? no ## Related issue number fixes #10506 ## Checklist - [x] I think the code is well written - [x] Unit tests for the changes exist - [x] Documentation reflects the changes - [x] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [x] 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, for example: ```rst Fixed issue with non-ascii contents in doctest text files -- by :user:`contributor-gh-handle`. ``` Use the past tense or the present tense a non-imperative mood, referring to what's changed compared to the last released version of this project. (cherry picked from commit d067260)

patchback · 2025-03-16T00:33:32Z

Backport to 3.12: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.12/d067260df75e4d04a23a0481cf9cf8f7194c80f1/pr-10551

Backported as #10562

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

…close connector socket (#10551)  ## What do these changes do? This is a followup to #10464 to handle the case where `socket.close()` can also raise. This matches the logic we have in aiohappyeyeballs: https://github.com/aio-libs/aiohappyeyeballs/blob/e3bd5bdf44f5d187802de6dcb08d27e1ca6da048/src/aiohappyeyeballs/impl.py#L227 We shouldn't raising `OSError` externally from this method as callers expect a `ClientError` ## Are there changes in behavior for the user? bugfix ## Is it a substantial burden for the maintainers to support this? no ## Related issue number fixes #10506 ## Checklist - [x] I think the code is well written - [x] Unit tests for the changes exist - [x] Documentation reflects the changes - [x] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [x] 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, for example: ```rst Fixed issue with non-ascii contents in doctest text files -- by :user:`contributor-gh-handle`. ``` Use the past tense or the present tense a non-imperative mood, referring to what's changed compared to the last released version of this project. (cherry picked from commit d067260)

…ionError when failing to explicitly close connector socket (#10561) **This is a backport of PR #10551 as merged into master (d067260).**  ## What do these changes do? This is a followup to #10464 to handle the case where `socket.close()` can also raise. This matches the logic we have in aiohappyeyeballs: https://github.com/aio-libs/aiohappyeyeballs/blob/e3bd5bdf44f5d187802de6dcb08d27e1ca6da048/src/aiohappyeyeballs/impl.py#L227 We shouldn't raising `OSError` externally from this method as callers expect a `ClientError` ## Are there changes in behavior for the user? bugfix ## Is it a substantial burden for the maintainers to support this? no ## Related issue number fixes #10506 ## Checklist - [x] I think the code is well written - [x] Unit tests for the changes exist - [x] Documentation reflects the changes - [x] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [x] 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, for example: ```rst Fixed issue with non-ascii contents in doctest text files -- by :user:`contributor-gh-handle`. ``` Use the past tense or the present tense a non-imperative mood, referring to what's changed compared to the last released version of this project. Co-authored-by: J. Nick Koston <nick@koston.org>

…ionError when failing to explicitly close connector socket (#10562) **This is a backport of PR #10551 as merged into master (d067260).**  ## What do these changes do? This is a followup to #10464 to handle the case where `socket.close()` can also raise. This matches the logic we have in aiohappyeyeballs: https://github.com/aio-libs/aiohappyeyeballs/blob/e3bd5bdf44f5d187802de6dcb08d27e1ca6da048/src/aiohappyeyeballs/impl.py#L227 We shouldn't raising `OSError` externally from this method as callers expect a `ClientError` ## Are there changes in behavior for the user? bugfix ## Is it a substantial burden for the maintainers to support this? no ## Related issue number fixes #10506 ## Checklist - [x] I think the code is well written - [x] Unit tests for the changes exist - [x] Documentation reflects the changes - [x] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [x] 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, for example: ```rst Fixed issue with non-ascii contents in doctest text files -- by :user:`contributor-gh-handle`. ``` Use the past tense or the present tense a non-imperative mood, referring to what's changed compared to the last released version of this project. Co-authored-by: J. Nick Koston <nick@koston.org>

bdraco mentioned this pull request Mar 14, 2025

OSError: [Errno 9] Bad file descriptor with uvloop #10506

Open

1 task

bdraco added backport-3.11 labels Mar 15, 2025

changelog

c097a83

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Mar 15, 2025

changelog

f3ce6a8

fix docs mapping

a48e406

bdraco marked this pull request as ready for review March 15, 2025 00:14

bdraco requested review from asvetlov and webknjaz as code owners March 15, 2025 00:14

bdraco merged commit d067260 into master Mar 16, 2025
40 checks passed

bdraco deleted the reraise_close_failure_as_client_error branch March 16, 2025 00:33

patchback bot mentioned this pull request Mar 16, 2025

[PR #10551/d067260d backport][3.11] Re-raise OSError as ClientConnectionError when failing to explicitly close connector socket #10561

Merged

5 tasks

patchback bot mentioned this pull request Mar 16, 2025

[PR #10551/d067260d backport][3.12] Re-raise OSError as ClientConnectionError when failing to explicitly close connector socket #10562

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

Re-raise OSError as ClientConnectionError when failing to explicitly close connector socket#10551

Re-raise OSError as ClientConnectionError when failing to explicitly close connector socket#10551
bdraco merged 4 commits intomasterfrom
reraise_close_failure_as_client_error

bdraco commented Mar 14, 2025 •

edited

Loading

Uh oh!

codspeed-hq bot commented Mar 15, 2025 •

edited

Loading

Uh oh!

codecov bot commented Mar 15, 2025 •

edited

Loading

Uh oh!

Uh oh!

patchback bot commented Mar 16, 2025 •

edited

Loading

Uh oh!

patchback bot commented Mar 16, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Comments

Conversation

bdraco commented Mar 14, 2025 • 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

Uh oh!

codspeed-hq bot commented Mar 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #10551 will not alter performance

Summary

Uh oh!

codecov bot commented Mar 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

patchback bot commented Mar 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport to 3.11: 💚 backport PR created

Uh oh!

patchback bot commented Mar 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport to 3.12: 💚 backport PR created

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bdraco commented Mar 14, 2025 •

edited

Loading

codspeed-hq bot commented Mar 15, 2025 •

edited

Loading

codecov bot commented Mar 15, 2025 •

edited

Loading

patchback bot commented Mar 16, 2025 •

edited

Loading

patchback bot commented Mar 16, 2025 •

edited

Loading