Mailgun: improve API error messages

This commit is contained in:
medmunds
2021-01-26 16:29:14 -08:00
committed by Mike Edmunds
parent 2196ab8330
commit d33c9ea4ed
6 changed files with 95 additions and 2 deletions

View File

@@ -40,6 +40,8 @@ Fixes
Other Other
~~~~~ ~~~~~
* **Mailgun:** Improve error messages for some common configuration issues.
* Test against Django 3.2 prerelease (including support for Python 3.9) * Test against Django 3.2 prerelease (including support for Python 3.9)
* Move CI testing to GitHub Actions (and stop using Travis-CI). * Move CI testing to GitHub Actions (and stop using Travis-CI).

View File

@@ -47,6 +47,30 @@ class EmailBackend(AnymailRequestsBackend):
def build_message_payload(self, message, defaults): def build_message_payload(self, message, defaults):
return MailgunPayload(message, defaults, self) return MailgunPayload(message, defaults, self)
def raise_for_status(self, response, payload, message):
# Mailgun issues a terse 404 for unrecognized sender domains.
# Add some context:
if response.status_code == 404 and "Domain not found" in response.text:
raise AnymailRequestsAPIError(
"Unknown sender domain {sender_domain!r}.\n"
"Check the domain is verified with Mailgun, and that the ANYMAIL"
" MAILGUN_API_URL setting {api_url!r} is the correct region.".format(
sender_domain=payload.sender_domain, api_url=self.api_url),
email_message=message, payload=payload,
response=response, backend=self)
super().raise_for_status(response, payload, message)
# Mailgun issues a cryptic "Mailgun Magnificent API" success response
# for invalid API endpoints. Convert that to a useful error:
if response.status_code == 200 and "Mailgun Magnificent API" in response.text:
raise AnymailRequestsAPIError(
"Invalid Mailgun API endpoint %r.\n"
"Check your ANYMAIL MAILGUN_SENDER_DOMAIN"
" and MAILGUN_API_URL settings." % response.url,
email_message=message, payload=payload,
response=response, backend=self)
def parse_recipient_status(self, response, payload, message): def parse_recipient_status(self, response, payload, message):
# The *only* 200 response from Mailgun seems to be: # The *only* 200 response from Mailgun seems to be:
# { # {

View File

@@ -352,6 +352,15 @@ class MailgunInboundWebhookView(MailgunBaseWebhookView):
"You seem to have set Mailgun's *%s tracking* webhook " "You seem to have set Mailgun's *%s tracking* webhook "
"to Anymail's Mailgun *inbound* webhook URL." % request.POST['event']) "to Anymail's Mailgun *inbound* webhook URL." % request.POST['event'])
if 'attachments' in request.POST:
# Inbound route used store() rather than forward().
# ("attachments" seems to be the only POST param that differs between
# store and forward; Anymail could support store by handling the JSON
# attachments param in message_from_mailgun_parsed.)
raise AnymailConfigurationError(
"You seem to have configured Mailgun's receiving route using the store()"
" action. Anymail's inbound webhook requires the forward() action.")
if 'body-mime' in request.POST: if 'body-mime' in request.POST:
# Raw-MIME # Raw-MIME
message = AnymailInboundMessage.parse_raw_mime(request.POST['body-mime']) message = AnymailInboundMessage.parse_raw_mime(request.POST['body-mime'])

View File

