As a developer, I'd like to differentiate between Timeout exceptions, so that I can handle them differently

[bhargavrpatel]

General information

• SDK/Library version: HEAD (Edit: https://github.com/braintree/braintree_python/commit/d2eaf7c1b52f3548331a7a05bb34532c88bc4e4e)
• Environment: N/A
• Language, language version, and OS: Python, Python 2.7.4+, any

Issue description

Story: As a developer, I'd like to differentiate between Timeout exceptions, so that I can handle them differently.

Context: Braintree Python implementation utilizes the requests library and re-raises certain exception as its Braintree counterparts. At the moment, it seems we re-raise requests.exceptions.TimeoutError as braintree.exceptions.http.TimeoutError.

As documented in the Requests module exceptions documentation, requests.exceptions.TimeoutError is a generic exception which is used by requests.exceptions.ConnectTimeout as well as requests.exceptions.ReadTimeout.

Braintree documentation notes that not all timeouts imply a failed operation; a blind retry can lead to double auths and charges. I do believe that Connection timeouts can be retried as the connection between Client and Server was never established. Read Timeouts are where things get dicey and Timeout Reversals come into play.

I wonder if we can differentiate between requests.exceptions.ConnectTimeout, and requests.exceptions.ReadTimeout so that we can handle the two cases accordingly.

https://github.com/braintree/braintree_python/blob/d2eaf7c1b52f3548331a7a05bb34532c88bc4e4e/braintree/util/http.py#L112-L120

The above snippet shows the function that handles the exceptions. A caveat is that ConnectTimeout is additionally a child of ConnectionError which we handle first; so we'll raise braintree.exceptions.http.ConectionError! The issue there also is that a similar inheritance hierarchy/MRO does not exist for the BrainTree exceptions. Additionally, I'd still like to differentiate between other connection errors (such as SSL errors) which may require human intervention.

Solutions:

1. Add a similar inheritance hierarchy/MRO as the requests library
2. Subclass braintree.exceptions.TimeoutError and raise braintree.exceptions.ReadTimeoutError and braintree.exceptions.ConnectTimeoutError accordingly.

P.S: We've been using this library for quite some time so I wanted to say thank you for the awesome documentation.

P.P.S: More than happy to propose a PR as well.

Thanks, BRP

[crookedneighbor]

Feel free to open a PR. As long as it doesn't break existing integrations that are catching the timeout exception as shown in our docs:

try:
  some_bt_sdk_operation()
except braintree.exceptions.not_found_error.TimeoutError as e:
  print e

then it should be fine.

[crookedneighbor]

I just consulted with one of the developers here who specializes in Python, and he pointed out that if an integration is checking the type directly, subclassing the error could break that integration. In that case, it's probably better to hold off on a change like this until we release the next major version.

An alternative solution would be to add a property to the error that exposes the original error.

[bhargavrpatel]

@crookedneighbor Wondering if the developer can take a look at the PR I just tossed up as I believe I have addressed that concern. Here's why:

• Any current implementations likely handle the Timeout error by excepting it as shown below.

  try:
      raise braintree.exceptions.TimeoutError()
  except braintree.exceptions.TimeoutError as e:
      print("A generic timeout occurred")

• The proposed changes merely introduce child classes which means that even if the library now raises a more granular error, the callers will not have to change the code as shown below:

  try:
      raise braintree.exceptions.ReadTimeoutError()
  except braintree.exceptions.TimeoutError as e:
      print("A  generic timeout occurred")

• The proposed changes do however, allow the callers now to handle the ReadTimeout explicitly if they choose to do so as shown below:

  try:
      raise braintree.exceptions.ReadTimeoutError()
  except braintree.exceptions.ConnectionTimeoutError as e:
      print("A retriable timeout occurred. I'll retry this...")
  except braintree.exceptions.TimeoutError as e:
      print("A  generic timeout occurred")

Not sure if we are causing breaking changes here.

[crookedneighbor]

This will go out in the next release, will leave this open until the release goes out.

[crookedneighbor]

This is out in 3.55.0