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

Feature request: asyncio support #327

Closed
stephenmuss opened this issue Jun 12, 2017 · 57 comments
Closed

Feature request: asyncio support #327

stephenmuss opened this issue Jun 12, 2017 · 57 comments
Labels

Comments

@stephenmuss
Copy link

stephenmuss commented Jun 12, 2017

Hi folks,

I was wondering what it would take to get this library to support asyncio. This would mean it would be possible to use Python 3.5+ async/await coroutines with a number of async frameworks.

Looking at the code it would be possible to create a new Http Client with say aiohttp, but it looks like the code would need to be modified in other places in order to support it.

Has the Stripe team considered async support?

@brandur-stripe
Copy link
Contributor

Hi Stephen,

I really like this idea!

The bad news is that I don't know if we're going to be able to do it in the immediate future :/ The library is currently primarily targets Python 2, and we use "2to3" for compatibility across both major versions. I just ran some analytics against the Stripe platform, and requests from Python 2 outnumber requests from Python 3 by a full 10 to 1, so for the time being, this setup still makes sense for the project.

It might be possible to invert the primary major version by using something like "3to2" instead, but I suspect that it might not support really fancy Python 3+ constructs like async/await. Also, for now, the asyncio docs seem to suggest avoiding the package if wide version support is necessary:

The async def type of coroutine was added in Python 3.5, and is recommended if there is no need to support older Python versions.

Given that Python's 2 to 3 chasm seems to be a problem that might never be solved, I suspect that a more plausible way to get this in would be start maintaining a separate Python 3 fork that can leverage the richer features of 3+. Unfortunately, I can't promise this anytime soon because we'd really need to ramp up out internal language teams to make it happen.

Even so, I'm going to leave this open and tag it as "future", and hopefully it'll become tractable given a little more time.

@FranzPoize
Copy link

Any news on this ?

@ob-stripe
Copy link
Contributor

@FranzPoize @brandur-stripe's comment above is still largely valid. While we no longer use 2to3, we still need to support Python 2 for the foreseeable future (more than half of Stripe's Python users are on 2.7), and we currently don't have the bandwidth to start a new Py3-only library.

@glyph
Copy link

glyph commented Jun 26, 2018

Just food for thought: one way to make asyncio-compatible code with python 2 support is to support Twisted :).

@nathanielcompton
Copy link

Leaving a +1 on this issue to further raise awareness.

Now that we are within 18 months of Python 2's end of lifecycle support, I imagine there will be an ever-increasing trend of production-ready stripe-python implementations switching from Python 2 to 3.

@perpetual-hydrofoil
Copy link

https://pypi.org/project/asyncio_stripe/

@adeadman
Copy link

Hi all, given Python 2's end of life in January 2020, is there any intention to add official support for async Python and await coroutines to the Stripe library in the near future?

@ob-stripe
Copy link
Contributor

Hi @adeadman. We will likely keep supporting Python 2 for some time after the EOL deadline, depending on how quickly our userbase migrates to Python 3.

That said, we're considering shipping a separate Python module stripe-async that would only support Python 3. We've recently started generating parts of our client libraries code directly from the OpenAPI specification file for Stripe's API, so we could realistically maintain two different libraries for Python. We haven't reached a decision yet, but I'll make sure to post an update here when we do.

@adeadman
Copy link

That's great to hear, an officially maintained library would be preferable to us than contributing to an unofficial one.

How do you collect statistics on the python version your userbase are running? I know we've been python 3 for the last 2 years. The risk is that people who are still running py2 may not even be aware of the end of life - would a deprecation notice be displayed either in their application logs or in the Stripe dashboard?

@ob-stripe
Copy link
Contributor

How do you collect statistics on the python version your userbase are running?

Requests sent from our official client libraries include a X-Stripe-Client-User-Agent HTTP header containing some information about the environment such as the names and versions for the OS and runtime, etc. We use this for both aggregate statistics and individual support.

