Use client session for cloud with AF_INET connector

[ludeeus]

Proposed change

Disable IPv6 for the client session used by the cloud integration, similar to #98003, which was made possible by #102702.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Black (black --fast homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hey there @home-assistant/cloud, mind taking a look at this pull request as it has been labeled with an integration (cloud) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of cloud can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign cloud Removes the current integration label and assignees on the pull request, add the integration domain after the command.

[agners]

Since this is a general issue with aiohttp, I'd imagine we are going to apply this integration by integration. Could we maybe make this change globally somehow? 🤔

[ludeeus]

It was already implemented in august, after this PR, I plan on doing the same for cloudflare, so maybe 🤷‍♂️

[frenck]

Since this is a general issue with aiohttp, I'd imagine we are going to apply this integration by integration. Could we maybe make this change globally somehow? 🤔

This is basically what it does? As in, it is a property when getting the session. They will still share the same pools (when the family matches).

[balloob]

I think Stefan meant that we should just add this to the global create session. If IPv6 is broken, we should never fallback to it for any integration?

[agners]

I think Stefan meant that we should just add this to the global create session. If IPv6 is broken, we should never fallback to it for any integration?

Yes.

This would also allow to remove it globally once aiohttp gets fixed. This is assuming that all issues are related to the same aiohttp problem.

[ludeeus]

I'm unsure about august, but it might be the same as with cloud.

unrelated to core

For HACS I also see the same issue as with cloud from time-to-time

For Cloudflare, that only supports A records, so using v6 there does not make sense.

So there might be several reasons why forcing v4 is wanted.

[pvizeli]

I think that also depends on the service. So maybe in the future we will have IPv6 on our cloud, and then we can use it, but now that is not supported and need dual stack or ipv4 only networks.

btw, I don't get why it need a new file and not directly create this as single-line change

[ludeeus]

I did the new file for consistency but moved it to inline with 8edde8b

[agners]

For Cloudflare, that only supports A records, so using v6 there does not make sense.

If only A records are resolved, aiohttp (or rather the underlying socket implementation) will automatically choose the correct socket. For this no explicit socket configuration is required.

I think that also depends on the service. So maybe in the future we will have IPv6 on our cloud, and then we can use it, but now that is not supported and need dual stack or ipv4 only networks.

Wait, our servers do support IPv6 don't they? E.g. account.nabucasa.com resolves IPv6 addresses.

From what I understand, this is exactly the problem: Client has troubles connecting via IPv6 to the server. This is typically caused by bad IPv6 configuration on the users network (or routing issue on ISP side).

Usually, when such issues happen in IPv4 world, people fix them (as in ISP fixes the routes, user reboots/reconfigures/replaces the router etc). In IPv6 world, rather than debug the problem, the solution is usually to just disable IPv6 altogether (since there is the IPv4 fallback makes things then just work).

Just disabling IPv6 on the client as a whole is not an ideal solution, as this affects any IPv6 connection (other connections might still work e.g. local IPv6 connections etc.). To combat this problem RFC6555 aka Happy Eyeballs has been standardized. It essentially standardizes that even if a IP client has a valid (global) IPv6 address to speak to the server, and that connection fails for unknown reason, it falls back (quickly) to IPv4, so essentially does what this PR does automatically. And as #98003 mentions, aiohttp doesn't support Happy Eyeballs yet (aio-libs/aiohttp#4451).

So this PR is a work around for missing Happy Eyeballs support in aiohttp.

But yeah, in the end it depends also on the service: If a service only supports IPv4 on the other end, this work around is not needed. But it also wouldn't hurt actually, since the other endpoint only supports IPv4, the explicit ask for AF_INET won't make a difference.

IMHO, this really calls for a async_get_global_clientsession or async_get_cloud_clientsession. This allows to work around the missing happy eyeballs problem for any connection going out to the internet. That way, once happy eyeballs is implemented, we can change this back on a global level. This would also allow aiohttp session tuning for any session which is reaching out to the internet.

(Sidenote: If aiohttp doesn't get fixed, an people would like to use Home Assistant in a IPv6 only environment, this would allow to disable the work around on a global level too).

Maybe such a global session is a bit out of scope for this PR (since it would touch multiple integrations). I can do this in a separate PR, if preferred.

[ludeeus]

If only A records are resolved, aiohttp (or rather the underlying socket implementation) will automatically choose the correct socket. For this no explicit socket configuration is required.

The integration only supports that, the service supports both. Which then gives problems with dual-stack setups or setups with broken v6.

Closing this PR for now as what it set out to do currently seems impossible to get accepted.
FWIW #98003 and #102702 should probably be reverted as well.

[agners]

The integration only supports that, the service supports both. Which then gives problems with dual-stack setups or setups with broken v6.

What do you mean the integration only supports that (as in what does that refers to)?

Closing this PR for now as what it set out to do currently seems impossible to get accepted.
FWIW #98003 and #102702 should probably be reverted as well.

No please don't! I am sorry if I made the impression I am against this change! This change fixes an issue, I am very much agreeing. I just try to clarify what is exactly being addressed here, and try to make a suggestion on the implementation. As I said, I am happy to do this in a follow up PR!

[agners]

FWIW #98003 and #102702 should probably be reverted as well.

Agreed, those are linked, and I should have made my suggestions there already. I just got for this PR a notification, so that is why I jumped in here 🙈 .

[bdraco]

ESPHome has the same/similar issue esphome/aioesphomeapi#233

We need a happy eyeballs implementation that supports passing in a list of resolved addresses that works for python 3.8 for aiohttp so we can solve aio-libs/aiohttp#4451 (comment)

Ideally we build a lib for that which could be used for both esphome and aiohttp so we don't have to reinvent the wheel as I'm sure other places need it as well.

The 3.8 support will be a bit of a pain though since we need use a conditional import with https://pypi.org/project/async-stagger/ as asyncio.staggered.staggered_race is only available on newer python which makes the whole thing more complex and would make sense to make it in a new library

If I find the time to build something for esphome, I'll do it as a new library with a compatible license in case there is a chance we can reuse with aiohttp.

[bdraco]

aiohttp 3.10.0 has happyeyeballs support so hopefully if this was still an issue, it will be solved in 2024.8.0