add retry on RemoteDisconnect error, add backoff timer, increase testing

Author: maxkahan

http_client/src/vonage_http_client/errors.py

@@ -122,23 +122,6 @@ def __init__(self, response: Response, content_type: str):
         super().__init__(response, content_type)
 
 
-class FileStreamingError(HttpRequestError):
-    """Exception indicating an error occurred while streaming a file in a Vonage SDK
-    request.
-
-    Args:
-        response (requests.Response): The HTTP response object.
-        content_type (str): The response content type.
-
-    Attributes (inherited from HttpRequestError parent exception):
-        response (requests.Response): The HTTP response object.
-        message (str): The returned error message.
-    """
-
-    def __init__(self, response: Response, content_type: str):
-        super().__init__(response, content_type)
-
-
 class ServerError(HttpRequestError):
     """Exception indicating an error was returned by a Vonage server in response to a
     Vonage SDK request.
@@ -156,3 +139,8 @@ class ServerError(HttpRequestError):
 
     def __init__(self, response: Response, content_type: str):
         super().__init__(response, content_type)
+
+
+class FileStreamingError(VonageError):
+    """Exception indicating an error occurred while streaming a file in a Vonage SDK
+    request."""

http_client/src/vonage_http_client/http_client.py

@@ -6,7 +6,9 @@
 from pydantic import BaseModel, Field, ValidationError, validate_call
 from requests import PreparedRequest, Response
 from requests.adapters import HTTPAdapter
+from requests.exceptions import ConnectionError
 from requests.sessions import Session
+from urllib3 import Retry
 from vonage_http_client.auth import Auth
 from vonage_http_client.errors import (
     AuthenticationError,
@@ -90,7 +92,9 @@ def __init__(
         self._adapter = HTTPAdapter(
             pool_connections=self._http_client_options.pool_connections,
             pool_maxsize=self._http_client_options.pool_maxsize,
-            max_retries=self._http_client_options.max_retries,
+            max_retries=Retry(
+                total=self._http_client_options.max_retries, backoff_factor=0.1
+            ),
         )
         self._session.mount('https://', self._adapter)
 
@@ -216,6 +220,30 @@ def make_request(
         sent_data_type: Literal['json', 'form', 'query_params'] = 'json',
         token: Optional[str] = None,
     ):
+        """Make an HTTP request to the specified host. This method will automatically
+        handle retries in the event of a connection error caused by a RemoteDisconnect
+        exception.
+
+        It will retry the amount of times equal to the maximum number of connections
+        allowed in a connection pool. I.e., assuming if all connections in a given pool
+        are in use but the TCP connections to the Vonage host have failed, it will retry
+        the amount of times equal to the maximum number of connections in the pool.
+
+        Args:
+            request_type (str): The type of request to make (GET, POST, PATCH, PUT, DELETE).
+            host (str): The host to make the request to.
+            request_path (str, optional): The path to make the request to.
+            params (dict, optional): The parameters to send with the request.
+            auth_type (str, optional): The type of authentication to use with the request.
+            sent_data_type (str, optional): The type of data being sent with the request.
+            token (str, optional): The token to use for OAuth2 authentication.
+
+        Returns:
+            dict: The response data from the request.
+
+        Raises:
+            ConnectionError: If the request fails after the maximum number of retries.
+        """
         url = f'https://{host}{request_path}'
         logger.debug(
             f'{request_type} request to {url}, with data: {params}; headers: {self._headers}'
@@ -248,8 +276,23 @@ def make_request(
         elif sent_data_type == 'form':
             request_params['data'] = params
 
-        with self._session.request(**request_params) as response:
-            return self._parse_response(response)
+        max_retries = self._http_client_options.pool_maxsize or 10
+        attempt = 0
+        while attempt < max_retries:
+            try:
+                with self._session.request(**request_params) as response:
+                    return self._parse_response(response)
+            except ConnectionError as e:
+                logger.debug(f'Connection Error: {e}')
+                if 'RemoteDisconnected' in str(e.args):
+                    attempt += 1
+                    if attempt >= max_retries:
+                        raise e
+                    logger.debug(
+                        f'ConnectionError caused by RemoteDisconnected exception. Retrying request, attempt {attempt + 1} of {max_retries}'
+                    )
+                else:
+                    raise e
 
     def download_file_stream(self, url: str, file_path: str) -> bytes:
         """Download a file from a URL and save it to a local file. This method streams the
@@ -272,6 +315,8 @@ def download_file_stream(self, url: str, file_path: str) -> bytes:
         )
         try:
             with self._session.get(url, headers=headers, stream=True) as response:
+                if response.status_code >= 400:
+                    self._parse_response(response)
                 with open(file_path, 'wb') as f:
                     for chunk in response.iter_content(chunk_size=4096):
                         f.write(chunk)
@@ -296,8 +341,6 @@ def _parse_response(self, response: Response) -> Union[dict, None]:
             try:
                 return response.json()
             except JSONDecodeError:
-                if hasattr(response.headers, 'Content-Type'):
-                    return response.content
                 return None
         if response.status_code >= 400:
             content_type = response.headers['Content-Type'].split(';', 1)[0]

http_client/tests/test_http_client.py

@@ -1,9 +1,12 @@
+from http.client import RemoteDisconnected
 from json import loads
 from os.path import abspath, dirname, join
+from unittest.mock import patch
 
 import responses
 from pytest import raises
-from requests import PreparedRequest, Response
+from requests import PreparedRequest, Response, Session
+from requests.exceptions import ConnectionError
 from responses import matchers
 from vonage_http_client.auth import Auth
 from vonage_http_client.errors import (
@@ -15,7 +18,7 @@
     RateLimitedError,
     ServerError,
 )