At the moment, ~35% of stripe-python users are still running Python 2. It's slowly declining, and I'm hoping there will be a sharper decrease once we reach the EOL deadline 🤞

The risk is that people who are still running py2 may not even be aware of the end of life - would a deprecation notice be displayed either in their application logs or in the Stripe dashboard?

Yes! This is also something we've been talking about. There should be some form of notice in the dashboard to inform users and encourage them to upgrade sooner rather than later.

@rubik
Copy link

rubik commented Jan 20, 2020

Any updates on this issue? For us being able to send requests asynchronously is quite critical. Without support from the library one is forced to use ugly workarounds like running the stripe calls in a separate executor.

@ob-stripe
Copy link
Contributor

Hi @rubik. As I mentioned in a previous message, we're leaning towards shipping a separate library (both because we'll probably need to support Python 2.7 for some time past the EOL, and because Python's async model does not make it easy to ship sync and async code side by side).

No ETA yet, but I'll see if this is something we can prioritize this half.

@arlyon
Copy link

arlyon commented Jun 26, 2020

I understand the need to support people running legacy versions from a business perspective but it is disappointing that EOL software is preventing progress (this is not a criticism of stripe directly, more a general commentary on the 2-vs-3 situation). Running in its own thread executor is enough for my simple use case for now... thanks for the lib!

@remi-stripe
Copy link
Contributor

@arlyon Thanks a lot for the feedback and we agree it'd be great to support this. The change is a pretty big paradigm change though and so doing a separate library makes more sense I think as we can also plan adding support for types and pinning to a specific API version. We're looking into this internally to see whether this is work we could prioritize this year!

@WardPearce
Copy link

Any new updates since 5 months ago?
I'm tempted to make my own more modern async and sync wrapper if not.

@remi-stripe
Copy link
Contributor

@WardPearce No progress just yet unfortunately. We focused on doing code generation for stripe-dotnet which is now complete. We're now improving stripe-java, with hope to add the new client/service infrastructure to it first. While adding async support to stripe-python is something we want to do, it'll likely take a while before we get to it unfortunately.

@WardPearce
Copy link

I got this 😎

@WardPearce
Copy link

I'm starting development over @ https://github.com/WardPearce/stripe3

Feel free to open some PRs or submit requests via issues. Pretty early stages development, development will start to speed up after the 13th.

@jleclanche
Copy link
Contributor

@WardPearce can I request that you keep #689 in mind if you're developing something from scratch? :)

@part1cleth1ef
Copy link

hmmm, i seem to have gotten most of this current lib working using the loop.run_in_executor(None, functools.partial(func, args)) but i seem to not be able to verify webhook requests, any idea why? i dont mind sending info as its still in the testmode part of stripe.

@remi-stripe
Copy link
Contributor

@part1cleth1ef I'd recommend talking to our support team for help instead: https://support.stripe.com/contact/email

@bhch
Copy link

bhch commented Jun 4, 2021

Hi, if anyone's interested, I've created an async wrapper for stripe: https://github.com/bhch/async-stripe.

@part1cleth1ef
Copy link

@remi-stripe nvm, i fixed it, it turned out that because i was using aiohttp.web instead of flask, i used the wrong method, i fed it json instead of a string which it apparently wanted. fixed the issue now lmao

@WardPearce
Copy link

I've been busy working on like 3 other projects, I will eventually get my wrapper done. It will be completely different to the current design stripe uses. Aiming to be more developer friendly.

Will be similar to my b2 wrapper https://github.com/WardPearce/backblaze

@lacasaprivata2
Copy link

lacasaprivata2 commented Oct 4, 2021

between stripe, square, et. al, it seems this is going to take years for adoption - one of the primary difficulties in async for large cos. whom consume stripe et. al is the chicken in the egg problem: a handful of third party libraries support async, and you need all libraries to support async in order to have a seamless transition.

thank you for the wrappers above - think this will be best and a separate github repo likely needed

