Skip to content

Commit

Permalink
Merge pull request #458 from alphagov/ldeb-add-mailchimp-json-respons…
Browse files Browse the repository at this point in the history
…e-to-error-logs

Update Mailchimp error logging to include the response JSON
  • Loading branch information
lfdebrux authored Sep 11, 2018
2 parents 9c56fa5 + 0b2142e commit c9d4766
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 17 deletions.
2 changes: 1 addition & 1 deletion dmutils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
from .flask_init import init_app, init_manager


__version__ = '44.0.1'
__version__ = '44.1.0'
51 changes: 35 additions & 16 deletions dmutils/email/dm_mailchimp.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@
PAGINATION_SIZE = 1000


def get_response_from_request_exception(exc):
try:
return exc.response.json()
except (AttributeError, ValueError, JSONDecodeError):
return {}


class DMMailChimpClient(object):

def __init__(
Expand Down Expand Up @@ -56,7 +63,10 @@ def create_campaign(self, campaign_data):
"Mailchimp failed to create campaign for '{campaign_title}'".format(
campaign_title=campaign_data.get("settings", {}).get("title")
),
extra={"error": str(e)}
extra={
"error": str(e),
"mailchimp_response": get_response_from_request_exception(e),
},
)
return False

Expand All @@ -67,7 +77,10 @@ def set_campaign_content(self, campaign_id, content_data):
except RequestException as e:
self.logger.error(
"Mailchimp failed to set content for campaign id '{0}'".format(campaign_id),
extra={"error": str(e)}
extra={
"error": str(e),
"mailchimp_response": get_response_from_request_exception(e),
},
)
return False

Expand All @@ -79,7 +92,10 @@ def send_campaign(self, campaign_id):
except RequestException as e:
self.logger.error(
"Mailchimp failed to send campaign id '{0}'".format(campaign_id),
extra={"error": str(e)}
extra={
"error": str(e),
"mailchimp_response": get_response_from_request_exception(e),
}
)
return False

Expand All @@ -97,28 +113,31 @@ def subscribe_new_email_to_list(self, list_id, email_address):
}
)
except RequestException as e:
response = get_response_from_request_exception(e)

# As defined in mailchimp API documentation, this particular error message may arise if a user has requested
# mailchimp to never add them to mailchimp lists. In this case, we resort to allowing a failed API call (but
# log) as a user of this method would unlikely be able to do anything as we have no control over this
# behaviour.
try:
if getattr(e, "response", None) is not None and (
"looks fake or invalid, please enter a real email address." in e.response.json().get("detail", "")
):
self.logger.error(
f"Expected error: Mailchimp failed to add user ({hashed_email}) to list ({list_id}). "
"API error: The email address looks fake or invalid, please enter a real email address.",
extra={"error": str(e)}
)
return True
except JSONDecodeError:
pass
if "looks fake or invalid, please enter a real email address." in response.get("detail", ""):
self.logger.error(
f"Expected error: Mailchimp failed to add user ({hashed_email}) to list ({list_id}). "
"API error: The email address looks fake or invalid, please enter a real email address.",
extra={
"error": str(e),
"mailchimp_response": response,
}
)
return True
self.logger.error(
"Mailchimp failed to add user ({}) to list ({})".format(
hashed_email,
list_id
),
extra={"error": str(e)}
extra={
"error": str(e),
"mailchimp_response": response,
}
)
return False

Expand Down

0 comments on commit c9d4766

Please sign in to comment.