celery.task.retry exc param should be a throwable.

This causes an issue when it hits the max retry limit, and tries to
throw your exception to let you deal with it - at this point it was
moaning because we pass in a string

if it's not defined, and we're inside an exception block celery uses
that instead.

app/celery/callback_tasks.py

@@ -13,6 +13,6 @@ def process_ses_results(self, response):
         errors = process_ses_response(response)
         if errors:
             current_app.logger.error(errors)
-    except Exception:
+    except Exception as exc:
         current_app.logger.exception('Error processing SES results')
-        self.retry(queue=QueueNames.RETRY, exc="SES responses processed with error")
+        self.retry(queue=QueueNames.RETRY)

app/celery/tasks.py

@@ -323,8 +323,9 @@ def build_dvla_file(self, job_id):
             )
             dao_update_job_status(job_id, JOB_STATUS_READY_TO_SEND)
         else:
-            current_app.logger.info("All notifications for job {} are not persisted".format(job_id))
+            msg = "All notifications for job {} are not persisted".format(job_id)
-            self.retry(queue=QueueNames.RETRY, exc="All notifications for job {} are not persisted".format(job_id))
+            current_app.logger.info(msg)
+            self.retry(queue=QueueNames.RETRY, exc=Exception(msg))
     except Exception as e:
         # ? should this retry?
         current_app.logger.exception("build_dvla_file threw exception")
@@ -520,9 +521,7 @@ def send_inbound_sms_to_service(self, inbound_sms_id, service_id):
         )
         if not isinstance(e, HTTPError) or e.response.status_code >= 500:
             try:
-                self.retry(queue=QueueNames.RETRY,
+                self.retry(queue=QueueNames.RETRY)
-                           exc='Unable to send_inbound_sms_to_service for service_id: {} and url: {}. \n{}'.format(
-                               service_id, inbound_api.url, e))
             except self.MaxRetriesExceededError:
                 current_app.logger.exception('Retry: send_inbound_sms_to_service has retried the max number of times')
 

tests/app/celery/test_tasks.py

@@ -1,8 +1,7 @@
-import codecs
 import json
 import uuid
 from datetime import datetime, timedelta
-from unittest.mock import Mock
+from unittest.mock import Mock, ANY
 import pytest
 import requests_mock
 from flask import current_app
@@ -1049,8 +1048,10 @@ def test_build_dvla_file_retries_if_all_notifications_are_not_created(sample_let
         build_dvla_file(job.id)
     mocked.assert_not_called()
 
-    tasks.build_dvla_file.retry.assert_called_with(queue="retry-tasks",
+    tasks.build_dvla_file.retry.assert_called_with(
-                                                   exc="All notifications for job {} are not persisted".format(job.id))
+        queue="retry-tasks",
+        exc=ANY
+    )
     assert Job.query.get(job.id).job_status == 'in progress'
     mocked_send_task.assert_not_called()
 
@@ -1157,7 +1158,6 @@ def test_send_inbound_sms_to_service_retries_if_request_returns_500(notify_api,
     )
     assert mocked.call_count == 1
     assert mocked.call_args[1]['queue'] == 'retry-tasks'
-    assert exc_msg in mocked.call_args[1]['exc']
 
 
 def test_send_inbound_sms_to_service_retries_if_request_throws_unknown(notify_api, sample_service, mocker):
@@ -1177,7 +1177,6 @@ def test_send_inbound_sms_to_service_retries_if_request_throws_unknown(notify_ap
     )
     assert mocked.call_count == 1
     assert mocked.call_args[1]['queue'] == 'retry-tasks'
-    assert exc_msg in mocked.call_args[1]['exc']
 
 
 def test_send_inbound_sms_to_service_does_not_retries_if_request_returns_404(notify_api, sample_service, mocker):