one place to start could also be to create an official rust api client w/ async & auto generate python wrappers

@WardPearce
Copy link

WardPearce commented Oct 5, 2021

If python 2 support could be removed from stripe-python, porting to async would basically just mean porting the http client to something like HTTPX (ofc thats a lot easier said then done).

Personally I'd take the project on myself, but I've been busy recently & stripe is a massive API

@abdullahmujahidali
Copy link

Any update?

@tsalpekar21
Copy link

  • the beta channel

@richardm-stripe Thank you so much for adding this feature in the beta channel! This feature will be immensely helpful in my current project.

I recently discovered that synchronous methods in our ASGI server are causing the server to be blocked (the event loop effectively stalls), so I decided to begin implementing raw_request_async to see how far I could get.

From my implementation with raw_request_async, I found that error handling for traditionally non-retryable requests don't work as expected. For instance, I wouldn't expect a 400 to trigger a retry, but the SDK retries that status code (our test suite ensures we handle requests for deleted customers). Additionally, 400's were caught in my exception handler as an ApiConnectionError object - I'd expect this to surface as a StripeError or InvalidRequestError.

Lmk if any of these assumptions I'm making are incorrect. These are issues I'm seeing in the diff between my current synchronous implementation and this new request. Unrelated - I'm excited for this PR of yours to land as well!

And again, thank you for adding this into the official Stripe SDK 👍

@Zac-HD
Copy link

Zac-HD commented Jan 24, 2024

Exciting to see that this is moving 😃

I tried it out, and unfortunately the single use of asyncio.sleep() means that we can't use it at work - everything is based on Trio. You could easily fix that by using the anyio compatibility library, literally anyio.sleep(), which runs on top of either Asyncio or Trio and supports best practices like asyncio.TaskGroup even on older versions of Python. It's not even adding a dependency, because you already indirectly depend on anyio via httpx!

It would additionally be fantastic to run your tests against both backends, which anyio makes remarkably easy, to ensure that everything stays compatible with the taskgroup idioms which are required by Trio and recommended by asyncio for new Python versions. @richardm-stripe if writing PRs would help make this happen, please let me know!

@richardm-stripe
Copy link
Contributor

Thank you @tsalpekar21 for trying out the beta! I've reproduced the "raw_request_async doesn't do retries or produce the correct errors" bug and have a fix in #1209.

@richardm-stripe
Copy link
Contributor

Thank you also @Zac-HD for trying out the beta, and for pointing out that HTTPX already introduces a dependency on anyio. I'm going to attempt something like this #1210 -- (we aren't going to officially register HTTPX or AnyIO as a dependency, but we'll check to see if it's there and use it if so), and I'm working on trying to set up the anyio test fixture.

PRs are certainly welcome, but I am actively working on this project again, so the best way you can help is to continue providing feedback and guidance in comments on this PR, as you have been.

@richardm-stripe
Copy link
Contributor

As a general update, I'm aiming for #1171 to be in next week's beta release, so it will be possible to try out async with the "real" SDK methods, instead of just .raw_request.

@Zac-HD
Copy link

Zac-HD commented Jan 26, 2024

Sounds great!

I do think it's important to run some integration tests using Trio though - the anyio.sleep() makes it possible, but the code 'above' this also needs to use a design compatible with structured concurrency (shorter intro: 9:30-12:50 of my PyCon talk). It seems unlikely to me that there would be any such problems within an HTTPClientAsync, but I can imagine future work introducing some concurrency further up in the SDK, which turns out to work on asyncio (including with the anyio sleep passthrough), but not if the app is executed via anyio.run() or trio.run().

At that point I'd personally accept a hard dependency on anyio - correct error handling gets easier, and as a less-direct customer benefit @pytest.mark.asyncio makes it trivial to run all your async tests against multiple backends. But of course it's your call, and I don't think you need to make it now.

@richardm-stripe
Copy link
Contributor

richardm-stripe commented Feb 6, 2024

