stripe-python icon indicating copy to clipboard operation
stripe-python copied to clipboard

[WIP] Test each client for thread safety

Open jameshageman-stripe opened this issue 7 years ago • 5 comments

In light of #533, I think it might be useful to have a smoke test for testing the thread safety of our clients. This PR adds a test to check that each client can be reused across multiple threads without throwing exceptions.

jameshageman-stripe avatar Jan 30 '19 17:01 jameshageman-stripe

Currently,

  • RequestsClient is failing because #534 hasn't been merged
  • pycurl and urlfetch client are failing because their packages aren't installed
  • urllib2 is flaky:

If I run just the urllib2 client:

pytest tests/test_integration.py -k urllib

Sometimes it succeeds, and sometimes I get the following network error (even when I have stripe.max_network_retries = 3:

Exception in thread Thread-3:
Traceback (most recent call last):
  File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/threading.py", line 801, in __bootstrap_inner
    self.run()
  File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/threading.py", line 754, in run
    self.__target(*self.__args, **self.__kwargs)
  File "/Users/jameshageman/stripe/stripe-python/tests/test_integration.py", line 162, in work
    res = stripe.Balance.retrieve()
  File "/Users/jameshageman/stripe/stripe-python/stripe/api_resources/abstract/singleton_api_resource.py", line 9, in retrieve
    return super(SingletonAPIResource, cls).retrieve(None, **params)
  File "/Users/jameshageman/stripe/stripe-python/stripe/api_resources/abstract/api_resource.py", line 12, in retrieve
    instance.refresh()
  File "/Users/jameshageman/stripe/stripe-python/stripe/api_resources/abstract/api_resource.py", line 16, in refresh
    self.refresh_from(self.request("get", self.instance_url()))
  File "/Users/jameshageman/stripe/stripe-python/stripe/stripe_object.py", line 243, in request
    response, api_key = requestor.request(method, url, params, headers)
  File "/Users/jameshageman/stripe/stripe-python/stripe/api_requestor.py", line 126, in request
    method.lower(), url, params, headers
  File "/Users/jameshageman/stripe/stripe-python/stripe/api_requestor.py", line 357, in request_raw
    method, abs_url, headers, post_data
  File "/Users/jameshageman/stripe/stripe-python/stripe/http_client.py", line 139, in request_with_retries
    raise connection_error
APIConnectionError: Unexpected error communicating with Stripe. If this problem persists,
let us know at [email protected].

(Network error: <urlopen error [Errno 32] Broken pipe>)

I'm not sure if the network error has to do with thread safety, or just because I'm hammering a local server in the same process with 10 requests at the exact same time. If its the latter, then maybe I could add some random sleep before the request is sent and also during the server-side processing of the event.

jameshageman-stripe avatar Jan 30 '19 18:01 jameshageman-stripe

Seems like a good idea James.

I'm not sure if the network error has to do with thread safety, or just because I'm hammering a local server in the same process with 10 requests at the exact same time. If its the latter, then maybe I could add some random sleep before the request is sent and also during the server-side processing of the event.

Admittedly, I didn't look into this too deeply, but from the docs on socketserver (of which HTTPServer is a descendant of):

These four classes process requests synchronously; each request must be completed before the next request can be started. This isn’t suitable if each request takes a long time to complete, because it requires a lot of computation, or because it returns a lot of data which the client is slow to process. The solution is to create a separate process or thread to handle each request; the ForkingMixIn and ThreadingMixIn mix-in classes can be used to support asynchronous behaviour.

So I don't think it's going to have a good time being bombarded with a bunch of parallel requests.

You may want to switch to something like ThreadingHTTPServer instead.

brandur-stripe avatar Jan 30 '19 19:01 brandur-stripe

ThreadingHTTPServer isn't available in python 2.7 but I can try to find something that is.

jameshageman-stripe avatar Jan 30 '19 19:01 jameshageman-stripe

ThreadingHTTPServer isn't available in python 2.7 but I can try to find something that is.

Ah, right. Well, I suspect that this is the problem anyway though.

Regarding alternatives: I'd suggest not doing anything too elaborate to get Python 2.* working. Given that it's now really nearing its EOL, it wouldn't be worth to pull in a new dependency or anything. Maybe you can detect 2.7 and skip those tests or run at lower concurrency, or something.

brandur-stripe avatar Jan 30 '19 21:01 brandur-stripe

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 21 '20 17:07 CLAassistant

Closing as obsolete.

pakrym-stripe avatar Jan 19 '23 19:01 pakrym-stripe