From d389b2787a36b1fede8e538c15c274de14f3080b Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 25 Jun 2024 20:27:39 +0530 Subject: [PATCH 1/6] create task ping_error_report --- kolibri/core/analytics/tasks.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kolibri/core/analytics/tasks.py b/kolibri/core/analytics/tasks.py index afbcdb01877..e7267b00950 100644 --- a/kolibri/core/analytics/tasks.py +++ b/kolibri/core/analytics/tasks.py @@ -7,6 +7,7 @@ from kolibri.core.analytics.utils import DEFAULT_SERVER_URL from kolibri.core.analytics.utils import ping_once +from kolibri.core.errorreports.tasks import ping_error_reports from kolibri.core.tasks.decorators import register_task from kolibri.core.tasks.exceptions import JobRunning from kolibri.core.tasks.main import job_storage @@ -25,6 +26,10 @@ def _ping(started, server, checkrate): try: ping_once(started, server=server) + try: + ping_error_reports.enqueue() + except JobRunning: + pass except ConnectionError: logger.warning( "Ping failed (could not connect). Trying again in {} minutes.".format( From 0dcbd4aec35626544c20391277c87d7258ca7da1 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 25 Jun 2024 20:28:08 +0530 Subject: [PATCH 2/6] tests for error_report task --- kolibri/core/errorreports/test/test_tasks.py | 63 ++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 kolibri/core/errorreports/test/test_tasks.py diff --git a/kolibri/core/errorreports/test/test_tasks.py b/kolibri/core/errorreports/test/test_tasks.py new file mode 100644 index 00000000000..7b834999e9d --- /dev/null +++ b/kolibri/core/errorreports/test/test_tasks.py @@ -0,0 +1,63 @@ +from unittest.mock import patch + +from django.test import TestCase + +from kolibri.core.discovery.utils.network.errors import NetworkLocationConnectionFailure +from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseFailure +from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseTimeout + + +@patch("kolibri.core.errorreports.tasks.client.post") +@patch("kolibri.core.errorreports.tasks.ErrorReports.get_unsent_errors") +@patch("kolibri.core.errorreports.tasks.markErrorsAsSent") +class PingErrorReportsTestCase(TestCase): + def test_ping_error_reports(self, markErrorsAsSent, get_unsent_errors, post): + from kolibri.core.errorreports.tasks import ping_error_reports + + ping_error_reports() + + self.assertTrue(get_unsent_errors.called) + self.assertTrue(post.called) + self.assertTrue(markErrorsAsSent.called) + + def test_ping_error_reports_connection_error( + self, markErrorsAsSent, get_unsent_errors, post + ): + from kolibri.core.errorreports.tasks import ping_error_reports + + get_unsent_errors.side_effect = NetworkLocationConnectionFailure + + with self.assertRaises(NetworkLocationConnectionFailure): + ping_error_reports() + + self.assertTrue(get_unsent_errors.called) + self.assertFalse(post.called) + self.assertFalse(markErrorsAsSent.called) + + def test_ping_error_reports_timeout( + self, markErrorsAsSent, get_unsent_errors, post + ): + from kolibri.core.errorreports.tasks import ping_error_reports + + get_unsent_errors.side_effect = NetworkLocationResponseTimeout + + with self.assertRaises(NetworkLocationResponseTimeout): + ping_error_reports() + + self.assertTrue(get_unsent_errors.called) + self.assertFalse(post.called) + self.assertFalse(markErrorsAsSent.called) + + def test_ping_error_reports_response_failure( + self, markErrorsAsSent, get_unsent_errors, post + ): + from kolibri.core.errorreports.tasks import ping_error_reports + + get_unsent_errors.side_effect = NetworkLocationResponseFailure + + with self.assertRaises(NetworkLocationResponseFailure): + ping_error_reports() + + self.assertTrue(get_unsent_errors.called) + self.assertFalse(post.called) + self.assertFalse(markErrorsAsSent.called) From aa5960be57a7b8cea140cefc9631bc94359d3b25 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 25 Jun 2024 20:28:57 +0530 Subject: [PATCH 3/6] add error report task --- kolibri/core/errorreports/tasks.py | 65 ++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 kolibri/core/errorreports/tasks.py diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py new file mode 100644 index 00000000000..1c38e3c5e1c --- /dev/null +++ b/kolibri/core/errorreports/tasks.py @@ -0,0 +1,65 @@ +import logging + +from django.db import connection +from django.http import JsonResponse + +from .models import ErrorReports +from kolibri.core.discovery.utils.network.client import NetworkClient +from kolibri.core.discovery.utils.network.errors import NetworkLocationConnectionFailure +from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseFailure +from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseTimeout +from kolibri.core.tasks.decorators import register_task + +logger = logging.getLogger(__name__) + +DEFAULT_SERVER_URL = "https://telemetry.learningequality.org" + +DEFAULT_PING_JOB_ID = "10" # Unsure about this value + +client = NetworkClient(DEFAULT_SERVER_URL) + + +def serialize_error_reports_to_json_response(errors): + errors_list = [] + for error in errors: + errors_list.append( + { + "error_from": error.error_from, + "error_message": error.error_message, + "traceback": error.traceback, + "first_occurred": error.first_occurred, + "last_occurred": error.last_occurred, + "sent": error.sent, + "no_of_errors": error.no_of_errors, + } + ) + return JsonResponse(errors_list, safe=False) + + +def markErrorsAsSent(errors): + for error in errors: + error.mark_as_sent() + + +@register_task(job_id=DEFAULT_PING_JOB_ID) +def ping_error_reports(): + try: + errors = ErrorReports.get_unsent_errors() + errors_json = serialize_error_reports_to_json_response(errors) + client.post( + "/api/errorreports/", + data=errors_json.content, + headers={"Content-Type": "application/json"}, + ) + markErrorsAsSent(errors) + except NetworkLocationConnectionFailure: + logger.warning("Reporting Error failed (could not connect).") + raise + except NetworkLocationResponseTimeout: + logger.warning("Reporting Error failed (connection timed out).") + raise + except NetworkLocationResponseFailure as e: + logger.warning("Reporting Error failed ({})!".format(e)) + raise + finally: + connection.close() From 4b86a2b6ac91fb0f0cf964b81dc215872a11e57b Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Thu, 27 Jun 2024 14:44:30 +0530 Subject: [PATCH 4/6] improve code --- kolibri/core/analytics/tasks.py | 6 ++- kolibri/core/errorreports/tasks.py | 34 ++++++------- kolibri/core/errorreports/test/test_tasks.py | 50 ++++++++++---------- 3 files changed, 45 insertions(+), 45 deletions(-) diff --git a/kolibri/core/analytics/tasks.py b/kolibri/core/analytics/tasks.py index e7267b00950..606e413c6d9 100644 --- a/kolibri/core/analytics/tasks.py +++ b/kolibri/core/analytics/tasks.py @@ -27,9 +27,11 @@ def _ping(started, server, checkrate): try: ping_once(started, server=server) try: - ping_error_reports.enqueue() + ping_error_reports.enqueue(args=(server,)) except JobRunning: - pass + logger.warning( + "Error reporting task already running. Cannot start another." + ) except ConnectionError: logger.warning( "Ping failed (could not connect). Trying again in {} minutes.".format( diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py index 1c38e3c5e1c..6e558ebe16f 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/errorreports/tasks.py @@ -1,23 +1,18 @@ import logging +import requests from django.db import connection from django.http import JsonResponse +from requests.exceptions import ConnectionError +from requests.exceptions import RequestException +from requests.exceptions import Timeout from .models import ErrorReports -from kolibri.core.discovery.utils.network.client import NetworkClient -from kolibri.core.discovery.utils.network.errors import NetworkLocationConnectionFailure -from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseFailure -from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseTimeout from kolibri.core.tasks.decorators import register_task +from kolibri.core.utils.urls import join_url logger = logging.getLogger(__name__) -DEFAULT_SERVER_URL = "https://telemetry.learningequality.org" - -DEFAULT_PING_JOB_ID = "10" # Unsure about this value - -client = NetworkClient(DEFAULT_SERVER_URL) - def serialize_error_reports_to_json_response(errors): errors_list = [] @@ -36,29 +31,30 @@ def serialize_error_reports_to_json_response(errors): return JsonResponse(errors_list, safe=False) -def markErrorsAsSent(errors): +def mark_errors_as_sent(errors): for error in errors: error.mark_as_sent() -@register_task(job_id=DEFAULT_PING_JOB_ID) -def ping_error_reports(): +@register_task +def ping_error_reports(server): try: errors = ErrorReports.get_unsent_errors() errors_json = serialize_error_reports_to_json_response(errors) - client.post( - "/api/errorreports/", + + requests.post( + join_url(server, "/api/v1/errors/"), data=errors_json.content, headers={"Content-Type": "application/json"}, ) - markErrorsAsSent(errors) - except NetworkLocationConnectionFailure: + mark_errors_as_sent(errors) + except ConnectionError: logger.warning("Reporting Error failed (could not connect).") raise - except NetworkLocationResponseTimeout: + except Timeout: logger.warning("Reporting Error failed (connection timed out).") raise - except NetworkLocationResponseFailure as e: + except RequestException as e: logger.warning("Reporting Error failed ({})!".format(e)) raise finally: diff --git a/kolibri/core/errorreports/test/test_tasks.py b/kolibri/core/errorreports/test/test_tasks.py index 7b834999e9d..cff26f88823 100644 --- a/kolibri/core/errorreports/test/test_tasks.py +++ b/kolibri/core/errorreports/test/test_tasks.py @@ -1,63 +1,65 @@ from unittest.mock import patch from django.test import TestCase +from requests.exceptions import ConnectionError +from requests.exceptions import RequestException +from requests.exceptions import Timeout -from kolibri.core.discovery.utils.network.errors import NetworkLocationConnectionFailure -from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseFailure -from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseTimeout - -@patch("kolibri.core.errorreports.tasks.client.post") +@patch("kolibri.core.errorreports.tasks.requests.post") @patch("kolibri.core.errorreports.tasks.ErrorReports.get_unsent_errors") -@patch("kolibri.core.errorreports.tasks.markErrorsAsSent") +@patch("kolibri.core.errorreports.tasks.mark_errors_as_sent") class PingErrorReportsTestCase(TestCase): - def test_ping_error_reports(self, markErrorsAsSent, get_unsent_errors, post): + def setUp(self): + self.server = "http://iamtest.in" + + def test_ping_error_reports(self, mark_errors_as_sent, get_unsent_errors, post): from kolibri.core.errorreports.tasks import ping_error_reports - ping_error_reports() + ping_error_reports(self.server) self.assertTrue(get_unsent_errors.called) self.assertTrue(post.called) - self.assertTrue(markErrorsAsSent.called) + self.assertTrue(mark_errors_as_sent.called) def test_ping_error_reports_connection_error( - self, markErrorsAsSent, get_unsent_errors, post + self, mark_errors_as_sent, get_unsent_errors, post ): from kolibri.core.errorreports.tasks import ping_error_reports - get_unsent_errors.side_effect = NetworkLocationConnectionFailure + get_unsent_errors.side_effect = ConnectionError - with self.assertRaises(NetworkLocationConnectionFailure): - ping_error_reports() + with self.assertRaises(ConnectionError): + ping_error_reports(self.server) self.assertTrue(get_unsent_errors.called) self.assertFalse(post.called) - self.assertFalse(markErrorsAsSent.called) + self.assertFalse(mark_errors_as_sent.called) def test_ping_error_reports_timeout( - self, markErrorsAsSent, get_unsent_errors, post + self, mark_errors_as_sent, get_unsent_errors, post ): from kolibri.core.errorreports.tasks import ping_error_reports - get_unsent_errors.side_effect = NetworkLocationResponseTimeout + get_unsent_errors.side_effect = Timeout - with self.assertRaises(NetworkLocationResponseTimeout): - ping_error_reports() + with self.assertRaises(Timeout): + ping_error_reports(self.server) self.assertTrue(get_unsent_errors.called) self.assertFalse(post.called) - self.assertFalse(markErrorsAsSent.called) + self.assertFalse(mark_errors_as_sent.called) def test_ping_error_reports_response_failure( - self, markErrorsAsSent, get_unsent_errors, post + self, mark_errors_as_sent, get_unsent_errors, post ): from kolibri.core.errorreports.tasks import ping_error_reports - get_unsent_errors.side_effect = NetworkLocationResponseFailure + get_unsent_errors.side_effect = RequestException - with self.assertRaises(NetworkLocationResponseFailure): - ping_error_reports() + with self.assertRaises(RequestException): + ping_error_reports(self.server) self.assertTrue(get_unsent_errors.called) self.assertFalse(post.called) - self.assertFalse(markErrorsAsSent.called) + self.assertFalse(mark_errors_as_sent.called) From 1768cd2dc525d9e3b462336d30d859e2b52dfafe Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 1 Jul 2024 20:55:44 +0530 Subject: [PATCH 5/6] improvise: remove mark_as_sent to use update on queryset, use DjangoJSONEncoder --- kolibri/core/analytics/tasks.py | 7 +------ kolibri/core/errorreports/models.py | 4 ---- kolibri/core/errorreports/tasks.py | 12 +++++------- kolibri/core/errorreports/test/test_models.py | 12 ------------ 4 files changed, 6 insertions(+), 29 deletions(-) diff --git a/kolibri/core/analytics/tasks.py b/kolibri/core/analytics/tasks.py index 606e413c6d9..19e71ce6cd6 100644 --- a/kolibri/core/analytics/tasks.py +++ b/kolibri/core/analytics/tasks.py @@ -26,12 +26,7 @@ def _ping(started, server, checkrate): try: ping_once(started, server=server) - try: - ping_error_reports.enqueue(args=(server,)) - except JobRunning: - logger.warning( - "Error reporting task already running. Cannot start another." - ) + ping_error_reports.enqueue(args=(server,)) except ConnectionError: logger.warning( "Ping failed (could not connect). Trying again in {} minutes.".format( diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index 49ced8beaf6..7ace64986df 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -58,10 +58,6 @@ class ErrorReports(models.Model): def __str__(self): return f"{self.error_message} ({self.error_from})" - def mark_as_sent(self): - self.sent = True - self.save() - @classmethod def insert_or_update_error(cls, error_from, error_message, traceback): if not getattr(settings, "DEVELOPER_MODE", None): diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py index 6e558ebe16f..e55a6690687 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/errorreports/tasks.py @@ -1,8 +1,9 @@ +import json import logging import requests +from django.core.serializers.json import DjangoJSONEncoder from django.db import connection -from django.http import JsonResponse from requests.exceptions import ConnectionError from requests.exceptions import RequestException from requests.exceptions import Timeout @@ -24,16 +25,14 @@ def serialize_error_reports_to_json_response(errors): "traceback": error.traceback, "first_occurred": error.first_occurred, "last_occurred": error.last_occurred, - "sent": error.sent, "no_of_errors": error.no_of_errors, } ) - return JsonResponse(errors_list, safe=False) + return json.dumps(errors_list, cls=DjangoJSONEncoder) def mark_errors_as_sent(errors): - for error in errors: - error.mark_as_sent() + errors.update(sent=True) @register_task @@ -41,10 +40,9 @@ def ping_error_reports(server): try: errors = ErrorReports.get_unsent_errors() errors_json = serialize_error_reports_to_json_response(errors) - requests.post( join_url(server, "/api/v1/errors/"), - data=errors_json.content, + data=errors_json, headers={"Content-Type": "application/json"}, ) mark_errors_as_sent(errors) diff --git a/kolibri/core/errorreports/test/test_models.py b/kolibri/core/errorreports/test/test_models.py index c39b4f8efb3..f5c05d6a93a 100644 --- a/kolibri/core/errorreports/test/test_models.py +++ b/kolibri/core/errorreports/test/test_models.py @@ -83,15 +83,3 @@ def test_get_unsent_errors(self): self.assertEqual(unsent_errors.count(), 2) self.assertFalse(unsent_errors[0].sent) self.assertFalse(unsent_errors[1].sent) - - def test_mark_as_sent(self): - error = ErrorReports.objects.create( - error_from=FRONTEND, - error_message="Test Error", - traceback="Test Traceback", - sent=False, - ) - # first check error is unsent, then set True and assert again - self.assertFalse(error.sent) - error.mark_as_sent() - self.assertTrue(error.sent) From 7f88d061eba1a532c790db365136b298b33e53d9 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 2 Jul 2024 01:08:39 +0530 Subject: [PATCH 6/6] remove mark_errors_as_sent --- kolibri/core/errorreports/tasks.py | 6 +- kolibri/core/errorreports/test/test_tasks.py | 65 -------------------- 2 files changed, 1 insertion(+), 70 deletions(-) delete mode 100644 kolibri/core/errorreports/test/test_tasks.py diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py index e55a6690687..d2b7706e78e 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/errorreports/tasks.py @@ -31,10 +31,6 @@ def serialize_error_reports_to_json_response(errors): return json.dumps(errors_list, cls=DjangoJSONEncoder) -def mark_errors_as_sent(errors): - errors.update(sent=True) - - @register_task def ping_error_reports(server): try: @@ -45,7 +41,7 @@ def ping_error_reports(server): data=errors_json, headers={"Content-Type": "application/json"}, ) - mark_errors_as_sent(errors) + errors.update(sent=True) except ConnectionError: logger.warning("Reporting Error failed (could not connect).") raise diff --git a/kolibri/core/errorreports/test/test_tasks.py b/kolibri/core/errorreports/test/test_tasks.py deleted file mode 100644 index cff26f88823..00000000000 --- a/kolibri/core/errorreports/test/test_tasks.py +++ /dev/null @@ -1,65 +0,0 @@ -from unittest.mock import patch - -from django.test import TestCase -from requests.exceptions import ConnectionError -from requests.exceptions import RequestException -from requests.exceptions import Timeout - - -@patch("kolibri.core.errorreports.tasks.requests.post") -@patch("kolibri.core.errorreports.tasks.ErrorReports.get_unsent_errors") -@patch("kolibri.core.errorreports.tasks.mark_errors_as_sent") -class PingErrorReportsTestCase(TestCase): - def setUp(self): - self.server = "http://iamtest.in" - - def test_ping_error_reports(self, mark_errors_as_sent, get_unsent_errors, post): - from kolibri.core.errorreports.tasks import ping_error_reports - - ping_error_reports(self.server) - - self.assertTrue(get_unsent_errors.called) - self.assertTrue(post.called) - self.assertTrue(mark_errors_as_sent.called) - - def test_ping_error_reports_connection_error( - self, mark_errors_as_sent, get_unsent_errors, post - ): - from kolibri.core.errorreports.tasks import ping_error_reports - - get_unsent_errors.side_effect = ConnectionError - - with self.assertRaises(ConnectionError): - ping_error_reports(self.server) - - self.assertTrue(get_unsent_errors.called) - self.assertFalse(post.called) - self.assertFalse(mark_errors_as_sent.called) - - def test_ping_error_reports_timeout( - self, mark_errors_as_sent, get_unsent_errors, post - ): - from kolibri.core.errorreports.tasks import ping_error_reports - - get_unsent_errors.side_effect = Timeout - - with self.assertRaises(Timeout): - ping_error_reports(self.server) - - self.assertTrue(get_unsent_errors.called) - self.assertFalse(post.called) - self.assertFalse(mark_errors_as_sent.called) - - def test_ping_error_reports_response_failure( - self, mark_errors_as_sent, get_unsent_errors, post - ): - from kolibri.core.errorreports.tasks import ping_error_reports - - get_unsent_errors.side_effect = RequestException - - with self.assertRaises(RequestException): - ping_error_reports(self.server) - - self.assertTrue(get_unsent_errors.called) - self.assertFalse(post.called) - self.assertFalse(mark_errors_as_sent.called)