Hello async awaiters!

Recently released to beta

https://github.com/stripe/stripe-python/releases/tag/v8.2.0b1 contains several async-related updates to the async support in the beta channel, notably code-generated definitions for resource methods -- i.e. now you can await stripe.PaymentIntent.create(...). It also contains fixes for trio/anyio and more comprehensive testing.

What's still pending:

  • There is no support yet for supporting asynchronous methods off the new StripeClient interface introduced in v8.0.0.
  • There is no support for "streaming" methods like stripe.Quote.pdf(...)
  • HTTPClientAsync might need redesigning.

Discussion questions / How you can help

  • We are interested to hear about any kind of experience anybody has trying out the new async support.
  • We would be particularly grateful to any user willing to try out writing/using a custom implementation of HTTPClientAsync and providing feedback. (If your application uses an asynchronous HTTP client other than HTTPX, this can be you!).
  • Feedback on customizing HTTPClient/HTTPClientAsync? - While we're iterating on HTTPClient we're generally interested any feedback or suggestions from users who aren't using a different HTTP library, but still end up needing to implement their own subclasses of HTTPClient/HTTPClient async because they need to customize the options that are used, or have another advanced use case that requires customization.
  • Should HTTPClientAsync and HTTPClient be separate classes? This is something we are discussing internally and would appreciate any thoughts by anybody on this thread. Right now, it's kind of weird that HTTPX is only supported for making async requests, and you can't use HTTPX for making sync requests (unless you implement it yourself). We're considering changing the implementation so, instead of HTTPClientAsync and HTTPClient being two separate classes, and stripe.default_http_client vs. stripe.default_http_client_async as two separate properties that you set, there would be only a single HTTPClient class with both sync and async methods defined. We're not willing to change away from requests being the default http client made for synchronous requests, though, so the default stripe.default_http_client would be some sort of mash-up between requests and httpx.

Thanks everybody for your patience and willingness to provide feedback so far!

@aneeshusa
Copy link

(I work with Zac) we updated to the beta and it has been running smoothly! Thanks for working on this.

@AstreaTSS
Copy link

AstreaTSS commented Feb 9, 2024

At least for aiohttp, making a custom implementation of HTTPClientAsync was extremely easy. You can see the results below in a Gist for anyone curious, but I'm pretty happy with how the base class was designed - its design results in practically a copy of HTTPXClient besides for a few line changes.

https://gist.github.com/AstreaTSS/b87fc66a68f48e95a59dd951b49f9cdc

(Note that I've only done basic tests on it, though I will note one thing - the client should be initialized inside of a task, IE in an async startup event function. This isn't exclusive to aiohttp but can catch you off-guard.)

EDIT: Edited for 8.4.0b1.

@Padge91
Copy link

Padge91 commented Feb 12, 2024

I started trying out some of the async support last week, and I ran into an issue when trying to run tests with unittest.IsolatedAsyncioTestCase. It seems like it may be related to the async context of httpx.

Python version: 3.10.11
Stripe version: v8.2.0b1
httpx version: 0.26.0
OS: macOS Ventura 13.2.1 (but also happens in an ubuntu container)

Code to reproduce:

import os
import unittest

import stripe
stripe.api_key = os.environ.get("STRIPE_API_KEY")


class TestBug(unittest.IsolatedAsyncioTestCase):
    async def test_bug_a(self):
        # this will succeed
        customer = await stripe.Customer.create_async()

    async def test_bug_b(self):
        # this will fail
        customer = await stripe.Customer.create_async()


if __name__ == "__main__":
    unittest.main()

Stack trace:

Traceback (most recent call last):
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/anyio/streams/tls.py", line 131, in _call_sslobject_method
    result = func(*args)
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/ssl.py", line 917, in read
    v = self._sslobj.read(len)
