darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2025-12-22 16:31:15 -05:00
Code Issues Packages Projects Releases Wiki Activity

fix invalid logging

Browse Source 
the first argument to ANY logger.____ function is ALWAYS cast to a
string and used as a format argument for ALL remaining arguments
using %s formatting. even `logger.exception`, which just logs as
normal and then appends the stack trace.

so we shouldn't be passing `e` into logger.exception - just
`logger.exception('something went wrong!')`

also de-duplicated a test
This commit is contained in:
Leo Hemsted
2016-12-15 17:30:05 +00:00
parent a197754906
commit 0136e1e32d
3 changed files with 6 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
app/celery/provider_tasks.py
View File

@@ -43,15 +43,13 @@ def deliver_sms(self, notification_id):
send_to_providers.send_sms_to_provider(notification)
except Exception as e:
try:
current_app.logger.error(
current_app.logger.exception(
"RETRY: SMS notification {} failed".format(notification_id)
)
current_app.logger.exception(e)
self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries))
except self.MaxRetriesExceededError:
current_app.logger.error(
current_app.logger.exception(
"RETRY FAILED: task send_sms_to_provider failed for notification {}".format(notification_id),
e
)
update_notification_status_by_id(notification_id, 'technical-failure')
@@ -69,14 +67,12 @@ def deliver_email(self, notification_id):
update_notification_status_by_id(notification_id, 'technical-failure')
except Exception as e:
try:
current_app.logger.error(
current_app.logger.exception(
"RETRY: Email notification {} failed".format(notification_id)
)
current_app.logger.exception(e)
self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries))
except self.MaxRetriesExceededError:
current_app.logger.error(
"RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification_id),
e
"RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification_id)
)
update_notification_status_by_id(notification_id, 'technical-failure')

2
config.py
View File

@@ -137,7 +137,7 @@ class Config(object):
REDIS_ENABLED = False
SENDING_NOTIFICATIONS_TIMEOUT_PERIOD = 259200
SENDING_NOTIFICATIONS_TIMEOUT_PERIOD = 259200 # 3 days
SIMULATED_EMAIL_ADDRESSES = ('simulate-delivered@notifications.service.gov.uk',
'simulate-permanent-failure@notifications.service.gov.uk',

2
tests/app/clients/test_firetext.py
View File

@@ -98,7 +98,7 @@ def test_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client):
assert '"code": 1' in exc.value.text
def test_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client):
def test_send_sms_raises_if_firetext_rejects_with_unexpected_data(mocker, mock_firetext_client):
to = content = reference = 'foo'
response_dict = {"something": "gone bad"}