From 8ba1e90f9ab7876a0d5ef9f323ccba46ce5c37dd Mon Sep 17 00:00:00 2001 From: Sartography Date: Mon, 7 Dec 2020 08:39:00 -0500 Subject: [PATCH 1/3] Test for ticket 153 to include user in error messages --- tests/test_user_in_logs.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 tests/test_user_in_logs.py diff --git a/tests/test_user_in_logs.py b/tests/test_user_in_logs.py new file mode 100644 index 00000000..1ebb9b79 --- /dev/null +++ b/tests/test_user_in_logs.py @@ -0,0 +1,21 @@ +from crc.api.common import ApiError +from tests.base_test import BaseTest + + +class TestUserID(BaseTest): + + def test_user_id(self): + # try: + # False + # except: + # raise ApiError + + # ApiError() + # self.assertEqual(True,False) + + # with self.assertRaises(ApiError) as api_error: + # self.assertEqual(2, 3) + workflow = self.create_workflow('email') + first_task = self.get_workflow_api(workflow).next_task + + raise ApiError('unknown_approval', 'Please provide a valid Approval ID.') From 856fe445b0b85c70ddbd41339174da8b3473c9ee Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Fri, 11 Dec 2020 16:26:03 -0500 Subject: [PATCH 2/3] Added user.uid to ApiError and Sentry logging --- crc/api/common.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crc/api/common.py b/crc/api/common.py index e2b13b9a..78cbc018 100644 --- a/crc/api/common.py +++ b/crc/api/common.py @@ -4,6 +4,8 @@ from flask import g from crc import ma, app +import sentry_sdk + class ApiError(Exception): def __init__(self, code, message, status_code=400, @@ -16,6 +18,13 @@ class ApiError(Exception): self.file_name = file_name or "" # OPTIONAL: The file that caused the error. self.tag = tag or "" # OPTIONAL: The XML Tag that caused the issue. self.task_data = task_data or "" # OPTIONAL: A snapshot of data connected to the task when error ocurred. + if hasattr(g,'user'): + user = g.user.uid + else: + user = 'Unknown' + self.task_user = user + # This is for sentry logging into Slack + sentry_sdk.set_context("User", {'user': user}) Exception.__init__(self, self.message) @classmethod @@ -59,7 +68,7 @@ class ApiError(Exception): class ApiErrorSchema(ma.Schema): class Meta: fields = ("code", "message", "workflow_name", "file_name", "task_name", "task_id", - "task_data") + "task_data", "task_user") @app.errorhandler(ApiError) From 7defc2b02f1eff37696dbc3dc5d863254971ccc6 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Fri, 11 Dec 2020 17:47:53 -0500 Subject: [PATCH 3/3] Tests for uid in logs. Currently we test for uid in a response. This covers ApiError. Currently, we don't have a test for Sentry. Unsure how to do this. Also added a script, service and test workflow to help. (Also to learn about adding a script and service.) --- crc/scripts/failing_script.py | 15 ++++++ crc/services/failing_service.py | 11 ++++ .../failing_workflow/failing_workflow.bpmn | 52 +++++++++++++++++++ tests/test_user_in_logs.py | 35 ++++++++----- 4 files changed, 99 insertions(+), 14 deletions(-) create mode 100644 crc/scripts/failing_script.py create mode 100644 crc/services/failing_service.py create mode 100644 tests/data/failing_workflow/failing_workflow.bpmn diff --git a/crc/scripts/failing_script.py b/crc/scripts/failing_script.py new file mode 100644 index 00000000..2e381cd2 --- /dev/null +++ b/crc/scripts/failing_script.py @@ -0,0 +1,15 @@ +from crc.scripts.script import Script +from crc.services.failing_service import FailingService + + +class FailingScript(Script): + + def get_description(self): + return """It fails""" + + def do_task_validate_only(self, task, *args, **kwargs): + pass + + def do_task(self, task, *args, **kwargs): + + FailingService.fail_as_service() \ No newline at end of file diff --git a/crc/services/failing_service.py b/crc/services/failing_service.py new file mode 100644 index 00000000..6afae9fc --- /dev/null +++ b/crc/services/failing_service.py @@ -0,0 +1,11 @@ +from crc.api.common import ApiError + + +class FailingService(object): + + @staticmethod + def fail_as_service(): + """It fails""" + + raise ApiError(code='bad_service', + message='This is my failing service') diff --git a/tests/data/failing_workflow/failing_workflow.bpmn b/tests/data/failing_workflow/failing_workflow.bpmn new file mode 100644 index 00000000..36869390 --- /dev/null +++ b/tests/data/failing_workflow/failing_workflow.bpmn @@ -0,0 +1,52 @@ + + + + + Flow_0cszvz2 + + + + Flow_0cszvz2 + Flow_1l02umo + failing_script() + + + + Flow_1l02umo + Flow_08zq7mf + print('I am a placeholder.') + + + Flow_08zq7mf + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_user_in_logs.py b/tests/test_user_in_logs.py index 1ebb9b79..e6c65447 100644 --- a/tests/test_user_in_logs.py +++ b/tests/test_user_in_logs.py @@ -1,21 +1,28 @@ -from crc.api.common import ApiError from tests.base_test import BaseTest +from crc import db +from crc.models.user import UserModel +import json class TestUserID(BaseTest): - def test_user_id(self): - # try: - # False - # except: - # raise ApiError + def test_user_id_in_request(self): + """This assures the uid is in response via ApiError""" - # ApiError() - # self.assertEqual(True,False) + workflow = self.create_workflow('failing_workflow') + user_uid = workflow.study.user_uid + user = db.session.query(UserModel).filter_by(uid=user_uid).first() + rv = self.app.get(f'/v1.0/workflow/{workflow.id}' + f'?soft_reset={str(False)}' + f'&hard_reset={str(False)}' + f'&do_engine_steps={str(True)}', + headers=self.logged_in_headers(user), + content_type="application/json") + data = json.loads(rv.data) + self.assertEqual(data['task_user'], user_uid) - # with self.assertRaises(ApiError) as api_error: - # self.assertEqual(2, 3) - workflow = self.create_workflow('email') - first_task = self.get_workflow_api(workflow).next_task - - raise ApiError('unknown_approval', 'Please provide a valid Approval ID.') + def test_user_id_in_sentry(self): + """This assures the uid is in Sentry. + We use this to send errors to Slack.""" + # Currently have no clue how to do this :( + pass