From 09def3086819a8ddc669561d91bae5c45133c135 Mon Sep 17 00:00:00 2001 From: medmunds Date: Wed, 17 Jan 2018 14:36:50 -0800 Subject: [PATCH] Add timeout to all Requests calls Use a default timeout of 30 seconds for all requests, and add a REQUESTS_TIMEOUT Anymail setting to override. (I'm making a judgement call that this is not a breaking change in the real world, and not bumping the major version. Theoretically, it could affect you if your network somehow takes >30s to connect to your ESP, but eventually succeeds. If so, set REQUESTS_TIMEOUT to None to restore the earlier behavior.) Fixes #80. --- anymail/backends/base_requests.py | 3 ++ anymail/backends/mailjet.py | 2 +- docs/installation.rst | 11 ++++++ tests/test_base_backends.py | 65 +++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 tests/test_base_backends.py diff --git a/anymail/backends/base_requests.py b/anymail/backends/base_requests.py index 2530244..7b4589f 100644 --- a/anymail/backends/base_requests.py +++ b/anymail/backends/base_requests.py @@ -4,6 +4,7 @@ import requests # noinspection PyUnresolvedReferences from six.moves.urllib.parse import urljoin +from anymail.utils import get_anymail_setting from .base import AnymailBaseBackend, BasePayload from ..exceptions import AnymailRequestsAPIError, AnymailSerializationError from .._version import __version__ @@ -17,6 +18,7 @@ class AnymailRequestsBackend(AnymailBaseBackend): def __init__(self, api_url, **kwargs): """Init options from Django settings""" self.api_url = api_url + self.timeout = get_anymail_setting('requests_timeout', kwargs=kwargs, default=30) super(AnymailRequestsBackend, self).__init__(**kwargs) self.session = None @@ -65,6 +67,7 @@ class AnymailRequestsBackend(AnymailBaseBackend): Can raise AnymailRequestsAPIError for HTTP errors in the post """ params = payload.get_request_params(self.api_url) + params.setdefault('timeout', self.timeout) try: response = self.session.request(**params) except requests.RequestException as err: diff --git a/anymail/backends/mailjet.py b/anymail/backends/mailjet.py index 6dd8003..7154ee1 100644 --- a/anymail/backends/mailjet.py +++ b/anymail/backends/mailjet.py @@ -115,7 +115,7 @@ class MailjetPayload(RequestsPayload): if template_id and not self.data.get("FromEmail"): response = self.backend.session.get( "%sREST/template/%s/detailcontent" % (self.backend.api_url, template_id), - auth=self.auth + auth=self.auth, timeout=self.backend.timeout ) self.backend.raise_for_status(response, None, self.message) json_response = self.backend.deserialize_json_response(response, None, self.message) diff --git a/docs/installation.rst b/docs/installation.rst index 2eaf4d1..485e932 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -269,3 +269,14 @@ This is actually implemented using HTTP basic authorization, and the string is technically a "username:password" format. But you should *not* use any real username or password for this shared secret. + +.. setting:: ANYMAIL_REQUESTS_TIMEOUT + +.. rubric:: REQUESTS_TIMEOUT + +.. versionadded:: 1.3 + +For Requests-based Anymail backends, the timeout value used for all API calls to your ESP. +The default is 30 seconds. You can set to a single float, a 2-tuple of floats for +separate connection and read timeouts, or `None` to disable timeouts (not recommended). +See :ref:`requests:timeouts` in the Requests docs for more information. diff --git a/tests/test_base_backends.py b/tests/test_base_backends.py new file mode 100644 index 0000000..a778058 --- /dev/null +++ b/tests/test_base_backends.py @@ -0,0 +1,65 @@ +from django.test import override_settings + +from anymail.backends.base_requests import AnymailRequestsBackend, RequestsPayload +from anymail.message import AnymailMessage, AnymailRecipientStatus + +from .mock_requests_backend import RequestsBackendMockAPITestCase + + +class MinimalRequestsBackend(AnymailRequestsBackend): + """(useful only for these tests)""" + + esp_name = "Example" + + def __init__(self, **kwargs): + super(MinimalRequestsBackend, self).__init__("https://esp.example.com/api/", **kwargs) + + def build_message_payload(self, message, defaults): + return MinimalRequestsPayload(message, defaults, self) + + def parse_recipient_status(self, response, payload, message): + return {'to@example.com': AnymailRecipientStatus('message-id', 'sent')} + + +class MinimalRequestsPayload(RequestsPayload): + def init_payload(self): + pass + + def _noop(self, *args, **kwargs): + pass + + set_from_email = _noop + set_recipients = _noop + set_subject = _noop + set_reply_to = _noop + set_extra_headers = _noop + set_text_body = _noop + set_html_body = _noop + add_attachment = _noop + + +@override_settings(EMAIL_BACKEND='tests.test_base_backends.MinimalRequestsBackend') +class RequestsBackendBaseTestCase(RequestsBackendMockAPITestCase): + """Test common functionality in AnymailRequestsBackend""" + + def setUp(self): + super(RequestsBackendBaseTestCase, self).setUp() + self.message = AnymailMessage('Subject', 'Text Body', 'from@example.com', ['to@example.com']) + + def test_minimal_requests_backend(self): + """Make sure the testing backend defined above actually works""" + self.message.send() + self.assert_esp_called("https://esp.example.com/api/") + + def test_timeout_default(self): + """All requests have a 30 second default timeout""" + self.message.send() + timeout = self.get_api_call_arg('timeout') + self.assertEqual(timeout, 30) + + @override_settings(ANYMAIL_REQUESTS_TIMEOUT=5) + def test_timeout_setting(self): + """You can use the Anymail setting REQUESTS_TIMEOUT to override the default""" + self.message.send() + timeout = self.get_api_call_arg('timeout') + self.assertEqual(timeout, 5)