Merge pull request #716 from alphagov/error-handling

Error handling

app/celery/scheduled_tasks.py

@@ -31,7 +31,7 @@ def run_scheduled_jobs():
             process_job.apply_async([str(job.id)], queue="process-job")
             current_app.logger.info("Job ID {} added to process job queue".format(job.id))
     except SQLAlchemyError as e:
-        current_app.logger.exception("Failed to run scheduled jobs", e)
+        current_app.logger.exception("Failed to run scheduled jobs")
         raise
 
 
@@ -45,7 +45,7 @@ def delete_verify_codes():
             "Delete job started {} finished {} deleted {} verify codes".format(start, datetime.utcnow(), deleted)
         )
     except SQLAlchemyError as e:
-        current_app.logger.exception("Failed to delete verify codes", e)
+        current_app.logger.exception("Failed to delete verify codes")
         raise
 
 
@@ -63,7 +63,7 @@ def delete_successful_notifications():
             )
         )
     except SQLAlchemyError as e:
-        current_app.logger.exception("Failed to delete successful notifications", e)
+        current_app.logger.exception("Failed to delete successful notifications")
         raise
 
 
@@ -84,7 +84,7 @@ def delete_failed_notifications():
             )
         )
     except SQLAlchemyError as e:
-        current_app.logger.exception("Failed to delete failed notifications", e)
+        current_app.logger.exception("Failed to delete failed notifications")
         raise
 
 
@@ -98,7 +98,7 @@ def delete_invitations():
             "Delete job started {} finished {} deleted {} invitations".format(start, datetime.utcnow(), deleted)
         )
     except SQLAlchemyError as e:
-        current_app.logger.exception("Failed to delete invitations", e)
+        current_app.logger.exception("Failed to delete invitations")
         raise
 
 

app/errors.py

@@ -42,10 +42,7 @@ def register_errors(blueprint):
 
     @blueprint.app_errorhandler(400)
     def bad_request(e):
-        if isinstance(e, str):
+        msg = e.description or "Invalid request parameters"
-            msg = e
-        else:
-            msg = e.description or "Invalid request parameters"
         current_app.logger.exception(msg)
         return jsonify(result='error', message=str(msg)), 400
 
@@ -61,10 +58,7 @@ def register_errors(blueprint):
 
     @blueprint.app_errorhandler(404)
     def page_not_found(e):
-        if isinstance(e, str):
+        msg = e.description or "Not found"
-            msg = e
-        else:
-            msg = e.description or "Not found"
         current_app.logger.exception(msg)
         return jsonify(result='error', message=msg), 404
 
@@ -73,18 +67,9 @@ def register_errors(blueprint):
         current_app.logger.exception(e)
         return jsonify(result='error', message=str(e.description)), 429
 
-    @blueprint.app_errorhandler(500)
-    def internal_server_error(e):
-        current_app.logger.exception(e)
-        return jsonify(result='error', message="Internal server error"), 500
-
     @blueprint.app_errorhandler(NoResultFound)
-    def no_result_found(e):
-        current_app.logger.exception(e)
-        return jsonify(result='error', message="No result found"), 404
-
     @blueprint.app_errorhandler(DataError)
-    def data_error(e):
+    def no_result_found(e):
         current_app.logger.exception(e)
         return jsonify(result='error', message="No result found"), 404
 
@@ -101,3 +86,10 @@ def register_errors(blueprint):
                 )]}
             ), 400
         return jsonify(result='error', message="Internal server error"), 500
+
+    # this must be defined after all other error handlers since it catches the generic Exception object
+    @blueprint.app_errorhandler(500)
+    @blueprint.app_errorhandler(Exception)
+    def internal_server_error(e):
+        current_app.logger.exception(e)
+        return jsonify(result='error', message="Internal server error"), 500

app/notifications/rest.py

@@ -309,7 +309,7 @@ def persist_notification(
                 queue='send-email' if not research_mode else 'research-mode'
             )
     except Exception as e:
-        current_app.logger.exception("Failed to send to SQS exception", e)
+        current_app.logger.exception("Failed to send to SQS exception")
         dao_delete_notifications_and_history_by_id(notification_id)
         raise InvalidRequest(message="Internal server error", status_code=500)