@@ -21,12 +21,22 @@ class RequestsBackendMockAPITestCase(AnymailTestMixin, SimpleTestCase):
class MockResponse(requests.Response): class MockResponse(requests.Response):
"""requests.request return value mock sufficient for testing""" """requests.request return value mock sufficient for testing"""
def __init__(self, status_code=200, raw=b"RESPONSE", encoding='utf-8', reason=None): def __init__(self, status_code=200, raw=b"RESPONSE", encoding='utf-8', reason=None, test_case=None):
super().__init__() super().__init__()
self.status_code = status_code self.status_code = status_code
self.encoding = encoding self.encoding = encoding
self.reason = reason or ("OK" if 200 <= status_code < 300 else "ERROR") self.reason = reason or ("OK" if 200 <= status_code < 300 else "ERROR")
self.raw = BytesIO(raw) self.raw = BytesIO(raw)
self.test_case = test_case
@property
def url(self):
return self.test_case.get_api_call_arg('url', required=False)
@url.setter
def url(self, url):
if url is not None:
raise ValueError("MockResponse can't handle url assignment")
def setUp(self): def setUp(self):
super().setUp() super().setUp()
@@ -38,7 +48,7 @@ class RequestsBackendMockAPITestCase(AnymailTestMixin, SimpleTestCase):
def set_mock_response(self, status_code=DEFAULT_STATUS_CODE, raw=UNSET, encoding='utf-8', reason=None): def set_mock_response(self, status_code=DEFAULT_STATUS_CODE, raw=UNSET, encoding='utf-8', reason=None):
if raw is UNSET: if raw is UNSET:
raw = self.DEFAULT_RAW_RESPONSE raw = self.DEFAULT_RAW_RESPONSE
mock_response = self.MockResponse(status_code, raw=raw, encoding=encoding, reason=reason) mock_response = self.MockResponse(status_code, raw=raw, encoding=encoding, reason=reason, test_case=self)
self.mock_request.return_value = mock_response self.mock_request.return_value = mock_response
return mock_response return mock_response

View File

@@ -667,6 +667,31 @@ class MailgunBackendAnymailFeatureTests(MailgunBackendMockAPITestCase):
self.message.send() self.message.send()
self.assert_esp_called('/example.com%20%23%20oops/messages') self.assert_esp_called('/example.com%20%23%20oops/messages')
def test_unknown_sender_domain(self):
self.set_mock_response(raw=b"""{
"message": "Domain not found: example.com"
}""", status_code=404)
with self.assertRaisesMessage(
AnymailAPIError,
"Unknown sender domain 'example.com'.\n"
"Check the domain is verified with Mailgun, and that the ANYMAIL MAILGUN_API_URL"
" setting 'https://api.mailgun.net/v3/' is the correct region."
):
self.message.send()
@override_settings(
# This is *not* a valid MAILGUN_API_URL setting (it should end at "...v3/"):
ANYMAIL_MAILGUN_API_URL='https://api.mailgun.net/v3/example.com/messages')
def test_magnificent_api(self):
# (Wouldn't a truly "magnificent API" just provide a helpful error message?)
self.set_mock_response(raw=b"Mailgun Magnificent API", status_code=200)
with self.assertRaisesMessage(
AnymailAPIError,
"Invalid Mailgun API endpoint 'https://api.mailgun.net/v3/example.com/messages/example.com/messages'.\n"
"Check your ANYMAIL MAILGUN_SENDER_DOMAIN and MAILGUN_API_URL settings."
):
self.message.send()
def test_default_omits_options(self): def test_default_omits_options(self):
"""Make sure by default we don't send any ESP-specific options. """Make sure by default we don't send any ESP-specific options.

View File

@@ -196,6 +196,29 @@ class MailgunInboundTestCase(WebhookTestCase):
self.client.post('/anymail/mailgun/inbound/', self.client.post('/anymail/mailgun/inbound/',
data=json.dumps(raw_event), content_type='application/json') data=json.dumps(raw_event), content_type='application/json')
def test_misconfigured_store_action(self):
# store() notification includes "attachments" json; forward() includes "attachment-count"
raw_event = mailgun_sign_legacy_payload({
'token': '06c96bafc3f42a66b9edd546347a2fe18dc23461fe80dc52f0',
'timestamp': '1461261330',
'recipient': 'test@inbound.example.com',
'sender': 'envelope-from@example.org',
'body-plain': 'Test body plain',
'body-html': '<div>Test body html</div>',
'attachments': json.dumps([{
"url": "https://storage.mailgun.net/v3/domains/example.com/messages/MESSAGE_KEY/attachments/0",
"content-type": "application/pdf",
"name": "attachment.pdf",
"size": 20202
}]),
})
with self.assertRaisesMessage(
AnymailConfigurationError,
"You seem to have configured Mailgun's receiving route using the store() action."
" Anymail's inbound webhook requires the forward() action."
):
self.client.post('/anymail/mailgun/inbound/', data=raw_event)
def test_misconfigured_tracking_legacy(self): def test_misconfigured_tracking_legacy(self):
raw_event = mailgun_sign_legacy_payload({ raw_event = mailgun_sign_legacy_payload({
'domain': 'example.com', 'domain': 'example.com',