Postmark: fix silent failures for send-time validation errors

Postmark uses their ErrorCode 300 to report several different
send-time validation errors, some of which identify invalid
recipients that need to be handled specially, but many of which
are ordinary API errors.

Rework the logic for parsing ErrorCode 300 error messages:
Handle only "Invalid 'To'" or "Error parsing 'To'" (or 'Cc'
or 'Bcc') as recipient errors. Otherwise raise an API error.

Fixes #238 silent failure when sending with long metadata keys.
This commit is contained in:
Mike Edmunds
2021-05-19 13:15:34 -07:00
committed by GitHub
parent b1a4f9809a
commit 44754c908e
3 changed files with 59 additions and 11 deletions

View File

@@ -41,6 +41,11 @@ Fixes
* **Postmark:** Fix two different errors when sending with a template but no merge * **Postmark:** Fix two different errors when sending with a template but no merge
data. (Thanks to `@kareemcoding`_ and `@Tobeyforce`_ for reporting them.) data. (Thanks to `@kareemcoding`_ and `@Tobeyforce`_ for reporting them.)
* **Postmark:** Fix silent failure when sending with long metadata keys and some
other errors Postmark detects at send time. Report invalid 'cc' and 'bcc' addresses
detected at send time the same as 'to' recipients. (Thanks to `@chrisgrande`_ for
reporting the problem.)
v8.2 v8.2
----- -----
@@ -1222,6 +1227,7 @@ Features
.. _@alee: https://github.com/alee .. _@alee: https://github.com/alee
.. _@anstosa: https://github.com/anstosa .. _@anstosa: https://github.com/anstosa
.. _@calvin: https://github.com/calvin .. _@calvin: https://github.com/calvin
.. _@chrisgrande: https://github.com/chrisgrande
.. _@costela: https://github.com/costela .. _@costela: https://github.com/costela
.. _@decibyte: https://github.com/decibyte .. _@decibyte: https://github.com/decibyte
.. _@dominik-lekse: https://github.com/dominik-lekse .. _@dominik-lekse: https://github.com/dominik-lekse

View File

@@ -94,22 +94,24 @@ class EmailBackend(AnymailRequestsBackend):
message_id=None, status='rejected') message_id=None, status='rejected')
elif error_code == 300: # Invalid email request elif error_code == 300: # Invalid email request
# Either the From address or at least one recipient was invalid. Email not sent. # Various parse-time validation errors, which may include invalid recipients. Email not sent.
# response["To"] is not populated for this error; must examine response["Message"]: # response["To"] is not populated for this error; must examine response["Message"]:
if re.match(r"^(Invalid|Error\s+parsing)\s+'(To|Cc|Bcc)'", msg, re.IGNORECASE):
# Recipient-related errors: use AnymailRecipientsRefused logic
# "Invalid 'To' address: '{addr_spec}'." # "Invalid 'To' address: '{addr_spec}'."
# "Error parsing 'To': Illegal email domain '{domain}' in address '{addr_spec}'." # "Error parsing 'Cc': Illegal email domain '{domain}' in address '{addr_spec}'."
# "Error parsing 'To': Illegal email address '{addr_spec}'. It must contain the '@' symbol." # "Error parsing 'Bcc': Illegal email address '{addr_spec}'. It must contain the '@' symbol."
# "Invalid 'From' address: '{email_address}'."
if "'From' address" in msg:
# Normal error
raise AnymailRequestsAPIError(email_message=message, payload=payload, response=response,
backend=self)
else:
# Use AnymailRecipientsRefused logic
invalid_addr_specs = self._addr_specs_from_error_msg(msg, r"address:?\s*'(.*)'") invalid_addr_specs = self._addr_specs_from_error_msg(msg, r"address:?\s*'(.*)'")
for invalid_addr_spec in invalid_addr_specs: for invalid_addr_spec in invalid_addr_specs:
recipient_status[invalid_addr_spec] = AnymailRecipientStatus( recipient_status[invalid_addr_spec] = AnymailRecipientStatus(
message_id=None, status='invalid') message_id=None, status='invalid')
else:
# Non-recipient errors; handle as normal API error response
# "Invalid 'From' address: '{email_address}'."
# "Error parsing 'Reply-To': Illegal email domain '{domain}' in address '{addr_spec}'."
# "Invalid metadata content. ..."
raise AnymailRequestsAPIError(email_message=message, payload=payload,
response=response, backend=self)
elif error_code == 406: # Inactive recipient elif error_code == 406: # Inactive recipient
# All recipients were rejected as hard-bounce or spam-complaint. Email not sent. # All recipients were rejected as hard-bounce or spam-complaint. Email not sent.

View File

@@ -709,6 +709,19 @@ class PostmarkBackendRecipientsRefusedTests(PostmarkBackendMockAPITestCase):
status = msg.anymail_status status = msg.anymail_status
self.assertEqual(status.recipients['Invalid@LocalHost'].status, 'invalid') self.assertEqual(status.recipients['Invalid@LocalHost'].status, 'invalid')
def test_recipients_parse_error(self):
self.set_mock_response(
status_code=422,
raw=b"""{
"ErrorCode": 300,
"Message": "Error parsing 'Cc': Illegal email domain '+' in address 'user@+'."
}""")
msg = mail.EmailMessage('Subject', 'Body', 'from@example.com', cc=["user@+"])
with self.assertRaises(AnymailRecipientsRefused):
msg.send()
status = msg.anymail_status
self.assertEqual(status.recipients['user@+'].status, 'invalid')
def test_from_email_invalid(self): def test_from_email_invalid(self):
# Invalid 'From' address generates same Postmark ErrorCode 300 as invalid 'To', # Invalid 'From' address generates same Postmark ErrorCode 300 as invalid 'To',
# but should raise a different Anymail error # but should raise a different Anymail error
@@ -720,6 +733,33 @@ class PostmarkBackendRecipientsRefusedTests(PostmarkBackendMockAPITestCase):
with self.assertRaises(AnymailAPIError): with self.assertRaises(AnymailAPIError):
msg.send() msg.send()
def test_reply_to_invalid(self):
# Make sure 'Reply-To' error doesn't get caught in invalid 'To' logic:
self.set_mock_response(
status_code=422,
raw=b"""{
"ErrorCode": 300,
"Message": "Error parsing 'Reply-To': Illegal email domain '+' in address 'invalid@+'."}
""")
msg = mail.EmailMessage('Subject', 'Body', 'from@example.com', ['to@example.com'],
reply_to=["invalid@+"])
with self.assertRaisesMessage(AnymailAPIError, "Error parsing 'Reply-To'"):
msg.send()
def test_errorcode_300(self):
# Various other problems generate same Postmark ErrorCode 300 as invalid 'To',
# but should raise ordinary API errors
self.set_mock_response(
status_code=422,
raw=b"""{
"ErrorCode": 300,
"Message": "Invalid metadata content. Field names are limited to 20 characters..."
}""")
msg = mail.EmailMessage('Subject', 'Body', 'from@example.com', ['to@example.com'])
msg.metadata = {"this-key-name-is-too-long": "data"}
with self.assertRaisesMessage(AnymailAPIError, "Invalid metadata content"):
msg.send()
def test_fail_silently(self): def test_fail_silently(self):
self.set_mock_response( self.set_mock_response(
status_code=422, status_code=422,