-from vonage_http_client.http_client import HttpClient
+from vonage_http_client.http_client import HttpClient, HttpClientOptions
 
 from testutils import build_response, get_mock_jwt_auth
 
@@ -265,10 +268,10 @@ def test_download_file_stream():
     client = HttpClient(get_mock_jwt_auth())
     client.download_file_stream(
         url='https://api.nexmo.com/v1/files/aaaaaaaa-bbbb-cccc-dddd-0123456789ab',
-        file_path='file.mp3',
+        file_path='http_client/tests/data/file_stream.mp3',
     )
 
-    with open('file.mp3', 'rb') as file:
+    with open('http_client/tests/data/file_stream.mp3', 'rb') as file:
         file_content = file.read()
         assert file_content.startswith(b'ID3')
 
@@ -280,15 +283,45 @@ def test_download_file_stream_error():
         'GET',
         'https://api.nexmo.com/v1/files/aaaaaaaa-bbbb-cccc-dddd-0123456789ab',
         status_code=400,
+        mock_path='400.json',
     )
 
     client = HttpClient(get_mock_jwt_auth())
-    try:
+    with raises(FileStreamingError) as e:
         client.download_file_stream(
             url='https://api.nexmo.com/v1/files/aaaaaaaa-bbbb-cccc-dddd-0123456789ab',
             file_path='file.mp3',
         )
-    except FileStreamingError as err:
-        assert '400 response from' in err.message
-        assert err.response.status_code == 400
-        assert err.response.json()['title'] == 'Bad Request'
+    assert '400 response from' in e.exconly()
+
+
+@patch.object(Session, 'request')
+def test_retry_on_remote_disconnected_connection_error(mock_request):
+    mock_request.side_effect = ConnectionError(
+        RemoteDisconnected('Remote end closed connection without response')
+    )
+    client = HttpClient(
+        Auth(application_id=application_id, private_key=private_key),
+        http_client_options=HttpClientOptions(),
+    )
+    params = {
+        'test': 'post request',
+        'testing': 'http client',
+    }
+    with raises(ConnectionError) as e:
+        client.post(host='example.com', request_path='/post_json', params=params)
+    assert mock_request.call_count == 10
+    assert 'Remote end closed connection without response' in str(e.value)
+
+
+@patch.object(Session, 'request')
+def test_dont_retry_on_generic_connection_error(mock_request):
+    mock_request.side_effect = ConnectionError('Error in connection to remote server')
+    client = HttpClient(
+        Auth(application_id=application_id, private_key=private_key),
+        http_client_options=HttpClientOptions(),
+    )
+    with raises(ConnectionError) as e:
+        client.get(host='example.com', request_path='/get_json')
+    assert mock_request.call_count == 1
+    assert 'Error in connection to remote server' in str(e.value)

jwt/CHANGES.md

@@ -1,3 +1,6 @@
+# 1.1.5
+- Improve `verify_signature` docstring
+
 # 1.1.4
 - Fix a bug with generating non-default JWTs
 

jwt/src/vonage_jwt/_version.py

@@ -1 +1 @@
-__version__ = '1.1.4'
+__version__ = '1.1.5'

jwt/src/vonage_jwt/verify_jwt.py

@@ -4,7 +4,18 @@
 
 
 def verify_signature(token: str, signature_secret: str = None) -> bool:
-    """Method to verify that an incoming JWT was sent by Vonage."""
+    """Method to verify that an incoming JWT was sent by Vonage.
+
+    Args:
+        token (str): The token to verify.
+        signature_secret (str, optional): The signature to verify the token against.
+
+    Returns:
+        bool: True if the token is verified, False otherwise.
+
+    Raises:
+        VonageVerifyJwtError: The signature could not be verified.
+    """
 
     try:
         decode(token, signature_secret, algorithms='HS256')

pants.toml

@@ -1,5 +1,5 @@
 [GLOBAL]
-pants_version = '2.23.0'
+pants_version = '2.24.0a0'
 
 backend_packages = [
   'pants.backend.python',

requirements.txt

@@ -1,10 +1,11 @@
 pytest>=8.0.0
 requests>=2.31.0
 responses>=0.24.1
-pydantic>=2.7.1
+pydantic>=2.9.2
 typing-extensions>=4.9.0
 pyjwt[crypto]>=1.6.4
 toml>=0.10.2
+urllib3
 
 -e jwt
 -e http_client

sample-6s.mp3

@@ -2,6 +2,7 @@
 
 from pydantic import validate_call
 from vonage_http_client.http_client import HttpClient
+from vonage_jwt.verify_jwt import verify_signature
 from vonage_utils.types import Dtmf
 from vonage_voice.models.ncco import NccoAction
 
@@ -274,3 +275,20 @@ def download_recording(self, url: str, file_path: str) -> bytes:
             bytes: The recording data.
         """
         self._http_client.download_file_stream(url=url, file_path=file_path)
+
+    @validate_call
+    def verify_signature(self, token: str, signature: str) -> bool:
+        """Verifies that a token has been signed with the provided signature. Used to
+        verify that a webhook was sent by Vonage.
+
+        Args:
+            token (str): The token to verify.
+            signature (str): The signature to verify the token against.
+
+        Returns:
+            bool: True if the token was signed with the provided signature, False otherwise.
+
+        Raises:
+            VonageVerifyJwtError: The signature could not be verified.
+        """
+        return verify_signature(token, signature)