Mailgun: improve API error messages

Author: medmunds

CHANGELOG.rst

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

anymail/backends/mailgun.py

@@ -47,6 +47,30 @@ def __init__(self, **kwargs):
     def build_message_payload(self, message, defaults):
         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):
         # The *only* 200 response from Mailgun seems to be:
         #     {

anymail/webhooks/mailgun.py

@@ -352,6 +352,15 @@ def esp_to_anymail_event(self, request):
                 "You seem to have set Mailgun's *%s tracking* webhook "
                 "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:
             # Raw-MIME
             message = AnymailInboundMessage.parse_raw_mime(request.POST['body-mime'])

tests/mock_requests_backend.py

@@ -21,12 +21,22 @@ class RequestsBackendMockAPITestCase(AnymailTestMixin, SimpleTestCase):
 
     class MockResponse(requests.Response):
         """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__()
             self.status_code = status_code
             self.encoding = encoding
             self.reason = reason or ("OK" if 200 <= status_code < 300 else "ERROR")
             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):
         super().setUp()
@@ -38,7 +48,7 @@ def setUp(self):
     def set_mock_response(self, status_code=DEFAULT_STATUS_CODE, raw=UNSET, encoding='utf-8', reason=None):
         if raw is UNSET:
             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
         return mock_response
 

tests/test_mailgun_backend.py

@@ -667,6 +667,31 @@ def test_encode_sender_domain(self):
         self.message.send()
         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):
         """Make sure by default we don't send any ESP-specific options.
 

tests/test_mailgun_inbound.py

@@ -196,6 +196,29 @@ def test_misconfigured_tracking(self):
             self.client.post('/anymail/mailgun/inbound/',
                              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': '[email protected]',
+            'sender': '[email protected]',
+            '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):
         raw_event = mailgun_sign_legacy_payload({
             'domain': 'example.com',