Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

session of AioHttpTransport is None at runtime, despite what's ruled out from typing #32309

Closed
alexpovel opened this issue Oct 2, 2023 · 12 comments
Assignees
Labels
Azure.Core Azure.Identity Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@alexpovel
Copy link
Contributor

  • Package Name: azure-identity (probably not relevant, error is in core?)
  • Package Version: 1.14.0
  • Operating System: Linux (Debian 12)
  • Python Version: 3.11.4

Describe the bug

Using ManagedIdentityCredential:

it at some point reaches out to actually get its token, reaching:

# pyright has trouble to understand that self.session is not None, since we raised at worst in the init
self.session = cast(aiohttp.ClientSession, self.session)
await self.session.__aenter__()

at which point it blows up, as self.session is None in my case. The impossibility ruled out on the type-level with a forced cast is possible after all at runtime, it looks like!

To Reproduce
Steps to reproduce the behavior:

  1. Use async version of ManagedIdentityCredential, provide credentials in some form

  2. Deploy to Container App Jobs (my Job has Managed Identity assigned: both user- as well as system-assigned raised the above issue)

  3. Observe stack trace:

      + Exception Group Traceback (most recent call last):
    
      |   File "<frozen runpy>", line 198, in _run_module_as_main
    
      |   File "<frozen runpy>", line 88, in _run_code
    
      |   File "/src/__main__.py", line 681, in <module>
      |     cli.APP()
    
      |   File "/src/__main__.py", line 660, in main_cli
      |     asyncio.run(
    
      |   File "/usr/local/lib/python3.11/asyncio/runners.py", line 190, in run
      |     return runner.run(main)
      |            ^^^^^^^^^^^^^^^^
    
      |   File "/usr/local/lib/python3.11/asyncio/runners.py", line 118, in run
      |     return self._loop.run_until_complete(task)
      |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
      |   File "/usr/local/lib/python3.11/asyncio/base_events.py", line 653, in run_until_complete
      |     return future.result()
      |            ^^^^^^^^^^^^^^^
    
      |   File "/src/__main__.py", line 186, in main_impl
      |     async with (
    
      |   File "/usr/local/lib/python3.11/asyncio/taskgroups.py", line 147, in __aexit__
      |     raise me from None
    
      | ExceptionGroup: unhandled errors in a TaskGroup (26 sub-exceptions)
    
      +-+---------------- 1 ----------------
    
        | Traceback (most recent call last):
    
        |   File "/src/storage/__init__.py", line 24, in store_and_unlink
        |     await self.store(path, on=None)
    
        |   File "/src/storage/azure/blob.py", line 108, in store
        |     await c.upload_blob(  # pyright: ignore[reportUnknownMemberType]
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/tracing/decorator_async.py", line 77, in wrapper_use_tracer
        |     return await func(*args, **kwargs)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/storage/blob/aio/_blob_client_async.py", line 416, in upload_blob
        |     return await upload_block_blob(**options)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/storage/blob/aio/_upload_helpers.py", line 82, in upload_block_blob
        |     response = await client.upload(
        |                ^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/tracing/decorator_async.py", line 77, in wrapper_use_tracer
        |     return await func(*args, **kwargs)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/storage/blob/_generated/aio/operations/_block_blob_operations.py", line 249, in upload
        |     pipeline_response: PipelineResponse = await self._client._pipeline.run(  # pylint: disable=protected-access
        |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 221, in run
        |     return await first_node.send(pipeline_request)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   [Previous line repeated 3 more times]
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/policies/_authentication_async.py", line 92, in send
        |     await await_result(self.on_request, request)
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_tools_async.py", line 58, in await_result
        |     return await result
        |            ^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/policies/_authentication_async.py", line 64, in on_request
        |     self._token = await await_result(self._credential.get_token, *self._scopes)
        |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_tools_async.py", line 58, in await_result
        |     return await result
        |            ^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_internal/decorators.py", line 21, in wrapper
        |     token = await fn(*args, **kwargs)
        |             ^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_credentials/managed_identity.py", line 141, in get_token
        |     return await self._credential.get_token(*scopes, claims=claims, tenant_id=tenant_id, **kwargs)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_internal/managed_identity_base.py", line 47, in get_token
        |     return await super().get_token(*scopes, claims=claims, tenant_id=tenant_id, **kwargs)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_internal/get_token_mixin.py", line 86, in get_token
        |     token = await self._request_token(*scopes, claims=claims, tenant_id=tenant_id, **kwargs)
        |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_internal/managed_identity_base.py", line 55, in _request_token
        |     return await cast(AsyncManagedIdentityClient, self._client).request_token(*scopes, **kwargs)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_internal/managed_identity_client.py", line 32, in request_token
        |     response = await self._pipeline.run(request, retry_on_methods=[request.method], **kwargs)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 221, in run
        |     return await first_node.send(pipeline_request)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   [Previous line repeated 1 more time]
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/policies/_retry_async.py", line 179, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   [Previous line repeated 1 more time]
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 106, in send
        |     await self._sender.send(request.http_request, **request.context.options),
        |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/transport/_aiohttp.py", line 231, in send
        |     await self.open()
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/transport/_aiohttp.py", line 127, in open
        |     await self.session.__aenter__()
        |           ^^^^^^^^^^^^^^^^^^^^^^^
    
        | AttributeError: 'NoneType' object has no attribute '__aenter__'
    
        +---------------- 2 ----------------
    
        | Traceback (most recent call last):
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/transport/_aiohttp.py", line 263, in send
        |     result = await self.session.request(  # type: ignore
        |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/aiohttp/client.py", line 560, in _request
        |     await resp.start(conn)
    
        |   File "/usr/local/lib/python3.11/site-packages/aiohttp/client_reqrep.py", line 899, in start
        |     message, payload = await protocol.read()  # type: ignore[union-attr]
        |                        ^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/aiohttp/streams.py", line 616, in read
        |     await self._waiter
    
        | aiohttp.client_exceptions.ServerDisconnectedError: Server disconnected
    
        | 
        | The above exception was the direct cause of the following exception:
        | 
    
        | Traceback (most recent call last):
    
        |   File "/src/storage/__init__.py", line 24, in store_and_unlink
        |     await self.store(path, on=None)
    
        |   File "/src/storage/azure/blob.py", line 108, in store
        |     await c.upload_blob(  # pyright: ignore[reportUnknownMemberType]
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/tracing/decorator_async.py", line 77, in wrapper_use_tracer
        |     return await func(*args, **kwargs)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/storage/blob/aio/_blob_client_async.py", line 416, in upload_blob
        |     return await upload_block_blob(**options)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/storage/blob/aio/_upload_helpers.py", line 82, in upload_block_blob
        |     response = await client.upload(
        |                ^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/tracing/decorator_async.py", line 77, in wrapper_use_tracer
        |     return await func(*args, **kwargs)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/storage/blob/_generated/aio/operations/_block_blob_operations.py", line 249, in upload
        |     pipeline_response: PipelineResponse = await self._client._pipeline.run(  # pylint: disable=protected-access
        |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 221, in run
        |     return await first_node.send(pipeline_request)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   [Previous line repeated 3 more times]
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/policies/_authentication_async.py", line 92, in send
        |     await await_result(self.on_request, request)
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_tools_async.py", line 58, in await_result
        |     return await result
        |            ^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/policies/_authentication_async.py", line 64, in on_request
        |     self._token = await await_result(self._credential.get_token, *self._scopes)
        |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_tools_async.py", line 58, in await_result
        |     return await result
        |            ^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_internal/decorators.py", line 21, in wrapper
        |     token = await fn(*args, **kwargs)
        |             ^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_credentials/managed_identity.py", line 141, in get_token
        |     return await self._credential.get_token(*scopes, claims=claims, tenant_id=tenant_id, **kwargs)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_internal/managed_identity_base.py", line 47, in get_token
        |     return await super().get_token(*scopes, claims=claims, tenant_id=tenant_id, **kwargs)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_internal/get_token_mixin.py", line 86, in get_token
        |     token = await self._request_token(*scopes, claims=claims, tenant_id=tenant_id, **kwargs)
        |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_internal/managed_identity_base.py", line 55, in _request_token
        |     return await cast(AsyncManagedIdentityClient, self._client).request_token(*scopes, **kwargs)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/identity/aio/_internal/managed_identity_client.py", line 32, in request_token
        |     response = await self._pipeline.run(request, retry_on_methods=[request.method], **kwargs)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 221, in run
        |     return await first_node.send(pipeline_request)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   [Previous line repeated 1 more time]
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/policies/_retry_async.py", line 205, in send
        |     raise err
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/policies/_retry_async.py", line 179, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 69, in send
        |     response = await self.next.send(request)
        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   [Previous line repeated 1 more time]
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/_base_async.py", line 106, in send
        |     await self._sender.send(request.http_request, **request.context.options),
        |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
        |   File "/usr/local/lib/python3.11/site-packages/azure/core/pipeline/transport/_aiohttp.py", line 300, in send
        |     raise ServiceRequestError(err, error=err) from err
    
        | azure.core.exceptions.ServiceRequestError: Server disconnected
    

Expected behavior

self.session is never None at that point, so entering the async context manager works, so authentication succeeds. If there's a session issue, it should be raised earlier (as the comment suggests).

Additional context

  • The exception group above suggests there might be an issue with managed identities in Container App Jobs? We also get azure.core.exceptions.ServiceRequestError: Server disconnected. Nevertheless, that's a different issue and I'll open a separate ticket. I reckon given this edge case, the typing and None checks should still be sound and not cause a runtime failure.
  • In my code, you can see await c.upload_blob( # pyright: ignore[reportUnknownMemberType]. c is the result of async with azure.storage.blob.aio.BlobClient(...) as c. This passes mypy, so while my code also has a typing issue I am force-ignoring, I hope it's not the cause (my code works in principle, just not when deploying to Container App Jobs).
@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-triage Workflow: This issue needs the team to triage. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Storage Storage Service (Queues, Blobs, Files) labels Oct 2, 2023
@alexpovel
Copy link
Contributor Author

alexpovel commented Oct 2, 2023

So the Server disconnected part is quite possibly related to an already existing issue: microsoft/azure-container-apps#864 . Nothing the Python SDK can solve, but on the typing level there shouldn't be a hard cast if edge cases can still trigger the None branch?

EDIT: Although I'm unclear why in the above stack trace it mentions Server disconnected, whereas over in the other issue it mentions, much more specifically, ManagedIdentityCredential: Cannot connect to host localhost:42356 ssl:True [Connect call failed ('::1', 42356, 0, 0)]".

@mccoyp
Copy link
Member

mccoyp commented Oct 2, 2023

Hi @alexpovel, thank you for opening such a detailed issue and investigating the scenario! I'll tag some folks who can take a look and get back to you as soon as possible.

For assigned folks: requesting a look from both Identity and Core since the issue is raised in the latter but associated with the former.

@mccoyp mccoyp added Azure.Core Azure.Identity and removed Storage Storage Service (Queues, Blobs, Files) needs-team-triage Workflow: This issue needs the team to triage. labels Oct 2, 2023
@github-actions github-actions bot added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Oct 2, 2023
@kashifkhan
Copy link
Member

Hi @alexpovel ,

had a few follow up questions for you:

  • Does this issue still surface when you go downgrade azure-core to azure-core 1.29.0 ?
  • Are you by any chance sending in a sending in your own session and setting session_owner = True ?

@alexpovel
Copy link
Contributor Author

Hi,

thanks for getting back to me.

  • I actually don't know which core version I was using: I am using it as a transient dependency. If needed, I can take a look at poetry.lock at get back to you. But more importantly, I cannot downgrade it, sorry. I implemented a workaround now (exponential backoff and retry on authentication error) and don't want to open that can of worms again.
  • the kwargs session and session_owner I never used (in the context of this issue). They certainly aren't set on the objects that eventually ran into this error.

@joshfree joshfree moved this from Untriaged to Not Started in Azure Identity SDK Improvements Oct 9, 2023
@lmazuel
Copy link
Member

lmazuel commented Oct 9, 2023

Hi @alexpovel

We investigated the three of us @annatisch, @xiangyan99 and me, and concluded that you can end up in this scenario if your HTTP internal transport was closed, and then re-used a second time. This can be trigger by code like this for instance (pseudo-code):

transport = azure.core.pipeline.transport.AiohttpTransport()
async with StorageClient(endpoint, transport=transport) as client:
   client.do_something()
async with StorageClient(endpoint, transport=transport) as client:
   client.do_something()  # this will raise with your error

# or 

async with azure.identity.aio.ManagerIdentity() as credential:
   async with StorageClient(endpoint, credential) as client:
       client.do_something()

async with StorageClient(endpoint, credential) as client:
    client.do_something()  # this will raise with your error

You are probably not passing your own transport, so there may be a combination of clients embedded into each other with a context manager syntax that ends up in this situation, even if you didn't pass your own client.

As our credentials class own their own pipeline, did you use your credential instance several times in several clients maybe? With a context manager syntax?

Does any of that would make sense if you look at your app? If not, is there any way we can look into your code, to understand the combination of client creation you are doing? If you're willing to share your code, it doesn't have to be public (I can share my email).

Thanks,

@xiangyan99 xiangyan99 added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Oct 11, 2023
@github-actions github-actions bot removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Oct 11, 2023
@github-actions
Copy link

Hi @alexpovel. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@alexpovel
Copy link
Contributor Author

alexpovel commented Oct 16, 2023

Thanks for the swift feedback. Your analysis is spot-on. I stripped down my code to the bare essentials that should (not verified!) trigger the issue:

from azure.identity.aio import DefaultAzureCredential
from azure.storage.blob import BlobType
from azure.storage.blob.aio import BlobClient


class AzureBlobStorage:
    def __init__(
        self,
        storage_url: str,
        container_name: str,
        credential: DefaultAzureCredential | None = None,
    ) -> None:
        self.storage_url = storage_url
        self.container_name = container_name

        if credential is None:
            credential = DefaultAzureCredential()

        self.credential = credential

    async def store(self, name: str, data: bytes) -> None:
        async with self.credential as credential:
            async with BlobClient(
                str(self.storage_url),
                container_name=self.container_name,
                blob_name=name,
                #
                # https://github.com/Azure/azure-sdk-for-python/issues/27704:
                credential=credential,  # pyright: ignore[reportGeneralTypeIssues]
            ) as client:
                # No idea why Pylance/Pyright is throwing a fit here, `mypy` is happy.
                # Put on new line so we're below line length limit.
                await client.upload_blob(  # pyright: ignore[reportUnknownMemberType]
                    data=data,
                    blob_type=BlobType.BLOCKBLOB,
                )

AzureBlobStorage is used as a singleton, but store is used a ton.

As you can tell from the comments, typing this properly was challenging. mypy was satisfied, but pyright still wasn't, so I ignored it.

The optional context managers for the aio library are confusing to me. It's probably the source of the issue, as you mention:

  • BlobClient is used as a context manager to properly close its open connections once done

    (I have since used the session kwarg instead of a context manager as I was running into file descriptor limits due to too many open TCP connections; the kwarg was hard to discover ([Brainstorm] Make advanced kwargs discoverable #12122 , Azure SDKs for Python should support custom urllib3 connection pool size. #12102), but that's a different issue)

  • I figured it works the same for credential, aka closing dangling connections, e.g. when using managed identities with their chatter to auth endpoints. Seems even more important closing unmanaged (== OS-managed) resources properly when it comes to secrets!

    But this closes more than I intended I now realize. Can you confirm my use of credential is incorrect? The first call to store uses, then closes it, so all calls thereafter fail, as credential is shared state among all calls to store. <- not true, I am using credential as a context manager in working code, and even though there's just one object that gets closed after the first routine is done with it, it continues to work


Once my error is identified and fixed, I'd argue the code in question still isn't correct (although that's subjective to some degree):

# pyright has trouble to understand that self.session is not None, since we raised at worst in the init
self.session = cast(aiohttp.ClientSession, self.session)
await self.session.__aenter__()

This should probably be an assertion error or similar. A typing.cast isn't valid, as this situation is possible at runtime (as skillfully demonstrated by my accident 👀). Casts are to overcome type checker limitations, but nothing more. So perhaps an assert with a corresponding message. Benefits the type checker all the same (type narrowing), yet would've allowed me to debug this issue further myself (instead of a "cannot happen" comment, a "connection closed unexpectedly" error).

The only raising there is in the init is:

if not self._session_owner and not self.session:
raise ValueError("session_owner cannot be False if no session is provided")

but that covers a different case and can logically fall through with a None session making it into the instantiated object. Perhaps some code got out of sync (raise removed but code in open remained)?

@github-actions github-actions bot added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels Oct 16, 2023
@xiangyan99
Copy link
Member

Thanks for sharing the code.

async def store(self, name: str, data: bytes) -> None:
        async with self.credential as credential:
            async with BlobClient(
                str(self.storage_url),
                container_name=self.container_name,
                blob_name=name,
                #
                # https://github.com/Azure/azure-sdk-for-python/issues/27704:
                credential=credential,  # pyright: ignore[reportGeneralTypeIssues]
            ) as client:
                # No idea why Pylance/Pyright is throwing a fit here, `mypy` is happy.
                # Put on new line so we're below line length limit.
                await client.upload_blob(  # pyright: ignore[reportUnknownMemberType]
                    data=data,
                    blob_type=BlobType.BLOCKBLOB,
                )

Here you use context manager for the credential. Every time you run it, the exist of async with self.credential as credential: calls credential.close(). Hence the second time raises the session is None problem.

If you need to call the store method multiple times, you need to keep the credential open.

@xiangyan99 xiangyan99 added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Oct 17, 2023
@github-actions github-actions bot removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Oct 17, 2023
@github-actions
Copy link

Hi @alexpovel. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

@github-actions
Copy link

Hi @alexpovel, since you haven’t asked that we /unresolve the issue, we’ll close this out. If you believe further discussion is needed, please add a comment /unresolve to reopen the issue.

@marrobi
Copy link

marrobi commented Dec 21, 2023

I'm hitting this too, similar situation, but with CosmosClient. I was using context manger for the credentials like:

     async with credentials.get_credential_async() as credential:
        if MANAGED_IDENTITY_CLIENT_ID:
           logger.debug("Connecting with managed identity")
           cosmos_client = CosmosClient(
               url=STATE_STORE_ENDPOINT,
               credential=credential
           )

I've switched this to:

    credential = get_credential()
    if MANAGED_IDENTITY_CLIENT_ID:
        logger.debug("Connecting with managed identity")
        cosmos_client = CosmosClient(
            url=STATE_STORE_ENDPOINT,
            credential=credential
        )

But I still find I get an exception on this line on some calls when reusing the cosmos_client.

  File "/usr/local/lib/python3.8/site-packages/azure/core/pipeline/transport/_aiohttp.py", line 141, in open
    await self.session.__aenter__()
AttributeError: 'NoneType' object has no attribute '__aenter__'

We are storing the client in application state,, hence I can see how the credential will go out of context, this was working when using a master key, but now we have moved to MSI are hitting this issue. Can you provide an example as to how to "keep the credential open"?

Thank you.

@marrobi
Copy link

marrobi commented Dec 21, 2023

Actually I see the same behaviour when using a key, so not down to credentials is our case:

    async with cosmos_client:
  File "/usr/local/lib/python3.8/site-packages/azure/cosmos/aio/_cosmos_client.py", line 168, in __aenter__
    await self.client_connection.pipeline_client.__aenter__()
  File "/usr/local/lib/python3.8/site-packages/azure/core/_pipeline_client_async.py", line 169, in __aenter__
    await self._pipeline.__aenter__()
  File "/usr/local/lib/python3.8/site-packages/azure/core/pipeline/_base_async.py", line 157, in __aenter__
    await self._transport.__aenter__()
  File "/usr/local/lib/python3.8/site-packages/azure/core/pipeline/transport/_aiohttp.py", line 116, in __aenter__
    await self.open()
  File "/usr/local/lib/python3.8/site-packages/azure/core/pipeline/transport/_aiohttp.py", line 141, in open
    await self.session.__aenter__()

When it fails client_connection.session is set to None.

We are using azure-core==1.29.5

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Azure.Identity Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
Development

No branches or pull requests

8 participants