Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mailjet: Don't try to parse non-JSON responses as JSON. #411

Merged
merged 1 commit into from
Dec 10, 2024
Merged

Mailjet: Don't try to parse non-JSON responses as JSON. #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ Fixes
* **Mailjet:** Avoid a Mailjet API error when sending an inline image without a
filename. (Anymail now substitutes ``"attachment"`` for the missing filename.)
(Thanks to `@chickahoona`_ for reporting the issue.)
* **Mailjet:** Fix a JSON parsing error on Mailjet 429 "too many requests" API
responses. (Thanks to `@rodrigondec`_ for reporting the issue.)


v12.0
Expand Down Expand Up @@ -1779,6 +1781,7 @@ Features
.. _@originell: https://github.com/originell
.. _@puru02: https://github.com/puru02
.. _@RignonNoel: https://github.com/RignonNoel
.. _@rodrigondec: https://github.com/rodrigondec
.. _@sblondon: https://github.com/sblondon
.. _@scur-iolus: https://github.com/scur-iolus
.. _@sdarwin: https://github.com/sdarwin
Expand Down
5 changes: 4 additions & 1 deletion anymail/backends/mailjet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ def build_message_payload(self, message, defaults):
return MailjetPayload(message, defaults, self)

def raise_for_status(self, response, payload, message):
if 400 <= response.status_code <= 499:
content_type = (
response.headers.get("content-type", "").split(";")[0].strip().lower()
)
if 400 <= response.status_code <= 499 and content_type == "application/json":
# Mailjet uses 4xx status codes for partial failure in batch send;
# we'll determine how to handle below in parse_recipient_status.
return
Expand Down
85 changes: 48 additions & 37 deletions tests/test_mailjet_backend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

from anymail.exceptions import (
AnymailAPIError,
AnymailRequestsAPIError,
AnymailSerializationError,
AnymailUnsupportedFeature,
)
Expand Down Expand Up @@ -690,43 +691,41 @@ def test_mixed_status(self):
# Mailjet's v3.1 API will partially fail a batch send, allowing valid emails
# to go out. The API response doesn't identify the failed email addresses;
# make sure we represent them correctly in the anymail_status.
response_content = json.dumps(
{
"Messages": [
{
"Status": "success",
"CustomID": "",
"To": [
{
"Email": "[email protected]",
"MessageUUID": "556e896a-e041-4836-bb35-8bb75ee308c5",
"MessageID": 12345678901234500,
"MessageHref": "https://api.mailjet.com/v3/REST"
"/message/12345678901234500",
}
],
"Cc": [],
"Bcc": [],
},
{
"Errors": [
{
"ErrorIdentifier": "f480a5a2-0334-4e08"
"-b2b7-f372ce5669e0",
"ErrorCode": "mj-0013",
"StatusCode": 400,
"ErrorMessage": '"[email protected]" is an invalid'
" email address.",
"ErrorRelatedTo": ["To[0].Email"],
}
],
"Status": "error",
},
]
}
).encode("utf-8")
response_data = {
"Messages": [
{
"Status": "success",
"CustomID": "",
"To": [
{
"Email": "[email protected]",
"MessageUUID": "556e896a-e041-4836-bb35-8bb75ee308c5",
"MessageID": 12345678901234500,
"MessageHref": "https://api.mailjet.com/v3/REST"
"/message/12345678901234500",
}
],
"Cc": [],
"Bcc": [],
},
{
"Errors": [
{
"ErrorIdentifier": "f480a5a2-0334-4e08"
"-b2b7-f372ce5669e0",
"ErrorCode": "mj-0013",
"StatusCode": 400,
"ErrorMessage": '"[email protected]" is an invalid'
" email address.",
"ErrorRelatedTo": ["To[0].Email"],
}
],
"Status": "error",
},
]
}
# Mailjet uses 400 for partial success:
self.set_mock_response(raw=response_content, status_code=400)
self.set_mock_response(json_data=response_data, status_code=400)
msg = mail.EmailMessage(
"Subject",
"Message",
Expand All @@ -751,7 +750,7 @@ def test_mixed_status(self):
msg.anymail_status.recipients["[email protected]"].message_id, None
)
self.assertEqual(msg.anymail_status.message_id, {"12345678901234500", None})
self.assertEqual(msg.anymail_status.esp_response.content, response_content)
self.assertEqual(msg.anymail_status.esp_response.json(), response_data)

# noinspection PyUnresolvedReferences
def test_send_failed_anymail_status(self):
Expand All @@ -764,6 +763,18 @@ def test_send_failed_anymail_status(self):
self.assertEqual(self.message.anymail_status.recipients, {})
self.assertIsNone(self.message.anymail_status.esp_response)

def test_non_json_error(self):
"""429 (and other?) errors don't have a json payload"""
self.set_mock_response(
status_code=429, raw=b"Rate limit exceeded", content_type="text/html"
)
with self.assertRaisesRegex(
AnymailRequestsAPIError,
r"^Mailjet API response 429 .*Rate limit exceeded.*",
) as cm:
self.message.send()
self.assertNotIn("Invalid JSON", str(cm.exception))

# noinspection PyUnresolvedReferences
def test_send_unparsable_response(self):
"""
Expand Down
Loading