ssl.SSLWantReadError: The operation did not complete (read) (_ssl.c:2578)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_async/http11.py", line 111, in handle_async_request
    ) = await self._receive_response_headers(**kwargs)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_async/http11.py", line 176, in _receive_response_headers
    event = await self._receive_event(timeout=timeout)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_async/http11.py", line 212, in _receive_event
    data = await self._network_stream.read(
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_backends/anyio.py", line 34, in read
    return await self._stream.receive(max_bytes=max_bytes)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/anyio/streams/tls.py", line 196, in receive
    data = await self._call_sslobject_method(self._ssl_object.read, max_bytes)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/anyio/streams/tls.py", line 138, in _call_sslobject_method
    data = await self.transport_stream.receive()
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/anyio/_backends/_asyncio.py", line 1202, in receive
    self._transport.resume_reading()
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/selector_events.py", line 814, in resume_reading
    self._add_reader(self._sock_fd, self._read_ready)
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/selector_events.py", line 760, in _add_reader
    self._loop._add_reader(fd, callback, *args)
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/selector_events.py", line 253, in _add_reader
    self._check_closed()
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/base_events.py", line 515, in _check_closed
    raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/stripe/_http_client.py", line 1026, in request_async
    response = await self._client.request(
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpx/_client.py", line 1559, in request
    return await self.send(request, auth=auth, follow_redirects=follow_redirects)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpx/_client.py", line 1646, in send
    response = await self._send_handling_auth(
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpx/_client.py", line 1674, in _send_handling_auth
    response = await self._send_handling_redirects(
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpx/_client.py", line 1711, in _send_handling_redirects
    response = await self._send_single_request(request)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpx/_client.py", line 1748, in _send_single_request
    response = await transport.handle_async_request(request)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpx/_transports/default.py", line 371, in handle_async_request
    resp = await self._pool.handle_async_request(req)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_async/connection_pool.py", line 268, in handle_async_request
    raise exc
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_async/connection_pool.py", line 251, in handle_async_request
    response = await connection.handle_async_request(request)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_async/connection.py", line 103, in handle_async_request
    return await self._connection.handle_async_request(request)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_async/http11.py", line 132, in handle_async_request
    await self._response_closed()
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_async/http11.py", line 245, in _response_closed
    await self.aclose()
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_async/http11.py", line 253, in aclose
    await self._network_stream.aclose()
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/httpcore/_backends/anyio.py", line 54, in aclose
    await self._stream.aclose()
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/anyio/streams/tls.py", line 193, in aclose
    await self.transport_stream.aclose()
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/anyio/_backends/_asyncio.py", line 1261, in aclose
    self._transport.close()
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/selector_events.py", line 706, in close
    self._loop.call_soon(self._call_connection_lost, None)
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/base_events.py", line 753, in call_soon
    self._check_closed()
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/base_events.py", line 515, in _check_closed
    raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/async_case.py", line 64, in _callTestMethod
    self._callMaybeAsync(method)
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/async_case.py", line 87, in _callMaybeAsync
    return self._asyncioTestLoop.run_until_complete(fut)
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/base_events.py", line 649, in run_until_complete
    return future.result()
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/async_case.py", line 101, in _asyncioLoopRunner
    ret = await awaitable
  File "/Users/demo/.../test_bug.py", line 20, in test_bug_b
    customer_2 = await bug_function()
  File "/Users/demo/.../test_bug.py", line 9, in bug_function
    customer = await stripe.Customer.create_async()
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/stripe/_customer.py", line 1433, in create_async
    await cls._static_request_async(
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/stripe/_api_resource.py", line 197, in _static_request_async
    return await _APIRequestor._global_instance().request_async(
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/stripe/_api_requestor.py", line 220, in request_async
    rbody, rcode, rheaders = await requestor.request_raw_async(
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/stripe/_api_requestor.py", line 767, in request_raw_async
    ) = await self._get_http_client_async().request_with_retries_async(
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/stripe/_http_client.py", line 396, in request_with_retries_async
    return await self._request_with_retries_internal_async(
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/stripe/_http_client.py", line 491, in _request_with_retries_internal_async
    raise connection_error
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/stripe/_http_client.py", line 456, in _request_with_retries_internal_async
    response = await self.request_async(
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/stripe/_http_client.py", line 1030, in request_async
    self._handle_request_error(e)
  File "/Users/demo/Library/Caches/pypoetry/virtualenvs/demo-iqP4ihb8-py3.10/lib/python3.10/site-packages/stripe/_http_client.py", line 1046, in _handle_request_error
    raise APIConnectionError(msg, should_retry=should_retry)
stripe._error.APIConnectionError: Unexpected error communicating with Stripe. If this problem persists,
let us know at support@stripe.com.

(Network error: A RuntimeError was raised)

The function causing the error appears to be the second test function. If I move the two stripe statements to the same test, it works as expected. Hope that helps!

@OldSneerJaw
Copy link
Contributor

It's great to see async support being added to the official Python SDK. I for one would like to have it include a built in aiohttp client implementation, though.

@richardm-stripe
Copy link
Contributor

Hello @Padge91, thanks for describing your experiences with unittest.IsolatedAsyncioTestCase. I think you are right that the stripe global has inherited this behavior from httpx, e.g. this will produce the same error:

httpx_client = httpx.AsyncClient()
class TestBug(unittest.IsolatedAsyncioTestCase):
    async def test_bug_a(self):
        await httpx_client.get(...))

    async def test_bug_b(self):
        await httpx_client.get(...))

What you can do to avoid it is

    async def asyncSetUp(self):
        # This works in the latest beta release
        stripe.default_http_client = stripe.HTTPXClient()

    async def test_bug_a(self):
        await stripe.Customer.create_async()

    async def test_bug_b(self):
        await stripe.Customer.create_async()

Did you have anything particular in mind to make this experience nicer?

Something I learned is that this is caused by HTTPX's connection pool. If you

httpx_client = httpx.AsyncClient()
class TestBug(unittest.IsolatedAsyncioTestCase):
    async def test_bug_a(self):
        await httpx_client.get(..., headers={"Connection": "close"}))

    async def test_bug_b(self):
        await httpx_client.get(..., headers={"Connection": "close"}))

it won't use the connection pool, and this test will actually succeed.

Connection pooling is useful because it helps re-use resources across multiple requests made with the same client, but something I'm considering is disabling connection pooling by default in the automatically populated stripe.default_http_client, but enabling it by default for anybody who constructs stripe.HTTPXClient manually.

@richardm-stripe
Copy link
Contributor

richardm-stripe commented Feb 23, 2024

Hello async awaiters,

Updates

The async support has had two changes since the last time I posted an update, available in the latest beta release v8.5.0b1

This latter change you can probably address by making changes after these patterns.

- stripe.default_http_client_async = ...`
+ stripe.default_http_client = ...

- class MyCustomAsyncHttpClient(stripe.HTTPClientAsync):
-   ...

+ class MyCustomAsyncHttpClient(stripe.HTTPClient):
+   ...

Although if you are making synchronous requests in your integration still and want to leave those unchanged and don't want to direct those to the same http client as your asynchronous requests, you might have to do a little more work, see the PR description and the "async_fallback_client" option.

What's next?

  • Will look into bundling an aiohttp option with the library.
  • We're considering "collapsing" .auto_paging_iter_async into .auto_paging_iter.
  • Will look into disabling HTTPX's connection pool in some circumstances, as I replied to @Padge91 above. (Update: we opted not to do this for now. We'll leave it as it is, or attempt to introduce per-event-loop connection pools if there are too many more reports of users getting struck by this)

How you can help

  • Leave any feedback on the new interface now HTTPClientAsync has been merged into HTTPClient.
  • Some discussion questions:
    • In this PR we are toying with the idea of stripe.default_http_client = stripe.HTTPXClient(raise_error_on_sync_requests=True) so that a user who wanted to migrate completely to async could enable this option and be more confident that they didn't miss migrating any sync methods to async. Would this be useful?
    • I suspect that HTTPClient and StripeClient should be context managers/async context managers. Anybody have any thoughts/advice in that realm?

Thank you again to everybody on this thread for your patience, the excellent feedback so far, and helping develop this feature!

@Zac-HD
Copy link

Zac-HD commented Feb 23, 2024

  • raise_error_on_sync_requests=True would be great for us 🙏
    I guess for the few people who are all-in on async it would be even nicer to get the type-annotation benefits of an async-only client, but overall it seems like the single-client-type approach is much easier to implement and makes migrating easier too.
  • What setup/teardown logic do the client classes need to provide? Does it typically match some syntactic scope? If yes and yes, I'd reach for a context manager.
    Use an async cm if and only if you really need to wait during cleanup, and then see AsyncResource docs (same in anyio) for the cleanup semantics - you can await something for graceful shutdown, but must still shut down / release resources / whatever if cancelled or otherwise interrupted. Failing to do so a fairly common, subtle, and really annoying bug in my experience.

And alongside all of that, thanks! We really appreciate the thought and care going into this, and it shows 🙂

@Padge91
Copy link

Padge91 commented Feb 23, 2024

@richardm-stripe Thanks for response (and all your work on this), your workaround should be sufficient. I don't see a simpler way at this moment. I must have missed #2959 in my initial research, as it seems to be this exact issue.

Connection pooling is useful because it helps re-use resources across multiple requests made with the same client, but something I'm considering is disabling connection pooling by default in the automatically populated stripe.default_http_client, but enabling it by default for anybody who constructs stripe.HTTPXClient manually.

That seems reasonable to me. Otherwise, as the referenced issue in httpx suggests, repeated asyncio.run, asyncio.gather, etc calls would trigger this behavior too. In my case, I would need to disable connection pooling virtually every time I use the async functions. Making it the default seems less error-prone to me.

@richardm-stripe
Copy link
Contributor

Updates

Just cut a new release that

  • Adds stripe.AIOHTTPClient() (cc @OldSneerJaw)
  • Allows you to disable sync methods and cause them to raise an exception. This happens by default if you stripe.default_http_client = stripe.HTTPXClient() and you can pass allow_sync_methods=True in order to disable it.
  • Collapses .auto_paging_iter_async() into an .auto_paging_iter() that implements both AsyncIterator as well as Iterator. So to migrate to async you just have to change for into async for - you don't have to remember the async suffix on both the .list method and the .auto_paging_iter method.

With this release, we are considering async support to be "feature complete". We're leaving it in the beta branch for awhile, because we still want to reserve the right to change the interface breakingly if a problem is discovered, but things should be completely ready for use. So if something is missing that prevents you from migrating, this isn't just "the feature is incomplete", it's probably a bug. Please continue to leave feedback.

How you can help

  • Migrate your integration. Leave any feedback you have!

@OldSneerJaw
Copy link
Contributor

OldSneerJaw commented Feb 27, 2024

@richardm-stripe: Thanks for adding aiohttp support! However, I've found that I encounter the following error when I try to use stripe.default_http_client = stripe.AIOHTTPClient():

[ERROR] Unexpected error communicating with Stripe. If this problem persists,
let us know at support@stripe.com.

(Network error: A ClientConnectorCertificateError was raised)
Traceback (most recent call last):
  File "/root/.cache/pypoetry/virtualenvs/app-9TtSrW0h-py3.11/lib/python3.11/site-packages/aiohttp/connector.py", line 992, in _wrap_create_connection
    return await self._loop.create_connection(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "uvloop/loop.pyx", line 2084, in create_connection
  File "uvloop/loop.pyx", line 2079, in uvloop.loop.Loop.create_connection
  File "uvloop/sslproto.pyx", line 517, in uvloop.loop.SSLProtocol._on_handshake_complete
  File "uvloop/sslproto.pyx", line 499, in uvloop.loop.SSLProtocol._do_handshake
  File "/usr/local/lib/python3.11/ssl.py", line 979, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1006)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/root/.cache/pypoetry/virtualenvs/app-9TtSrW0h-py3.11/lib/python3.11/site-packages/stripe/_http_client.py", line 1475, in request_stream_async
    response = await self._session.request(*args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/.cache/pypoetry/virtualenvs/app-9TtSrW0h-py3.11/lib/python3.11/site-packages/aiohttp/client.py", line 578, in _request
    conn = await self._connector.connect(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/.cache/pypoetry/virtualenvs/app-9TtSrW0h-py3.11/lib/python3.11/site-packages/aiohttp/connector.py", line 544, in connect
    proto = await self._create_connection(req, traces, timeout)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/.cache/pypoetry/virtualenvs/app-9TtSrW0h-py3.11/lib/python3.11/site-packages/aiohttp/connector.py", line 911, in _create_connection
    _, proto = await self._create_direct_connection(req, traces, timeout)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/.cache/pypoetry/virtualenvs/app-9TtSrW0h-py3.11/lib/python3.11/site-packages/aiohttp/connector.py", line 1235, in _create_direct_connection
    raise last_exc
  File "/root/.cache/pypoetry/virtualenvs/app-9TtSrW0h-py3.11/lib/python3.11/site-packages/aiohttp/connector.py", line 1204, in _create_direct_connection
    transp, proto = await self._wrap_create_connection(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/.cache/pypoetry/virtualenvs/app-9TtSrW0h-py3.11/lib/python3.11/site-packages/aiohttp/connector.py", line 994, in _wrap_create_connection
    raise ClientConnectorCertificateError(req.connection_key, exc) from exc
aiohttp.client_exceptions.ClientConnectorCertificateError: Cannot connect to host api.stripe.com:443 ssl:True [SSLCertVerificationError: (1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1006)')]

When I saw the SSL cert verification error, I tried again with stripe.AIOHTTPClient(verify_ssl_certs=False) and it worked fine, but that's not really a good solution in production. I suspect the issue is in AIOHTTPClient's __init__; specifically, I believe that, since stripe.ca_bundle_path is a file path, the SSL context should be created instead with ssl.create_default_context(cafile=stripe.ca_bundle_path) (i.e. it should use cafile= instead of capath= because the capath param is meant to receive a directory path).

As an aside, I think that, for parity with the other HTTPClient implementations, AIOHTTPClient's __init__ should include a branch for the case where verify_ssl_certs=False. Right now, aiohttp falls back to using the OS's certificate bundle, which does not seem to be the intended behaviour when verify_ssl_certs is disabled. I suggest something like this, for example:

if self._verify_ssl_certs:
    ssl_context = ssl.create_default_context(cafile=stripe.ca_bundle_path)
    kwargs["connector"] = TCPConnector(ssl=ssl_context)
else:
    kwargs["connector"] = TCPConnector(verify_ssl=False)

@richardm-stripe
Copy link
Contributor

richardm-stripe commented Feb 28, 2024

Thank you for the report and investigation @OldSneerJaw! 🤦 really jumped the gun on announcing "feature complete".
Your described fix is in #1262 and I'll try and release that as soon as I can.

@OldSneerJaw
Copy link
Contributor

@richardm-stripe: BTW, I submitted a potential fix that should make it so that AIOHTTPClient does not need to be instantiated with an event loop running (e.g. if one wants to set stripe.default_http_client at the top level of a module): #1271.

@richardm-stripe
Copy link
Contributor

We released the async support to the stable channel today in https://github.com/stripe/stripe-python/releases/tag/v8.10.0 (#1288).
Thank you everybody for all your wonderful advice, feedback, encouragement and code contributions. All of you made this one of the most rewarding and fun projects I have been lucky enough to work on!

I am closing out the issue as the feature is now available, but please feel free to open new issues to leave feedback or any bug reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests