From d814aa13b34670cce6ec8e69c8ebbe56c44d958f Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Thu, 6 Jun 2024 01:27:23 +0530 Subject: [PATCH 01/74] add errors in additional_sqlite_databases --- kolibri/deployment/default/sqlite_db_names.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kolibri/deployment/default/sqlite_db_names.py b/kolibri/deployment/default/sqlite_db_names.py index 3118a4049ff..928d95a528e 100644 --- a/kolibri/deployment/default/sqlite_db_names.py +++ b/kolibri/deployment/default/sqlite_db_names.py @@ -10,5 +10,7 @@ NOTIFICATIONS = "notifications" +ERRORS = "errors" -ADDITIONAL_SQLITE_DATABASES = (SYNC_QUEUE, NETWORK_LOCATION, NOTIFICATIONS) + +ADDITIONAL_SQLITE_DATABASES = (SYNC_QUEUE, NETWORK_LOCATION, NOTIFICATIONS, ERRORS) From afe3ad97796d2bbad6e0991a7ed44e4abed0fcf5 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Thu, 6 Jun 2024 02:34:36 +0530 Subject: [PATCH 02/74] add errorreports database in aditional sqlite db --- kolibri/deployment/default/sqlite_db_names.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/kolibri/deployment/default/sqlite_db_names.py b/kolibri/deployment/default/sqlite_db_names.py index 928d95a528e..46f54ea8885 100644 --- a/kolibri/deployment/default/sqlite_db_names.py +++ b/kolibri/deployment/default/sqlite_db_names.py @@ -10,7 +10,12 @@ NOTIFICATIONS = "notifications" -ERRORS = "errors" +ERROR_REPORTS = "errorreports" -ADDITIONAL_SQLITE_DATABASES = (SYNC_QUEUE, NETWORK_LOCATION, NOTIFICATIONS, ERRORS) +ADDITIONAL_SQLITE_DATABASES = ( + SYNC_QUEUE, + NETWORK_LOCATION, + NOTIFICATIONS, + ERROR_REPORTS, +) From 5e10f5309cd6610db09b3d2127810339077189af Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Thu, 6 Jun 2024 02:35:07 +0530 Subject: [PATCH 03/74] create new errorreports app --- kolibri/core/errorreports/__init__.py | 0 kolibri/core/errorreports/apps.py | 10 ++++++++++ 2 files changed, 10 insertions(+) create mode 100644 kolibri/core/errorreports/__init__.py create mode 100644 kolibri/core/errorreports/apps.py diff --git a/kolibri/core/errorreports/__init__.py b/kolibri/core/errorreports/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/kolibri/core/errorreports/apps.py b/kolibri/core/errorreports/apps.py new file mode 100644 index 00000000000..afcb4477a8b --- /dev/null +++ b/kolibri/core/errorreports/apps.py @@ -0,0 +1,10 @@ +from django.apps import AppConfig + + +class KolibriErrorConfig(AppConfig): + name = "kolibri.core.errorreports" + label = "errorreports" + verbose_name = "Kolibri ErrorReports" + + def ready(self): + ... From abb63f369b8ef3f978890d39cb18beb850ef00dc Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Thu, 6 Jun 2024 02:37:44 +0530 Subject: [PATCH 04/74] add ErrorReports db router --- kolibri/core/errorreports/models.py | 38 +++++++++++++++++++++ kolibri/deployment/default/settings/base.py | 2 ++ 2 files changed, 40 insertions(+) create mode 100644 kolibri/core/errorreports/models.py diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py new file mode 100644 index 00000000000..cc2b608e5b0 --- /dev/null +++ b/kolibri/core/errorreports/models.py @@ -0,0 +1,38 @@ +from kolibri.deployment.default.sqlite_db_names import ERROR_REPORTS + + +class ErrorReportsRouter(object): + """ + Determine how to route database calls for the ErrorReports app. + ref: https://docs.djangoproject.com/en/5.0/topics/db/multi-db/ + + """ + + def db_for_read(self, model, **hints): + if model._meta.app_label == "errorreports": + return ERROR_REPORTS + return None + + def db_for_write(self, model, **hints): + if model._meta.app_label == "errorreports": + return ERROR_REPORTS + return None + + def allow_relation(self, obj1, obj2, **hints): + if ( + obj1._meta.app_label == "errorreports" + and obj2._meta.app_label == "errorreports" + ): + return True + elif "errorreports" not in [obj1._meta.app_label, obj2._meta.app_label]: + return None + + return False + + def allow_migrate(self, db, app_label, model_name=None, **hints): + if app_label == "errorreports": + return db == ERROR_REPORTS + elif db == ERROR_REPORTS: + return False + + return None diff --git a/kolibri/deployment/default/settings/base.py b/kolibri/deployment/default/settings/base.py index 8206a1b8a88..3c0f24fea2b 100644 --- a/kolibri/deployment/default/settings/base.py +++ b/kolibri/deployment/default/settings/base.py @@ -73,6 +73,7 @@ "kolibri.core.auth.apps.KolibriAuthConfig", "kolibri.core.bookmarks", "kolibri.core.content", + "kolibri.core.errorreports", "kolibri.core.logger", "kolibri.core.notifications.apps.KolibriNotificationsConfig", "kolibri.core.tasks.apps.KolibriTasksConfig", @@ -161,6 +162,7 @@ "kolibri.core.notifications.models.NotificationsRouter", "kolibri.core.device.models.SyncQueueRouter", "kolibri.core.discovery.models.NetworkLocationRouter", + "kolibri.core.errorreports.models.ErrorReportsRouter", ) elif conf.OPTIONS["Database"]["DATABASE_ENGINE"] == "postgres": From 0e01b17bb49b2f3c7e79dbde24dcf481ccd87560 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 11 Jun 2024 14:15:28 +0530 Subject: [PATCH 05/74] change ellipsis to pass --- kolibri/core/errorreports/apps.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/errorreports/apps.py b/kolibri/core/errorreports/apps.py index afcb4477a8b..967191266d0 100644 --- a/kolibri/core/errorreports/apps.py +++ b/kolibri/core/errorreports/apps.py @@ -7,4 +7,4 @@ class KolibriErrorConfig(AppConfig): verbose_name = "Kolibri ErrorReports" def ready(self): - ... + pass From 20e540c146d85517f03208a46f6dfeed9720be63 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 11 Jun 2024 16:52:03 +0530 Subject: [PATCH 06/74] remove unused ready --- kolibri/core/errorreports/apps.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/kolibri/core/errorreports/apps.py b/kolibri/core/errorreports/apps.py index 967191266d0..ae71f967170 100644 --- a/kolibri/core/errorreports/apps.py +++ b/kolibri/core/errorreports/apps.py @@ -5,6 +5,3 @@ class KolibriErrorConfig(AppConfig): name = "kolibri.core.errorreports" label = "errorreports" verbose_name = "Kolibri ErrorReports" - - def ready(self): - pass From 8b0d8cf3781c6d006f169ea590cdae56dc517eb6 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Fri, 7 Jun 2024 15:40:41 +0530 Subject: [PATCH 07/74] add ErrorReports model with and its class methods --- .../errorreports/migrations/0001_initial.py | 47 +++++++++++++++++++ .../core/errorreports/migrations/__init__.py | 0 kolibri/core/errorreports/models.py | 46 ++++++++++++++++++ 3 files changed, 93 insertions(+) create mode 100644 kolibri/core/errorreports/migrations/0001_initial.py create mode 100644 kolibri/core/errorreports/migrations/__init__.py diff --git a/kolibri/core/errorreports/migrations/0001_initial.py b/kolibri/core/errorreports/migrations/0001_initial.py new file mode 100644 index 00000000000..c25dded817d --- /dev/null +++ b/kolibri/core/errorreports/migrations/0001_initial.py @@ -0,0 +1,47 @@ +# Generated by Django 3.2.25 on 2024-06-07 09:20 +import django.utils.timezone +from django.db import migrations +from django.db import models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="ErrorReports", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "error_from", + models.CharField( + choices=[("frontend", "Frontend"), ("backend", "Backend")], + max_length=10, + ), + ), + ("error_message", models.CharField(max_length=255)), + ("traceback", models.TextField()), + ( + "first_occurred", + models.DateTimeField(default=django.utils.timezone.now), + ), + ( + "last_occurred", + models.DateTimeField(default=django.utils.timezone.now), + ), + ("sent", models.BooleanField(default=False)), + ("no_of_errors", models.IntegerField(default=1)), + ], + ), + ] diff --git a/kolibri/core/errorreports/migrations/__init__.py b/kolibri/core/errorreports/migrations/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index cc2b608e5b0..c23f466b17b 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -1,3 +1,6 @@ +from django.db import models +from django.utils import timezone + from kolibri.deployment.default.sqlite_db_names import ERROR_REPORTS @@ -36,3 +39,46 @@ def allow_migrate(self, db, app_label, model_name=None, **hints): return False return None + + +class ErrorReports(models.Model): + FRONTEND = "frontend" + BACKEND = "backend" + POSSIBLE_ERRORS = [ + (FRONTEND, "Frontend"), + (BACKEND, "Backend"), + ] + + error_from = models.CharField(max_length=10, choices=POSSIBLE_ERRORS) + error_message = models.CharField(max_length=255) + traceback = models.TextField() + first_occurred = models.DateTimeField(default=timezone.now) + last_occurred = models.DateTimeField(default=timezone.now) + sent = models.BooleanField(default=False) + no_of_errors = models.IntegerField(default=1) + + 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): + error, created = cls.objects.get_or_create( + error_from=error_from, error_message=error_message, traceback=traceback + ) + if not created: + error.no_of_errors += 1 + error.last_occurred = timezone.now() + error.save() + return error + + @classmethod + def get_unsent_errors(cls): + return cls.objects.filter(sent=False) + + @classmethod + def delete_error(cls): + ... From 1b50c66ac45bde15e2b53755b5b54237eb984bad Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Fri, 7 Jun 2024 15:44:37 +0530 Subject: [PATCH 08/74] add tests for model methods --- kolibri/core/errorreports/test/__init__.py | 0 kolibri/core/errorreports/test/test_models.py | 82 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 kolibri/core/errorreports/test/__init__.py create mode 100644 kolibri/core/errorreports/test/test_models.py diff --git a/kolibri/core/errorreports/test/__init__.py b/kolibri/core/errorreports/test/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/kolibri/core/errorreports/test/test_models.py b/kolibri/core/errorreports/test/test_models.py new file mode 100644 index 00000000000..3e5f0812d5c --- /dev/null +++ b/kolibri/core/errorreports/test/test_models.py @@ -0,0 +1,82 @@ +from django.test import TestCase +from django.utils import timezone + +from kolibri.core.errorreports.models import ErrorReports + + +class ErrorReportsTestCase(TestCase): + databases = "__all__" # I am not sure about this, maybe a overkill but works + + def test_insert_or_update_error(self): + error_from = ErrorReports.FRONTEND + error_message = "Test Error" + traceback = "Test Traceback" + + error = ErrorReports.insert_or_update_error( + error_from, error_message, traceback + ) + self.assertEqual(error.error_from, error_from) + self.assertEqual(error.error_message, error_message) + self.assertEqual(error.traceback, traceback) + self.assertEqual(error.no_of_errors, 1) + self.assertFalse(error.sent) + self.assertLess( + timezone.now() - error.first_occurred, timezone.timedelta(seconds=1) + ) + self.assertLess( + timezone.now() - error.last_occurred, timezone.timedelta(seconds=1) + ) + + # creating the error again, so this time it should update the error + error = ErrorReports.insert_or_update_error( + error_from, error_message, traceback + ) + self.assertEqual(error.error_from, error_from) + self.assertEqual(error.error_message, error_message) + self.assertEqual(error.traceback, traceback) + self.assertEqual(error.no_of_errors, 2) + self.assertFalse(error.sent) + self.assertLess( + timezone.now() - error.first_occurred, timezone.timedelta(seconds=1) + ) + self.assertLess( + timezone.now() - error.last_occurred, timezone.timedelta(seconds=1) + ) + + def test_get_unsent_errors(self): + ErrorReports.objects.create( + error_from=ErrorReports.FRONTEND, + error_message="Error 1", + traceback="Traceback 1", + sent=False, + ) + ErrorReports.objects.create( + error_from=ErrorReports.BACKEND, + error_message="Error 2", + traceback="Traceback 2", + sent=False, + ) + ErrorReports.objects.create( + error_from=ErrorReports.BACKEND, + error_message="Error 3", + traceback="Traceback 3", + sent=True, + ) + + # Get unsent errors, shall be only 2 as out of 3, 1 is sent + unsent_errors = ErrorReports.get_unsent_errors() + 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=ErrorReports.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 f89c6019cea435ab0d4c8a5242667d3449478510 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 10 Jun 2024 20:44:35 +0530 Subject: [PATCH 09/74] add DEVELOPER_MODE = False on settings --- kolibri/deployment/default/settings/base.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kolibri/deployment/default/settings/base.py b/kolibri/deployment/default/settings/base.py index 3c0f24fea2b..48d392dc1dd 100644 --- a/kolibri/deployment/default/settings/base.py +++ b/kolibri/deployment/default/settings/base.py @@ -58,6 +58,9 @@ # SECURITY WARNING: don't run with debug turned on in production! DEBUG = conf.OPTIONS["Server"]["DEBUG"] +# Developer mode should be False instead of None in production mode +DEVELOPER_MODE = False + ALLOWED_HOSTS = ["*"] # Application definition From e49b82d85d18ef32cdf12ad7730cd8ae9261e8bd Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 10 Jun 2024 20:46:44 +0530 Subject: [PATCH 10/74] conditional check of dev mode during writing into database --- kolibri/core/errorreports/models.py | 26 +++++++++++++------ kolibri/core/errorreports/test/test_models.py | 15 ++++++++++- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index c23f466b17b..2439cc9eb94 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -1,9 +1,15 @@ +import logging + +from django.conf import settings from django.db import models from django.utils import timezone from kolibri.deployment.default.sqlite_db_names import ERROR_REPORTS +logger = logging.getLogger(__name__) + + class ErrorReportsRouter(object): """ Determine how to route database calls for the ErrorReports app. @@ -66,14 +72,18 @@ def mark_as_sent(self): @classmethod def insert_or_update_error(cls, error_from, error_message, traceback): - error, created = cls.objects.get_or_create( - error_from=error_from, error_message=error_message, traceback=traceback - ) - if not created: - error.no_of_errors += 1 - error.last_occurred = timezone.now() - error.save() - return error + if not settings.DEVELOPER_MODE: + error, created = cls.objects.get_or_create( + error_from=error_from, error_message=error_message, traceback=traceback + ) + if not created: + error.no_of_errors += 1 + error.last_occurred = timezone.now() + error.save() + logger.error("ErrorReports: Database updated.") + return error + logger.error("ErrorReports: Database not updated, as DEVELOPER_MODE is True.") + return None @classmethod def get_unsent_errors(cls): diff --git a/kolibri/core/errorreports/test/test_models.py b/kolibri/core/errorreports/test/test_models.py index 3e5f0812d5c..a3a68a2db70 100644 --- a/kolibri/core/errorreports/test/test_models.py +++ b/kolibri/core/errorreports/test/test_models.py @@ -1,3 +1,4 @@ +from django.test import override_settings from django.test import TestCase from django.utils import timezone @@ -7,7 +8,8 @@ class ErrorReportsTestCase(TestCase): databases = "__all__" # I am not sure about this, maybe a overkill but works - def test_insert_or_update_error(self): + @override_settings(DEVELOPER_MODE=False) + def test_insert_or_update_error_prod_mode(self): error_from = ErrorReports.FRONTEND error_message = "Test Error" traceback = "Test Traceback" @@ -43,6 +45,17 @@ def test_insert_or_update_error(self): timezone.now() - error.last_occurred, timezone.timedelta(seconds=1) ) + @override_settings(DEVELOPER_MODE=True) + def test_insert_or_update_error_dev_mode(self): + error_from = ErrorReports.FRONTEND + error_message = "Test Error" + traceback = "Test Traceback" + + error = ErrorReports.insert_or_update_error( + error_from, error_message, traceback + ) + self.assertIsNone(error) + def test_get_unsent_errors(self): ErrorReports.objects.create( error_from=ErrorReports.FRONTEND, From 2114c4dd0e2f7b9e4db46494e95affb6113c820c Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 11 Jun 2024 17:35:07 +0530 Subject: [PATCH 11/74] pass>>>`...` --- kolibri/core/errorreports/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index 2439cc9eb94..6b543aa3b7b 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -91,4 +91,4 @@ def get_unsent_errors(cls): @classmethod def delete_error(cls): - ... + pass From 678d0cdf2692f4a37a2bbac2b5976e589c349c6a Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 11 Jun 2024 17:58:44 +0530 Subject: [PATCH 12/74] use getattr for accessing settings.DEVELOPER_MODE --- kolibri/core/errorreports/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index 6b543aa3b7b..eac3f76a1cc 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -72,7 +72,7 @@ def mark_as_sent(self): @classmethod def insert_or_update_error(cls, error_from, error_message, traceback): - if not settings.DEVELOPER_MODE: + if not getattr(settings, "DEVELOPER_MODE", None): error, created = cls.objects.get_or_create( error_from=error_from, error_message=error_message, traceback=traceback ) From 2e1b852ca47313560bd6ef9c475e669d4c108f12 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Sun, 9 Jun 2024 18:20:08 +0530 Subject: [PATCH 13/74] Add middleware for handling runtime errors --- kolibri/core/errorreports/middleware.py | 43 +++++++++++++++++++++ kolibri/deployment/default/settings/base.py | 1 + 2 files changed, 44 insertions(+) create mode 100644 kolibri/core/errorreports/middleware.py diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py new file mode 100644 index 00000000000..dcf26cbe563 --- /dev/null +++ b/kolibri/core/errorreports/middleware.py @@ -0,0 +1,43 @@ +import logging +import traceback + +from django.conf import settings +from django.db import IntegrityError + +from .models import ErrorReports + + +class ErrorReportingMiddleware: + """ + Middleware to log exceptions to the database. + ref: https://docs.djangoproject.com/en/5.0/topics/http/middleware/#writing-your-own-middleware + """ + + def __init__(self, get_response): + self.get_response = get_response + self.logger = logging.getLogger(__name__) + + def __call__(self, request): + response = self.get_response(request) + return response + + def process_exception(self, request, exception): + error_message = str(exception) + traceback_info = traceback.format_exc() + self.logger.error("Unexpected Error %s", error_message) + # do not write to the database if dev's mode, development is a market for errors + if not settings.DEVELOPER_MODE: + try: + self.logger.error("Saving error report to the database.") + ErrorReports.insert_or_update_error( + "Backend", error_message, traceback_info + ) + self.logger.info("Error report saved to the database.") + except IntegrityError: + self.logger.error( + "Error occurred while saving error report to the database." + ) + else: + self.logger.error( + "Developer mode is enabled. Error report will not be saved to the database." + ) diff --git a/kolibri/deployment/default/settings/base.py b/kolibri/deployment/default/settings/base.py index 48d392dc1dd..a92e2e3bef5 100644 --- a/kolibri/deployment/default/settings/base.py +++ b/kolibri/deployment/default/settings/base.py @@ -104,6 +104,7 @@ "django.middleware.common.CommonMiddleware", "django.middleware.csrf.CsrfViewMiddleware", "kolibri.core.auth.middleware.CustomAuthenticationMiddleware", + "kolibri.core.errorreports.middleware.ErrorReportingMiddleware", "django.contrib.messages.middleware.MessageMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware", "django.middleware.security.SecurityMiddleware", From 722efe7700beb2942acfe0f6338c9c65c94fc869 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Sun, 9 Jun 2024 18:21:15 +0530 Subject: [PATCH 14/74] Add test for error-report middleware --- .../core/errorreports/test/test_middleware.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 kolibri/core/errorreports/test/test_middleware.py diff --git a/kolibri/core/errorreports/test/test_middleware.py b/kolibri/core/errorreports/test/test_middleware.py new file mode 100644 index 00000000000..a8e96a4ca4c --- /dev/null +++ b/kolibri/core/errorreports/test/test_middleware.py @@ -0,0 +1,29 @@ +from django.test import override_settings +from django.test import TestCase + +from ..middleware import ErrorReportingMiddleware +from ..models import ErrorReports + + +class ErrorReportingMiddlewareTestCase(TestCase): + databases = "__all__" + + @override_settings(DEVELOPER_MODE=False) + def test_process_exception(self): + middleware = ErrorReportingMiddleware(lambda r: None) + request = self.client.request() + exception = Exception("Test Exception") + middleware.process_exception(request, exception=exception) + self.assertTrue( + ErrorReports.objects.filter(error_message="Test Exception").exists() + ) + + @override_settings(DEVELOPER_MODE=True) + def test_process_exception_developer_mode_enabled(self): + middleware = ErrorReportingMiddleware(lambda r: None) + request = self.client.request() + exception = Exception("Test Exception") + middleware.process_exception(request, exception=exception) + self.assertFalse( + ErrorReports.objects.filter(error_message="Test Exception").exists() + ) From 4c62c272960a6e9dac7cfb94a3f93ee1f2997029 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 10 Jun 2024 21:02:54 +0530 Subject: [PATCH 15/74] Simplify calling insert_or_update_error and tests --- kolibri/core/errorreports/middleware.py | 26 +++++++------------ .../core/errorreports/test/test_middleware.py | 23 ++++------------ 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index dcf26cbe563..a061bbd003f 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -1,7 +1,6 @@ import logging import traceback -from django.conf import settings from django.db import IntegrityError from .models import ErrorReports @@ -24,20 +23,15 @@ def __call__(self, request): def process_exception(self, request, exception): error_message = str(exception) traceback_info = traceback.format_exc() - self.logger.error("Unexpected Error %s", error_message) - # do not write to the database if dev's mode, development is a market for errors - if not settings.DEVELOPER_MODE: - try: - self.logger.error("Saving error report to the database.") - ErrorReports.insert_or_update_error( - "Backend", error_message, traceback_info - ) - self.logger.info("Error report saved to the database.") - except IntegrityError: - self.logger.error( - "Error occurred while saving error report to the database." - ) - else: + self.logger.error("Unexpected Error: %s", error_message) + + try: + self.logger.error("Saving error report to the database.") + ErrorReports.insert_or_update_error( + "Backend", error_message, traceback_info + ) + self.logger.info("Error report saved to the database.") + except IntegrityError: self.logger.error( - "Developer mode is enabled. Error report will not be saved to the database." + "Error occurred while saving error report to the database." ) diff --git a/kolibri/core/errorreports/test/test_middleware.py b/kolibri/core/errorreports/test/test_middleware.py index a8e96a4ca4c..fc58fde359d 100644 --- a/kolibri/core/errorreports/test/test_middleware.py +++ b/kolibri/core/errorreports/test/test_middleware.py @@ -1,4 +1,5 @@ -from django.test import override_settings +from unittest.mock import patch + from django.test import TestCase from ..middleware import ErrorReportingMiddleware @@ -6,24 +7,10 @@ class ErrorReportingMiddlewareTestCase(TestCase): - databases = "__all__" - - @override_settings(DEVELOPER_MODE=False) - def test_process_exception(self): - middleware = ErrorReportingMiddleware(lambda r: None) - request = self.client.request() - exception = Exception("Test Exception") - middleware.process_exception(request, exception=exception) - self.assertTrue( - ErrorReports.objects.filter(error_message="Test Exception").exists() - ) - - @override_settings(DEVELOPER_MODE=True) - def test_process_exception_developer_mode_enabled(self): + @patch.object(ErrorReports, "insert_or_update_error") + def test_process_exception(self, mock_insert_or_update_error): middleware = ErrorReportingMiddleware(lambda r: None) request = self.client.request() exception = Exception("Test Exception") middleware.process_exception(request, exception=exception) - self.assertFalse( - ErrorReports.objects.filter(error_message="Test Exception").exists() - ) + mock_insert_or_update_error.assert_called_once() From a859358666342b845b13281243d74309ae9bc4cb Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 11 Jun 2024 22:56:30 +0530 Subject: [PATCH 16/74] put all the constants together in errorreports --- kolibri/core/errorreports/constants.py | 2 ++ kolibri/core/errorreports/middleware.py | 6 ++---- kolibri/core/errorreports/models.py | 6 ++---- kolibri/core/errorreports/test/test_models.py | 14 ++++++++------ 4 files changed, 14 insertions(+), 14 deletions(-) create mode 100644 kolibri/core/errorreports/constants.py diff --git a/kolibri/core/errorreports/constants.py b/kolibri/core/errorreports/constants.py new file mode 100644 index 00000000000..a5a987d8a8d --- /dev/null +++ b/kolibri/core/errorreports/constants.py @@ -0,0 +1,2 @@ +FRONTEND = "frontend" +BACKEND = "backend" diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index a061bbd003f..e5ba76a5432 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -3,13 +3,13 @@ from django.db import IntegrityError +from .constants import BACKEND from .models import ErrorReports class ErrorReportingMiddleware: """ Middleware to log exceptions to the database. - ref: https://docs.djangoproject.com/en/5.0/topics/http/middleware/#writing-your-own-middleware """ def __init__(self, get_response): @@ -27,9 +27,7 @@ def process_exception(self, request, exception): try: self.logger.error("Saving error report to the database.") - ErrorReports.insert_or_update_error( - "Backend", error_message, traceback_info - ) + ErrorReports.insert_or_update_error(BACKEND, error_message, traceback_info) self.logger.info("Error report saved to the database.") except IntegrityError: self.logger.error( diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index eac3f76a1cc..564e08ede1b 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -4,6 +4,8 @@ from django.db import models from django.utils import timezone +from .constants import BACKEND +from .constants import FRONTEND from kolibri.deployment.default.sqlite_db_names import ERROR_REPORTS @@ -13,8 +15,6 @@ class ErrorReportsRouter(object): """ Determine how to route database calls for the ErrorReports app. - ref: https://docs.djangoproject.com/en/5.0/topics/db/multi-db/ - """ def db_for_read(self, model, **hints): @@ -48,8 +48,6 @@ def allow_migrate(self, db, app_label, model_name=None, **hints): class ErrorReports(models.Model): - FRONTEND = "frontend" - BACKEND = "backend" POSSIBLE_ERRORS = [ (FRONTEND, "Frontend"), (BACKEND, "Backend"), diff --git a/kolibri/core/errorreports/test/test_models.py b/kolibri/core/errorreports/test/test_models.py index a3a68a2db70..c39b4f8efb3 100644 --- a/kolibri/core/errorreports/test/test_models.py +++ b/kolibri/core/errorreports/test/test_models.py @@ -2,6 +2,8 @@ from django.test import TestCase from django.utils import timezone +from ..constants import BACKEND +from ..constants import FRONTEND from kolibri.core.errorreports.models import ErrorReports @@ -10,7 +12,7 @@ class ErrorReportsTestCase(TestCase): @override_settings(DEVELOPER_MODE=False) def test_insert_or_update_error_prod_mode(self): - error_from = ErrorReports.FRONTEND + error_from = FRONTEND error_message = "Test Error" traceback = "Test Traceback" @@ -47,7 +49,7 @@ def test_insert_or_update_error_prod_mode(self): @override_settings(DEVELOPER_MODE=True) def test_insert_or_update_error_dev_mode(self): - error_from = ErrorReports.FRONTEND + error_from = FRONTEND error_message = "Test Error" traceback = "Test Traceback" @@ -58,19 +60,19 @@ def test_insert_or_update_error_dev_mode(self): def test_get_unsent_errors(self): ErrorReports.objects.create( - error_from=ErrorReports.FRONTEND, + error_from=FRONTEND, error_message="Error 1", traceback="Traceback 1", sent=False, ) ErrorReports.objects.create( - error_from=ErrorReports.BACKEND, + error_from=BACKEND, error_message="Error 2", traceback="Traceback 2", sent=False, ) ErrorReports.objects.create( - error_from=ErrorReports.BACKEND, + error_from=BACKEND, error_message="Error 3", traceback="Traceback 3", sent=True, @@ -84,7 +86,7 @@ def test_get_unsent_errors(self): def test_mark_as_sent(self): error = ErrorReports.objects.create( - error_from=ErrorReports.FRONTEND, + error_from=FRONTEND, error_message="Test Error", traceback="Test Traceback", sent=False, From fa40e38f0a782fb58686756e66eb5eab90ccc144 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 11 Jun 2024 23:53:17 +0530 Subject: [PATCH 17/74] move POSSIBLE_ERRORS to contants.py --- kolibri/core/errorreports/constants.py | 5 +++++ kolibri/core/errorreports/models.py | 8 +------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/kolibri/core/errorreports/constants.py b/kolibri/core/errorreports/constants.py index a5a987d8a8d..b32b9331260 100644 --- a/kolibri/core/errorreports/constants.py +++ b/kolibri/core/errorreports/constants.py @@ -1,2 +1,7 @@ FRONTEND = "frontend" BACKEND = "backend" + +POSSIBLE_ERRORS = [ + (FRONTEND, "Frontend"), + (BACKEND, "Backend"), +] diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index 564e08ede1b..49ced8beaf6 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -4,8 +4,7 @@ from django.db import models from django.utils import timezone -from .constants import BACKEND -from .constants import FRONTEND +from .constants import POSSIBLE_ERRORS from kolibri.deployment.default.sqlite_db_names import ERROR_REPORTS @@ -48,11 +47,6 @@ def allow_migrate(self, db, app_label, model_name=None, **hints): class ErrorReports(models.Model): - POSSIBLE_ERRORS = [ - (FRONTEND, "Frontend"), - (BACKEND, "Backend"), - ] - error_from = models.CharField(max_length=10, choices=POSSIBLE_ERRORS) error_message = models.CharField(max_length=255) traceback = models.TextField() From b1c50bfe88aba226ccfdbcba3b511666a0976337 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Wed, 12 Jun 2024 15:07:53 +0530 Subject: [PATCH 18/74] improve testcase for middleware --- kolibri/core/errorreports/middleware.py | 4 +- .../core/errorreports/test/test_middleware.py | 38 +++++++++++++++++-- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index e5ba76a5432..0694fa9c167 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -29,7 +29,7 @@ def process_exception(self, request, exception): self.logger.error("Saving error report to the database.") ErrorReports.insert_or_update_error(BACKEND, error_message, traceback_info) self.logger.info("Error report saved to the database.") - except IntegrityError: + except IntegrityError as e: self.logger.error( - "Error occurred while saving error report to the database." + "Error occurred while saving error report to the database: %s", str(e) ) diff --git a/kolibri/core/errorreports/test/test_middleware.py b/kolibri/core/errorreports/test/test_middleware.py index fc58fde359d..019e8818110 100644 --- a/kolibri/core/errorreports/test/test_middleware.py +++ b/kolibri/core/errorreports/test/test_middleware.py @@ -1,16 +1,48 @@ +import logging +import traceback from unittest.mock import patch +from django.db import IntegrityError from django.test import TestCase +from ..constants import BACKEND from ..middleware import ErrorReportingMiddleware from ..models import ErrorReports class ErrorReportingMiddlewareTestCase(TestCase): @patch.object(ErrorReports, "insert_or_update_error") - def test_process_exception(self, mock_insert_or_update_error): + @patch.object(logging.Logger, "error") + def test_process_exception(self, mock_logger_error, mock_insert_or_update_error): middleware = ErrorReportingMiddleware(lambda r: None) request = self.client.request() exception = Exception("Test Exception") - middleware.process_exception(request, exception=exception) - mock_insert_or_update_error.assert_called_once() + try: + raise exception + except Exception as e: + middleware.process_exception(request, exception=e) + # I am just coverting exception.__traceback__ to string + expected_traceback_info = "".join( + traceback.format_exception( + type(exception), exception, exception.__traceback__ + ) + ) + mock_insert_or_update_error.assert_called_once_with( + BACKEND, str(exception), expected_traceback_info + ) + + @patch.object(ErrorReports, "insert_or_update_error") + @patch.object(logging.Logger, "error") + def test_process_exception_integrity_error( + self, mock_logger_error, mock_insert_or_update_error + ): + middleware = ErrorReportingMiddleware(lambda r: None) + request = self.client.request() + exception = Exception("Test Exception") + mock_insert_or_update_error.side_effect = IntegrityError("Some Integrity Error") + middleware.process_exception(request, exception) + + mock_logger_error.assert_any_call( + "Error occurred while saving error report to the database: %s", + str(mock_insert_or_update_error.side_effect), + ) From 388dd17d0a290ef08d112cb3b487e6f0c91128ad Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 10 Jun 2024 12:54:07 +0530 Subject: [PATCH 19/74] add serializer ErrorReprotsSerializers:frontend data validation --- kolibri/core/errorreports/serializers.py | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 kolibri/core/errorreports/serializers.py diff --git a/kolibri/core/errorreports/serializers.py b/kolibri/core/errorreports/serializers.py new file mode 100644 index 00000000000..0a9d320bf32 --- /dev/null +++ b/kolibri/core/errorreports/serializers.py @@ -0,0 +1,9 @@ +from rest_framework import serializers + +from .models import ErrorReports + + +class ErrorReportSerializer(serializers.Serializer): + error_from = serializers.ChoiceField(choices=ErrorReports.POSSIBLE_ERRORS) + error_message = serializers.CharField(max_length=255) + traceback = serializers.CharField() From 26f0018fb53f575852ee6dea146c13a6cdc83ce2 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 10 Jun 2024 12:55:15 +0530 Subject: [PATCH 20/74] add API for frontend error report --- kolibri/core/api_urls.py | 1 + kolibri/core/errorreports/api.py | 51 +++++++++++++++++++++++++++ kolibri/core/errorreports/api_urls.py | 5 +++ 3 files changed, 57 insertions(+) create mode 100644 kolibri/core/errorreports/api.py create mode 100644 kolibri/core/errorreports/api_urls.py diff --git a/kolibri/core/api_urls.py b/kolibri/core/api_urls.py index 7681b05498f..5e112955cfc 100644 --- a/kolibri/core/api_urls.py +++ b/kolibri/core/api_urls.py @@ -13,4 +13,5 @@ re_path(r"^discovery/", include("kolibri.core.discovery.api_urls")), re_path(r"^notifications/", include("kolibri.core.analytics.api_urls")), re_path(r"^public/", include("kolibri.core.public.api_urls")), + re_path(r"^errorreports/", include("kolibri.core.errorreports.api_urls")), ] diff --git a/kolibri/core/errorreports/api.py b/kolibri/core/errorreports/api.py new file mode 100644 index 00000000000..a791cf82957 --- /dev/null +++ b/kolibri/core/errorreports/api.py @@ -0,0 +1,51 @@ +import logging + +from django.conf import settings +from rest_framework import status +from rest_framework.decorators import api_view +from rest_framework.response import Response + +from .models import ErrorReports +from .serializers import ErrorReportSerializer + +logger = logging.getLogger(__name__) + + +@api_view(["POST"]) +def frontendreport(request): + """ + test with: + curl -X POST http://localhost:8000/api/errorreports/frontendreport/ -H "Content-Type: application/json" -d '{ + "error_from": "frontend", + "error_message": "An example error occurred", + "traceback": "Traceback (most recent call last): ..." + }' + """ + serializer = ErrorReportSerializer(data=request.data) + if serializer.is_valid(): + data = serializer.validated_data + if not settings.DEVELOPER_MODE: + logger.error("Saving error report in the database.") + error = ErrorReports.insert_or_update_error( + error_from=data["error_from"], + error_message=data["error_message"], + traceback=data["traceback"], + ) + logger.error( + "Error report saved successfully. Error ID: {}".format(error.id) + ) + return Response( + {"message": "Error report saved successfully.", "error_id": error.id}, + status=status.HTTP_201_CREATED, + ) + logger.error( + "Developer mode is enabled. Error report is not saved in the database." + ) + return Response( + { + "message": "Error report not saved in the database. As we are in developer mode" + }, + status=status.HTTP_200_OK, + ) + else: + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) diff --git a/kolibri/core/errorreports/api_urls.py b/kolibri/core/errorreports/api_urls.py new file mode 100644 index 00000000000..63252102380 --- /dev/null +++ b/kolibri/core/errorreports/api_urls.py @@ -0,0 +1,5 @@ +from django.urls import re_path + +from .api import frontendreport + +urlpatterns = [re_path(r"^frontendreport", frontendreport, name="frontendreport")] From b6de2847f0752b4ea50911c7340237811c087df6 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 10 Jun 2024 12:56:17 +0530 Subject: [PATCH 21/74] testcase for frontendreport view --- kolibri/core/errorreports/test/test_api.py | 53 ++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 kolibri/core/errorreports/test/test_api.py diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py new file mode 100644 index 00000000000..4aacec6f018 --- /dev/null +++ b/kolibri/core/errorreports/test/test_api.py @@ -0,0 +1,53 @@ +from django.test import override_settings +from django.test import TestCase +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient + +from ..models import ErrorReports + + +class FrontendReportTestCase(TestCase): + databases = "__all__" + + def setUp(self): + self.client = APIClient() + + @override_settings(DEVELOPER_MODE=False) + def test_frontend_report_prod_mode(self): + url = reverse("kolibri:core:frontendreport") + data = { + "error_from": ErrorReports.FRONTEND, + "error_message": "Something went wrong", + "traceback": "Traceback information", + } + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(ErrorReports.objects.count(), 1) + error = ErrorReports.objects.first() + self.assertEqual(error.error_from, ErrorReports.FRONTEND) + self.assertEqual(error.error_message, "Something went wrong") + self.assertEqual(error.traceback, "Traceback information") + + @override_settings(DEVELOPER_MODE=True) + def test_frontend_report_dev_mode(self): + url = reverse("kolibri:core:frontendreport") + data = { + "error_from": ErrorReports.FRONTEND, + "error_message": "Something went wrong", + "traceback": "Traceback information", + } + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(ErrorReports.objects.count(), 0) + + def test_frontend_report_invalid_data(self): + url = reverse("kolibri:core:frontendreport") + data = { + "error_from": ErrorReports.FRONTEND, + "error_message": "", + "traceback": "Traceback information", + } + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(ErrorReports.objects.count(), 0) From c67851b222c03ce7519e67e2a8d6a6f846945ad7 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 10 Jun 2024 14:02:21 +0530 Subject: [PATCH 22/74] make error_from default to 'frontend' --- kolibri/core/errorreports/api.py | 3 +-- kolibri/core/errorreports/serializers.py | 3 --- kolibri/core/errorreports/test/test_api.py | 3 --- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/kolibri/core/errorreports/api.py b/kolibri/core/errorreports/api.py index a791cf82957..aa77b2c12e7 100644 --- a/kolibri/core/errorreports/api.py +++ b/kolibri/core/errorreports/api.py @@ -16,7 +16,6 @@ def frontendreport(request): """ test with: curl -X POST http://localhost:8000/api/errorreports/frontendreport/ -H "Content-Type: application/json" -d '{ - "error_from": "frontend", "error_message": "An example error occurred", "traceback": "Traceback (most recent call last): ..." }' @@ -27,7 +26,7 @@ def frontendreport(request): if not settings.DEVELOPER_MODE: logger.error("Saving error report in the database.") error = ErrorReports.insert_or_update_error( - error_from=data["error_from"], + error_from=ErrorReports.FRONTEND, error_message=data["error_message"], traceback=data["traceback"], ) diff --git a/kolibri/core/errorreports/serializers.py b/kolibri/core/errorreports/serializers.py index 0a9d320bf32..7e67d84cdb3 100644 --- a/kolibri/core/errorreports/serializers.py +++ b/kolibri/core/errorreports/serializers.py @@ -1,9 +1,6 @@ from rest_framework import serializers -from .models import ErrorReports - class ErrorReportSerializer(serializers.Serializer): - error_from = serializers.ChoiceField(choices=ErrorReports.POSSIBLE_ERRORS) error_message = serializers.CharField(max_length=255) traceback = serializers.CharField() diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py index 4aacec6f018..b9adf5fb487 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/core/errorreports/test/test_api.py @@ -17,7 +17,6 @@ def setUp(self): def test_frontend_report_prod_mode(self): url = reverse("kolibri:core:frontendreport") data = { - "error_from": ErrorReports.FRONTEND, "error_message": "Something went wrong", "traceback": "Traceback information", } @@ -33,7 +32,6 @@ def test_frontend_report_prod_mode(self): def test_frontend_report_dev_mode(self): url = reverse("kolibri:core:frontendreport") data = { - "error_from": ErrorReports.FRONTEND, "error_message": "Something went wrong", "traceback": "Traceback information", } @@ -44,7 +42,6 @@ def test_frontend_report_dev_mode(self): def test_frontend_report_invalid_data(self): url = reverse("kolibri:core:frontendreport") data = { - "error_from": ErrorReports.FRONTEND, "error_message": "", "traceback": "Traceback information", } From cc968b6c4ebe80fb8963216f1cb13e9d129f1f6a Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 10 Jun 2024 21:46:27 +0530 Subject: [PATCH 23/74] simplify API: remove conditioning before calling insert_or_update_error() --- kolibri/core/errorreports/api.py | 31 ++++++--------------- kolibri/core/errorreports/api_urls.py | 4 +-- kolibri/core/errorreports/test/test_api.py | 32 ++++------------------ 3 files changed, 16 insertions(+), 51 deletions(-) diff --git a/kolibri/core/errorreports/api.py b/kolibri/core/errorreports/api.py index aa77b2c12e7..442e3d0e81b 100644 --- a/kolibri/core/errorreports/api.py +++ b/kolibri/core/errorreports/api.py @@ -1,6 +1,5 @@ import logging -from django.conf import settings from rest_framework import status from rest_framework.decorators import api_view from rest_framework.response import Response @@ -12,10 +11,10 @@ @api_view(["POST"]) -def frontendreport(request): +def report(request): """ test with: - curl -X POST http://localhost:8000/api/errorreports/frontendreport/ -H "Content-Type: application/json" -d '{ + curl -X POST http://localhost:8000/api/errorreports/report/ -H "Content-Type: application/json" -d '{ "error_message": "An example error occurred", "traceback": "Traceback (most recent call last): ..." }' @@ -23,28 +22,14 @@ def frontendreport(request): serializer = ErrorReportSerializer(data=request.data) if serializer.is_valid(): data = serializer.validated_data - if not settings.DEVELOPER_MODE: - logger.error("Saving error report in the database.") - error = ErrorReports.insert_or_update_error( - error_from=ErrorReports.FRONTEND, - error_message=data["error_message"], - traceback=data["traceback"], - ) - logger.error( - "Error report saved successfully. Error ID: {}".format(error.id) - ) - return Response( - {"message": "Error report saved successfully.", "error_id": error.id}, - status=status.HTTP_201_CREATED, - ) - logger.error( - "Developer mode is enabled. Error report is not saved in the database." + error = ErrorReports.insert_or_update_error( + error_from=ErrorReports.FRONTEND, + error_message=data["error_message"], + traceback=data["traceback"], ) + return Response( - { - "message": "Error report not saved in the database. As we are in developer mode" - }, - status=status.HTTP_200_OK, + {"error_id": error.id if error else None}, status=status.HTTP_200_OK ) else: return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) diff --git a/kolibri/core/errorreports/api_urls.py b/kolibri/core/errorreports/api_urls.py index 63252102380..02be1b333d9 100644 --- a/kolibri/core/errorreports/api_urls.py +++ b/kolibri/core/errorreports/api_urls.py @@ -1,5 +1,5 @@ from django.urls import re_path -from .api import frontendreport +from .api import report -urlpatterns = [re_path(r"^frontendreport", frontendreport, name="frontendreport")] +urlpatterns = [re_path(r"^report", report, name="report")] diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py index b9adf5fb487..a72495a7bc6 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/core/errorreports/test/test_api.py @@ -1,11 +1,9 @@ -from django.test import override_settings from django.test import TestCase from django.urls import reverse -from rest_framework import status +from rest_framework.status import HTTP_200_OK +from rest_framework.status import HTTP_400_BAD_REQUEST from rest_framework.test import APIClient -from ..models import ErrorReports - class FrontendReportTestCase(TestCase): databases = "__all__" @@ -13,38 +11,20 @@ class FrontendReportTestCase(TestCase): def setUp(self): self.client = APIClient() - @override_settings(DEVELOPER_MODE=False) def test_frontend_report_prod_mode(self): - url = reverse("kolibri:core:frontendreport") - data = { - "error_message": "Something went wrong", - "traceback": "Traceback information", - } - response = self.client.post(url, data, format="json") - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(ErrorReports.objects.count(), 1) - error = ErrorReports.objects.first() - self.assertEqual(error.error_from, ErrorReports.FRONTEND) - self.assertEqual(error.error_message, "Something went wrong") - self.assertEqual(error.traceback, "Traceback information") - - @override_settings(DEVELOPER_MODE=True) - def test_frontend_report_dev_mode(self): - url = reverse("kolibri:core:frontendreport") + url = reverse("kolibri:core:report") data = { "error_message": "Something went wrong", "traceback": "Traceback information", } response = self.client.post(url, data, format="json") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(ErrorReports.objects.count(), 0) + self.assertEqual(response.status_code, HTTP_200_OK) def test_frontend_report_invalid_data(self): - url = reverse("kolibri:core:frontendreport") + url = reverse("kolibri:core:report") data = { "error_message": "", "traceback": "Traceback information", } response = self.client.post(url, data, format="json") - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(ErrorReports.objects.count(), 0) + self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST) From 09e2af3abe0716b97bdfddc1e010be020bd42add Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Wed, 12 Jun 2024 15:18:35 +0530 Subject: [PATCH 24/74] name changes --- kolibri/core/errorreports/api.py | 3 ++- kolibri/core/errorreports/test/test_api.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/kolibri/core/errorreports/api.py b/kolibri/core/errorreports/api.py index 442e3d0e81b..ae677c19bfa 100644 --- a/kolibri/core/errorreports/api.py +++ b/kolibri/core/errorreports/api.py @@ -4,6 +4,7 @@ from rest_framework.decorators import api_view from rest_framework.response import Response +from .constants import FRONTEND from .models import ErrorReports from .serializers import ErrorReportSerializer @@ -23,7 +24,7 @@ def report(request): if serializer.is_valid(): data = serializer.validated_data error = ErrorReports.insert_or_update_error( - error_from=ErrorReports.FRONTEND, + error_from=FRONTEND, error_message=data["error_message"], traceback=data["traceback"], ) diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py index a72495a7bc6..af14d5ff894 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/core/errorreports/test/test_api.py @@ -11,7 +11,7 @@ class FrontendReportTestCase(TestCase): def setUp(self): self.client = APIClient() - def test_frontend_report_prod_mode(self): + def test_frontend_report(self): url = reverse("kolibri:core:report") data = { "error_message": "Something went wrong", From d103ff8e6866bc072b5173d298e504fa08d39ca8 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Wed, 12 Jun 2024 20:37:02 +0530 Subject: [PATCH 25/74] expect (AttributeError, Exception) while calling insert_or_update --- kolibri/core/errorreports/api.py | 24 ++++++++++------ kolibri/core/errorreports/test/test_api.py | 33 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/kolibri/core/errorreports/api.py b/kolibri/core/errorreports/api.py index ae677c19bfa..60e52e1f2f8 100644 --- a/kolibri/core/errorreports/api.py +++ b/kolibri/core/errorreports/api.py @@ -23,14 +23,20 @@ def report(request): serializer = ErrorReportSerializer(data=request.data) if serializer.is_valid(): data = serializer.validated_data - error = ErrorReports.insert_or_update_error( - error_from=FRONTEND, - error_message=data["error_message"], - traceback=data["traceback"], - ) - - return Response( - {"error_id": error.id if error else None}, status=status.HTTP_200_OK - ) + try: + error = ErrorReports.insert_or_update_error( + error_from=FRONTEND, + error_message=data["error_message"], + traceback=data["traceback"], + ) + return Response( + {"error_id": error.id if error else None}, status=status.HTTP_200_OK + ) + except (AttributeError, Exception) as e: + logger.error("Error while saving error report: {}".format(e)) + return Response( + {"error": "Error while saving error report"}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) else: return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py index af14d5ff894..ddfdcf60c4c 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/core/errorreports/test/test_api.py @@ -1,7 +1,10 @@ +from unittest.mock import patch + from django.test import TestCase from django.urls import reverse from rest_framework.status import HTTP_200_OK from rest_framework.status import HTTP_400_BAD_REQUEST +from rest_framework.status import HTTP_500_INTERNAL_SERVER_ERROR from rest_framework.test import APIClient @@ -28,3 +31,33 @@ def test_frontend_report_invalid_data(self): } response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST) + + @patch( + "kolibri.core.errorreports.models.ErrorReports.insert_or_update_error", + side_effect=AttributeError("Mocked AttributeError"), + ) + def test_frontend_report_server_error_attribute_error( + self, mock_insert_or_update_error + ): + url = reverse("kolibri:core:report") + data = { + "error_message": "Something went wrong", + "traceback": "Traceback information", + } + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, HTTP_500_INTERNAL_SERVER_ERROR) + self.assertIn("error", response.data) + + @patch( + "kolibri.core.errorreports.models.ErrorReports.insert_or_update_error", + side_effect=Exception("Mocked exception"), + ) + def test_frontend_report_server_error(self, mock_insert_or_update_error): + url = reverse("kolibri:core:report") + data = { + "error_message": "Something went wrong", + "traceback": "Traceback information", + } + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, HTTP_500_INTERNAL_SERVER_ERROR) + self.assertIn("error", response.data) From 00b7be189fea313dc84a9cd3496a638b3bd6d61e Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Wed, 12 Jun 2024 20:50:26 +0530 Subject: [PATCH 26/74] test for anything other than AttributeError or Exception can be caught --- kolibri/core/errorreports/test/test_api.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py index ddfdcf60c4c..fb4a687c96a 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/core/errorreports/test/test_api.py @@ -1,5 +1,6 @@ from unittest.mock import patch +from django.db.utils import IntegrityError from django.test import TestCase from django.urls import reverse from rest_framework.status import HTTP_200_OK @@ -52,7 +53,26 @@ def test_frontend_report_server_error_attribute_error( "kolibri.core.errorreports.models.ErrorReports.insert_or_update_error", side_effect=Exception("Mocked exception"), ) - def test_frontend_report_server_error(self, mock_insert_or_update_error): + def test_frontend_report_server_error_general_exception( + self, mock_insert_or_update_error + ): + url = reverse("kolibri:core:report") + data = { + "error_message": "Something went wrong", + "traceback": "Traceback information", + } + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, HTTP_500_INTERNAL_SERVER_ERROR) + self.assertIn("error", response.data) + + @patch( + "kolibri.core.errorreports.models.ErrorReports.insert_or_update_error", + side_effect=IntegrityError("Mocked exception integrity error"), + ) + def test_frontend_report_server_error_any_other_exception( + self, mock_insert_or_update_error + ): + # this is to check that anything other than AttributeError or Exception can be caught url = reverse("kolibri:core:report") data = { "error_message": "Something went wrong", From 77507a4bb37571a0448b7ec9bedb48b63c93bd7c Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 17 Jun 2024 23:45:12 +0530 Subject: [PATCH 27/74] error-handling mechanism --- kolibri/core/assets/src/core-app/index.js | 22 ++++++++++++++++++++ kolibri/core/assets/src/utils/reportError.js | 22 ++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 kolibri/core/assets/src/utils/reportError.js diff --git a/kolibri/core/assets/src/core-app/index.js b/kolibri/core/assets/src/core-app/index.js index 10e451939c4..5bce884bfd7 100644 --- a/kolibri/core/assets/src/core-app/index.js +++ b/kolibri/core/assets/src/core-app/index.js @@ -16,6 +16,7 @@ import heartbeat from 'kolibri.heartbeat'; import ContentRenderer from '../views/ContentRenderer'; import initializeTheme from '../styles/initializeTheme'; import { i18nSetup } from '../utils/i18n'; +import report from '../utils/reportError'; import setupPluginMediator from './pluginMediator'; import apiSpec from './apiSpec'; @@ -71,6 +72,27 @@ heartbeat.startPolling(); i18nSetup().then(coreApp.ready); +// these shall be responsibe for catching runtime errors + +//for errors from Vue components + +Vue.config.errorHandler = function(err, vm, info) { + console.error('Unexpected Error: ', err.message, err.stack, vm, info); // eslint-disable-line no-console + report(err); +}; + +window.addEventListener('error', e => { + const { message, filename, lineno, colno, error } = e; + console.error('Unexpected Error: ', message); // eslint-disable-line no-console + report(error); +}); + +window.addEventListener('unhandledrejection', event => { + event.preventDefault(); + console.error('Unhandled Rejection:', event.reason); // eslint-disable-line no-console + report(event.reason); +}); + // This is exported by webpack as the kolibriCoreAppGlobal object, due to the 'output.library' flag // which exports the coreApp at the bottom of this file as a named global variable: // https://webpack.github.io/docs/configuration.html#output-library diff --git a/kolibri/core/assets/src/utils/reportError.js b/kolibri/core/assets/src/utils/reportError.js new file mode 100644 index 00000000000..28e8b0039e5 --- /dev/null +++ b/kolibri/core/assets/src/utils/reportError.js @@ -0,0 +1,22 @@ +import urls from 'kolibri.urls'; +import { httpClient } from 'kolibri.client'; + +function report(error) { + const url = urls['kolibri:core:report'](); + const data = { + error_message: error.message, + traceback: error.stack, + }; + httpClient({ + method: 'POST', + url, + data, + }) + .then(response => { + console.log('Error reported successfully:', response.data); // eslint-disable-line no-console + }) + .catch(err => { + console.error('Failed to report error:', err); // eslint-disable-line no-console + }); +} +export default report; From 30c13b46a974854002940ba5c7637f8245507ced Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 18 Jun 2024 16:23:17 +0530 Subject: [PATCH 28/74] optimized code for backend call --- .../__tests__/errorReport.test.js | 37 +++++++++++++++++++ .../assets/src/api-resources/errorReport.js | 18 +++++++++ .../core/assets/src/api-resources/index.js | 1 + kolibri/core/assets/src/core-app/index.js | 20 ++++------ 4 files changed, 64 insertions(+), 12 deletions(-) create mode 100644 kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js create mode 100644 kolibri/core/assets/src/api-resources/errorReport.js diff --git a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js b/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js new file mode 100644 index 00000000000..77896ec347d --- /dev/null +++ b/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js @@ -0,0 +1,37 @@ +import urls from 'kolibri.urls'; +import Resource from '../errorReport'; + +/* eslint-env jest */ +jest.mock('kolibri.urls', () => ({ + 'kolibri:core:report': jest.fn(), +})); + +describe('Error Report', () => { + beforeEach(() => { + urls['kolibri:core:report'].mockReturnValue('/api/core/report'); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should call api/core/report with the error message and traceback', () => { + const error = new Error('My error'); + error.stack = 'My stack trace'; + + const expectedData = { + error_message: 'My error', + traceback: 'My stack trace', + }; + + Resource.client = jest.fn(); + + Resource.report(error); + + expect(Resource.client).toHaveBeenCalledWith({ + url: '/api/core/report', + method: 'post', + data: expectedData, + }); + }); +}); diff --git a/kolibri/core/assets/src/api-resources/errorReport.js b/kolibri/core/assets/src/api-resources/errorReport.js new file mode 100644 index 00000000000..d5f29460d93 --- /dev/null +++ b/kolibri/core/assets/src/api-resources/errorReport.js @@ -0,0 +1,18 @@ +import { Resource } from 'kolibri.lib.apiResource'; +import urls from 'kolibri.urls'; + +export default new Resource({ + name: 'errorreports', + report(error) { + const url = urls['kolibri:core:report'](); + const data = { + error_message: error.message, + traceback: error.stack, + }; + return this.client({ + url, + method: 'post', + data: data, + }); + }, +}); diff --git a/kolibri/core/assets/src/api-resources/index.js b/kolibri/core/assets/src/api-resources/index.js index 2fd46b3e9da..8e0b2331b31 100644 --- a/kolibri/core/assets/src/api-resources/index.js +++ b/kolibri/core/assets/src/api-resources/index.js @@ -24,6 +24,7 @@ export { default as UserSyncStatusResource } from './userSyncStatus'; export { default as ContentRequestResource } from './contentRequest'; export { default as ContentNodeProgressResource } from './contentNodeProgress'; export { default as DevicePermissionsResource } from './devicePermissions'; +export { default as ErrorReportResource } from './errorReport'; export { default as RemoteChannelResource } from './remoteChannel'; export { default as LessonResource } from './lesson'; export { default as AttemptLogResource } from './attemptLog'; diff --git a/kolibri/core/assets/src/core-app/index.js b/kolibri/core/assets/src/core-app/index.js index 5bce884bfd7..5a1359a9d55 100644 --- a/kolibri/core/assets/src/core-app/index.js +++ b/kolibri/core/assets/src/core-app/index.js @@ -16,7 +16,7 @@ import heartbeat from 'kolibri.heartbeat'; import ContentRenderer from '../views/ContentRenderer'; import initializeTheme from '../styles/initializeTheme'; import { i18nSetup } from '../utils/i18n'; -import report from '../utils/reportError'; +import { ErrorReportResource } from '../api-resources'; import setupPluginMediator from './pluginMediator'; import apiSpec from './apiSpec'; @@ -73,24 +73,20 @@ heartbeat.startPolling(); i18nSetup().then(coreApp.ready); // these shall be responsibe for catching runtime errors - -//for errors from Vue components - -Vue.config.errorHandler = function(err, vm, info) { - console.error('Unexpected Error: ', err.message, err.stack, vm, info); // eslint-disable-line no-console - report(err); +Vue.config.errorHandler = function(err) { + logging.error(`Unexpected Error: ${err}`); + ErrorReportResource.report(err); }; window.addEventListener('error', e => { - const { message, filename, lineno, colno, error } = e; - console.error('Unexpected Error: ', message); // eslint-disable-line no-console - report(error); + logging.error(`Unexpected Error: ${e.error}`); + ErrorReportResource.report(e.error); }); window.addEventListener('unhandledrejection', event => { event.preventDefault(); - console.error('Unhandled Rejection:', event.reason); // eslint-disable-line no-console - report(event.reason); + logging.error(`Unhandled Rejection: ${event.reason}`); + ErrorReportResource.report(event.reason); }); // This is exported by webpack as the kolibriCoreAppGlobal object, due to the 'output.library' flag From 2e7539b2abeed56d6eba531b5548dd88798b6cb4 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 18 Jun 2024 16:25:20 +0530 Subject: [PATCH 29/74] remove old reporting util --- kolibri/core/assets/src/utils/reportError.js | 22 -------------------- 1 file changed, 22 deletions(-) delete mode 100644 kolibri/core/assets/src/utils/reportError.js diff --git a/kolibri/core/assets/src/utils/reportError.js b/kolibri/core/assets/src/utils/reportError.js deleted file mode 100644 index 28e8b0039e5..00000000000 --- a/kolibri/core/assets/src/utils/reportError.js +++ /dev/null @@ -1,22 +0,0 @@ -import urls from 'kolibri.urls'; -import { httpClient } from 'kolibri.client'; - -function report(error) { - const url = urls['kolibri:core:report'](); - const data = { - error_message: error.message, - traceback: error.stack, - }; - httpClient({ - method: 'POST', - url, - data, - }) - .then(response => { - console.log('Error reported successfully:', response.data); // eslint-disable-line no-console - }) - .catch(err => { - console.error('Failed to report error:', err); // eslint-disable-line no-console - }); -} -export default report; From 86b980e2a5a0ee4f1f8dfa735639b519570edf5e Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Thu, 20 Jun 2024 20:17:54 +0530 Subject: [PATCH 30/74] pass entire error object to report() --- .../__tests__/errorReport.test.js | 64 +++++++++++++++++-- .../assets/src/api-resources/errorReport.js | 5 +- kolibri/core/assets/src/core-app/index.js | 16 +++-- .../core/assets/src/utils/errorReportUtils.js | 36 +++++++++++ 4 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 kolibri/core/assets/src/utils/errorReportUtils.js diff --git a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js b/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js index 77896ec347d..e99bff9b925 100644 --- a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js +++ b/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js @@ -1,5 +1,10 @@ import urls from 'kolibri.urls'; import Resource from '../errorReport'; +import { + VueErrorReport, + JavascriptErrorReport, + UnhandledRejectionErrorReport, +} from '../../utils/errorReportUtils'; /* eslint-env jest */ jest.mock('kolibri.urls', () => ({ @@ -15,18 +20,67 @@ describe('Error Report', () => { jest.clearAllMocks(); }); - it('should call api/core/report with the error message and traceback', () => { - const error = new Error('My error'); - error.stack = 'My stack trace'; + it('should call api/core/report with VueErrorReport data', () => { + const vueError = new Error('Vue error'); + vueError.stack = 'My stack trace'; + const errorReport = new VueErrorReport(vueError); const expectedData = { - error_message: 'My error', + error_message: 'Vue error', traceback: 'My stack trace', }; Resource.client = jest.fn(); - Resource.report(error); + Resource.report(errorReport); + + expect(Resource.client).toHaveBeenCalledWith({ + url: '/api/core/report', + method: 'post', + data: expectedData, + }); + }); + + it('should call api/core/report with JavascriptErrorReport data', () => { + const jsErrorEvent = { + error: new Error('Javascript error'), + }; + jsErrorEvent.error.stack = 'My stack trace'; + + const errorReport = new JavascriptErrorReport(jsErrorEvent); + + const expectedData = { + error_message: 'Javascript error', + traceback: 'My stack trace', + }; + + Resource.client = jest.fn(); + + Resource.report(errorReport); + + expect(Resource.client).toHaveBeenCalledWith({ + url: '/api/core/report', + method: 'post', + data: expectedData, + }); + }); + + it('should call api/core/report with UnhandledRejectionErrorReport data', () => { + const rejectionEvent = { + reason: new Error('Unhandled rejection'), + }; + rejectionEvent.reason.stack = 'My stack trace'; + + const errorReport = new UnhandledRejectionErrorReport(rejectionEvent); + + const expectedData = { + error_message: 'Unhandled rejection', + traceback: 'My stack trace', // This will be 'undefined' because 'reason.stack' is not set in this mock scenario + }; + + Resource.client = jest.fn(); + + Resource.report(errorReport); expect(Resource.client).toHaveBeenCalledWith({ url: '/api/core/report', diff --git a/kolibri/core/assets/src/api-resources/errorReport.js b/kolibri/core/assets/src/api-resources/errorReport.js index d5f29460d93..11289ee9c6a 100644 --- a/kolibri/core/assets/src/api-resources/errorReport.js +++ b/kolibri/core/assets/src/api-resources/errorReport.js @@ -5,10 +5,7 @@ export default new Resource({ name: 'errorreports', report(error) { const url = urls['kolibri:core:report'](); - const data = { - error_message: error.message, - traceback: error.stack, - }; + const data = error.getErrorReport(); return this.client({ url, method: 'post', diff --git a/kolibri/core/assets/src/core-app/index.js b/kolibri/core/assets/src/core-app/index.js index 5a1359a9d55..b2a920e5050 100644 --- a/kolibri/core/assets/src/core-app/index.js +++ b/kolibri/core/assets/src/core-app/index.js @@ -17,8 +17,13 @@ import ContentRenderer from '../views/ContentRenderer'; import initializeTheme from '../styles/initializeTheme'; import { i18nSetup } from '../utils/i18n'; import { ErrorReportResource } from '../api-resources'; -import setupPluginMediator from './pluginMediator'; +import { + VueErrorReport, + JavascriptErrorReport, + UnhandledRejectionErrorReport, +} from '../utils/errorReportUtils'; import apiSpec from './apiSpec'; +import setupPluginMediator from './pluginMediator'; // Do this before any async imports to ensure that public paths // are set correctly @@ -75,18 +80,21 @@ i18nSetup().then(coreApp.ready); // these shall be responsibe for catching runtime errors Vue.config.errorHandler = function(err) { logging.error(`Unexpected Error: ${err}`); - ErrorReportResource.report(err); + const error = new VueErrorReport(err); + ErrorReportResource.report(error); }; window.addEventListener('error', e => { logging.error(`Unexpected Error: ${e.error}`); - ErrorReportResource.report(e.error); + const error = new JavascriptErrorReport(e); + ErrorReportResource.report(error); }); window.addEventListener('unhandledrejection', event => { event.preventDefault(); logging.error(`Unhandled Rejection: ${event.reason}`); - ErrorReportResource.report(event.reason); + const error = new UnhandledRejectionErrorReport(event); + ErrorReportResource.report(error); }); // This is exported by webpack as the kolibriCoreAppGlobal object, due to the 'output.library' flag diff --git a/kolibri/core/assets/src/utils/errorReportUtils.js b/kolibri/core/assets/src/utils/errorReportUtils.js new file mode 100644 index 00000000000..e639623461f --- /dev/null +++ b/kolibri/core/assets/src/utils/errorReportUtils.js @@ -0,0 +1,36 @@ +class ErrorReport { + constructor(e) { + this.e = e; + } + + getErrorReport() { + throw new Error('getErrorReport() method must be implemented.'); + } +} + +export class VueErrorReport extends ErrorReport { + getErrorReport() { + return { + error_message: this.e.message, + traceback: this.e.stack, + }; + } +} + +export class JavascriptErrorReport extends ErrorReport { + getErrorReport() { + return { + error_message: this.e.error.message, + traceback: this.e.error.stack, + }; + } +} + +export class UnhandledRejectionErrorReport extends ErrorReport { + getErrorReport() { + return { + error_message: this.e.reason.message, + traceback: this.e.reason.stack, + }; + } +} From a0ea7b494382204b5db01eb7da7181bee2575682 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Thu, 20 Jun 2024 20:20:07 +0530 Subject: [PATCH 31/74] useless comment --- .../core/assets/src/api-resources/__tests__/errorReport.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js b/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js index e99bff9b925..7dc0eecd5eb 100644 --- a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js +++ b/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js @@ -75,7 +75,7 @@ describe('Error Report', () => { const expectedData = { error_message: 'Unhandled rejection', - traceback: 'My stack trace', // This will be 'undefined' because 'reason.stack' is not set in this mock scenario + traceback: 'My stack trace', }; Resource.client = jest.fn(); From d389b2787a36b1fede8e538c15c274de14f3080b Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 25 Jun 2024 20:27:39 +0530 Subject: [PATCH 32/74] 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 33/74] 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 34/74] 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 35/74] 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 36/74] 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 37/74] 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) From d02097fd8b26ef8ae8e2023083f7d0147d82088c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Thu, 25 Jul 2024 19:07:38 +0000 Subject: [PATCH 38/74] [pre-commit.ci lite] apply automatic fixes --- kolibri/core/assets/src/core-app/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/assets/src/core-app/index.js b/kolibri/core/assets/src/core-app/index.js index b2a920e5050..42ea08b554e 100644 --- a/kolibri/core/assets/src/core-app/index.js +++ b/kolibri/core/assets/src/core-app/index.js @@ -78,7 +78,7 @@ heartbeat.startPolling(); i18nSetup().then(coreApp.ready); // these shall be responsibe for catching runtime errors -Vue.config.errorHandler = function(err) { +Vue.config.errorHandler = function (err) { logging.error(`Unexpected Error: ${err}`); const error = new VueErrorReport(err); ErrorReportResource.report(error); From d9ef827178a43624d362ae27eeb8c6ca706a8c92 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 1 Jul 2024 13:59:41 +0530 Subject: [PATCH 39/74] update ErrorReports for new fields --- .../0002_newfields_renamedfields.py | 114 +++++++++++++ kolibri/core/errorreports/models.py | 55 +++++- kolibri/core/errorreports/schemas.py | 54 ++++++ kolibri/core/errorreports/test/test_models.py | 160 +++++++++++++----- 4 files changed, 330 insertions(+), 53 deletions(-) create mode 100644 kolibri/core/errorreports/migrations/0002_newfields_renamedfields.py create mode 100644 kolibri/core/errorreports/schemas.py diff --git a/kolibri/core/errorreports/migrations/0002_newfields_renamedfields.py b/kolibri/core/errorreports/migrations/0002_newfields_renamedfields.py new file mode 100644 index 00000000000..3954d34886b --- /dev/null +++ b/kolibri/core/errorreports/migrations/0002_newfields_renamedfields.py @@ -0,0 +1,114 @@ +# Generated by Django 3.2.25 on 2024-06-29 11:09 +from django.db import migrations +from django.db import models + +import kolibri.core.fields +import kolibri.core.utils.validators + + +class Migration(migrations.Migration): + + dependencies = [ + ("errorreports", "0001_initial"), + ] + + operations = [ + migrations.RenameField( + model_name="errorreports", + old_name="error_from", + new_name="category", + ), + migrations.RenameField( + model_name="errorreports", + old_name="no_of_errors", + new_name="events", + ), + migrations.RenameField( + model_name="errorreports", + old_name="sent", + new_name="reported", + ), + migrations.AddField( + model_name="errorreports", + name="context_backend", + field=kolibri.core.fields.JSONField( + blank=True, + default={ + "packages": {}, + "python_version": "", + "request_info": {}, + "server": {}, + }, + null=True, + validators=[ + kolibri.core.utils.validators.JSON_Schema_Validator( + { + "properties": { + "packages": {"optional": True, "type": "object"}, + "python_version": {"optional": True, "type": "string"}, + "request_info": { + "properties": { + "body": {"optional": True, "type": "string"}, + "headers": {"optional": True, "type": "object"}, + "method": {"optional": True, "type": "string"}, + "url": {"optional": True, "type": "string"}, + }, + "type": "object", + }, + "server": { + "properties": { + "host": {"optional": True, "type": "string"}, + "port": {"optional": True, "type": "integer"}, + }, + "type": "object", + }, + }, + "type": "object", + } + ) + ], + ), + ), + migrations.AddField( + model_name="errorreports", + name="context_frontend", + field=kolibri.core.fields.JSONField( + blank=True, + default={"browser": "", "component": "N/A", "device": {}}, + null=True, + validators=[ + kolibri.core.utils.validators.JSON_Schema_Validator( + { + "properties": { + "browser": {"type": "string"}, + "component": {"optional": True, "type": "string"}, + "device": { + "properties": { + "platform": { + "optional": True, + "type": "string", + }, + "screen": { + "properties": { + "height": {"type": "integer"}, + "width": {"type": "integer"}, + }, + "type": "object", + }, + "type": {"type": "string"}, + }, + "type": "object", + }, + }, + "type": "object", + } + ) + ], + ), + ), + migrations.AddField( + model_name="errorreports", + name="release_version", + field=models.CharField(default="0.16", max_length=64), + ), + ] diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index 7ace64986df..ebb80423f44 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -5,6 +5,13 @@ from django.utils import timezone from .constants import POSSIBLE_ERRORS +from .schemas import context_backend_schema +from .schemas import context_frontend_schema +from .schemas import default_context_backend_schema +from .schemas import default_context_frontend_schema +from kolibri import VERSION +from kolibri.core.fields import JSONField +from kolibri.core.utils.validators import JSON_Schema_Validator from kolibri.deployment.default.sqlite_db_names import ERROR_REPORTS @@ -47,25 +54,55 @@ def allow_migrate(self, db, app_label, model_name=None, **hints): class ErrorReports(models.Model): - error_from = models.CharField(max_length=10, choices=POSSIBLE_ERRORS) + category = models.CharField(max_length=10, choices=POSSIBLE_ERRORS) error_message = models.CharField(max_length=255) traceback = models.TextField() first_occurred = models.DateTimeField(default=timezone.now) last_occurred = models.DateTimeField(default=timezone.now) - sent = models.BooleanField(default=False) - no_of_errors = models.IntegerField(default=1) + reported = models.BooleanField(default=False) + events = models.IntegerField(default=1) + release_version = models.CharField( + max_length=64, default=".".join(map(str, VERSION[:2])) + ) + context_frontend = JSONField( + null=True, + blank=True, + validators=[JSON_Schema_Validator(context_frontend_schema)], + default=default_context_frontend_schema, + ) + context_backend = JSONField( + null=True, + blank=True, + validators=[JSON_Schema_Validator(context_backend_schema)], + default=default_context_backend_schema, + ) def __str__(self): - return f"{self.error_message} ({self.error_from})" + return f"{self.error_message} ({self.category})" + + def mark_reported(self): + self.reported = True + self.save() @classmethod - def insert_or_update_error(cls, error_from, error_message, traceback): + def insert_or_update_error( + cls, + category, + error_message, + traceback, + context_frontend=None, + context_backend=None, + ): if not getattr(settings, "DEVELOPER_MODE", None): error, created = cls.objects.get_or_create( - error_from=error_from, error_message=error_message, traceback=traceback + category=category, + error_message=error_message, + traceback=traceback, + context_frontend=context_frontend, + context_backend=context_backend, ) if not created: - error.no_of_errors += 1 + error.events += 1 error.last_occurred = timezone.now() error.save() logger.error("ErrorReports: Database updated.") @@ -74,8 +111,8 @@ def insert_or_update_error(cls, error_from, error_message, traceback): return None @classmethod - def get_unsent_errors(cls): - return cls.objects.filter(sent=False) + def get_unreported_errors(cls): + return cls.objects.filter(reported=False) @classmethod def delete_error(cls): diff --git a/kolibri/core/errorreports/schemas.py b/kolibri/core/errorreports/schemas.py new file mode 100644 index 00000000000..84822e0abd6 --- /dev/null +++ b/kolibri/core/errorreports/schemas.py @@ -0,0 +1,54 @@ +context_frontend_schema = { + "type": "object", + "properties": { + "browser": {"type": "string"}, + "component": {"type": "string", "optional": True}, + "device": { + "type": "object", + "properties": { + "type": {"type": "string"}, # "desktop", "tablet", "mobile" + "platform": { + "type": "string", + "optional": True, + }, # "windows", "mac", "linux", "android", "ios" + "screen": { + "type": "object", + "properties": { + "width": {"type": "integer"}, + "height": {"type": "integer"}, + }, + }, + }, + }, + }, +} +default_context_frontend_schema = {"browser": "", "component": "N/A", "device": {}} +context_backend_schema = { + "type": "object", + "properties": { + "request_info": { + "type": "object", + "properties": { + "url": {"type": "string", "optional": True}, + "method": {"type": "string", "optional": True}, + "headers": {"type": "object", "optional": True}, + "body": {"type": "string", "optional": True}, + }, + }, + "server": { + "type": "object", + "properties": { + "host": {"type": "string", "optional": True}, + "port": {"type": "integer", "optional": True}, + }, + }, + "packages": {"type": "object", "optional": True}, + "python_version": {"type": "string", "optional": True}, + }, +} +default_context_backend_schema = { + "request_info": {}, + "server": {}, + "packages": {}, + "python_version": "", +} diff --git a/kolibri/core/errorreports/test/test_models.py b/kolibri/core/errorreports/test/test_models.py index f5c05d6a93a..a823e3e8431 100644 --- a/kolibri/core/errorreports/test/test_models.py +++ b/kolibri/core/errorreports/test/test_models.py @@ -8,22 +8,68 @@ class ErrorReportsTestCase(TestCase): - databases = "__all__" # I am not sure about this, maybe a overkill but works + databases = "__all__" + + def setUp(self): + self.category_frontend = FRONTEND + self.category_backend = BACKEND + self.error_message = "Test Error" + self.traceback = "Test Traceback" + self.context_frontend = { + "browser": "Chrome", + "component": "HeaderComponent", + "device": { + "type": "desktop", + "platform": "windows", + "screen": {"width": 1920, "height": 1080}, + }, + } + self.context_backend = { + "request_info": { + "url": "/api/test", + "method": "GET", + "headers": {"User-Agent": "TestAgent"}, + "body": "", + }, + "server": {"host": "localhost", "port": 8000}, + "packages": {"django": "3.2", "kolibri": "0.15.8"}, + "python_version": "3.9.1", + } + + def create_error( + self, + category, + error_message, + traceback, + context_frontend, + context_backend, + reported=False, + ): + return ErrorReports.objects.create( + category=category, + error_message=error_message, + traceback=traceback, + context_frontend=context_frontend, + context_backend=context_backend, + reported=reported, + ) @override_settings(DEVELOPER_MODE=False) def test_insert_or_update_error_prod_mode(self): - error_from = FRONTEND - error_message = "Test Error" - traceback = "Test Traceback" - error = ErrorReports.insert_or_update_error( - error_from, error_message, traceback + self.category_frontend, + self.error_message, + self.traceback, + self.context_frontend, + self.context_backend, ) - self.assertEqual(error.error_from, error_from) - self.assertEqual(error.error_message, error_message) - self.assertEqual(error.traceback, traceback) - self.assertEqual(error.no_of_errors, 1) - self.assertFalse(error.sent) + self.assertEqual(error.category, self.category_frontend) + self.assertEqual(error.error_message, self.error_message) + self.assertEqual(error.traceback, self.traceback) + self.assertEqual(error.context_frontend, self.context_frontend) + self.assertEqual(error.context_backend, self.context_backend) + self.assertEqual(error.events, 1) + self.assertFalse(error.reported) self.assertLess( timezone.now() - error.first_occurred, timezone.timedelta(seconds=1) ) @@ -31,15 +77,21 @@ def test_insert_or_update_error_prod_mode(self): timezone.now() - error.last_occurred, timezone.timedelta(seconds=1) ) - # creating the error again, so this time it should update the error + # Creating the error again, so this time it should update the error error = ErrorReports.insert_or_update_error( - error_from, error_message, traceback + self.category_frontend, + self.error_message, + self.traceback, + self.context_frontend, + self.context_backend, ) - self.assertEqual(error.error_from, error_from) - self.assertEqual(error.error_message, error_message) - self.assertEqual(error.traceback, traceback) - self.assertEqual(error.no_of_errors, 2) - self.assertFalse(error.sent) + self.assertEqual(error.category, self.category_frontend) + self.assertEqual(error.error_message, self.error_message) + self.assertEqual(error.traceback, self.traceback) + self.assertEqual(error.context_frontend, self.context_frontend) + self.assertEqual(error.context_backend, self.context_backend) + self.assertEqual(error.events, 2) + self.assertFalse(error.reported) self.assertLess( timezone.now() - error.first_occurred, timezone.timedelta(seconds=1) ) @@ -49,37 +101,57 @@ def test_insert_or_update_error_prod_mode(self): @override_settings(DEVELOPER_MODE=True) def test_insert_or_update_error_dev_mode(self): - error_from = FRONTEND - error_message = "Test Error" - traceback = "Test Traceback" - error = ErrorReports.insert_or_update_error( - error_from, error_message, traceback + self.category_frontend, + self.error_message, + self.traceback, + self.context_frontend, + self.context_backend, ) self.assertIsNone(error) - def test_get_unsent_errors(self): - ErrorReports.objects.create( - error_from=FRONTEND, - error_message="Error 1", - traceback="Traceback 1", - sent=False, + def test_get_unreported_errors(self): + self.create_error( + self.category_frontend, + "Error 1", + "Traceback 1", + self.context_frontend, + self.context_backend, + reported=False, ) - ErrorReports.objects.create( - error_from=BACKEND, - error_message="Error 2", - traceback="Traceback 2", - sent=False, + self.create_error( + self.category_backend, + "Error 2", + "Traceback 2", + self.context_frontend, + self.context_backend, + reported=False, ) - ErrorReports.objects.create( - error_from=BACKEND, - error_message="Error 3", - traceback="Traceback 3", - sent=True, + self.create_error( + self.category_backend, + "Error 3", + "Traceback 3", + self.context_frontend, + self.context_backend, + reported=True, ) - # Get unsent errors, shall be only 2 as out of 3, 1 is sent - unsent_errors = ErrorReports.get_unsent_errors() - self.assertEqual(unsent_errors.count(), 2) - self.assertFalse(unsent_errors[0].sent) - self.assertFalse(unsent_errors[1].sent) + # Get unreported errors, should be only 2 as out of 3, 1 is reported + unreported_errors = ErrorReports.get_unreported_errors() + self.assertEqual(unreported_errors.count(), 2) + self.assertFalse(unreported_errors[0].reported) + self.assertFalse(unreported_errors[1].reported) + + def test_mark_reported(self): + error = self.create_error( + self.category_frontend, + self.error_message, + self.traceback, + self.context_frontend, + self.context_backend, + reported=False, + ) + # First check error is unreported, then set to True and assert again + self.assertFalse(error.reported) + error.mark_reported() + self.assertTrue(error.reported) From 984261fcf8775d1964fb76e5875ae913e524d05f Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 1 Jul 2024 14:01:40 +0530 Subject: [PATCH 40/74] add context field in errorreports/report/ --- kolibri/core/errorreports/api.py | 19 +++++++-- kolibri/core/errorreports/serializers.py | 6 +++ kolibri/core/errorreports/test/test_api.py | 45 ++++++++++------------ 3 files changed, 42 insertions(+), 28 deletions(-) diff --git a/kolibri/core/errorreports/api.py b/kolibri/core/errorreports/api.py index 60e52e1f2f8..52d4f312059 100644 --- a/kolibri/core/errorreports/api.py +++ b/kolibri/core/errorreports/api.py @@ -18,6 +18,17 @@ def report(request): curl -X POST http://localhost:8000/api/errorreports/report/ -H "Content-Type: application/json" -d '{ "error_message": "An example error occurred", "traceback": "Traceback (most recent call last): ..." + "context": { + "browser":"", + "component":"my component", + "device":{ + "type": "desktop", + "platform":"Windows", + "screen": { + "width":545, + "height":858 + } + } }' """ serializer = ErrorReportSerializer(data=request.data) @@ -25,13 +36,15 @@ def report(request): data = serializer.validated_data try: error = ErrorReports.insert_or_update_error( - error_from=FRONTEND, - error_message=data["error_message"], - traceback=data["traceback"], + FRONTEND, + data["error_message"], + data["traceback"], + context_frontend=data["context"], ) return Response( {"error_id": error.id if error else None}, status=status.HTTP_200_OK ) + except (AttributeError, Exception) as e: logger.error("Error while saving error report: {}".format(e)) return Response( diff --git a/kolibri/core/errorreports/serializers.py b/kolibri/core/errorreports/serializers.py index 7e67d84cdb3..b682b8eebeb 100644 --- a/kolibri/core/errorreports/serializers.py +++ b/kolibri/core/errorreports/serializers.py @@ -1,6 +1,12 @@ from rest_framework import serializers +from .schemas import context_frontend_schema +from kolibri.core.utils.validators import JSON_Schema_Validator + class ErrorReportSerializer(serializers.Serializer): error_message = serializers.CharField(max_length=255) traceback = serializers.CharField() + context = serializers.JSONField( + validators=[JSON_Schema_Validator(context_frontend_schema)] + ) diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py index fb4a687c96a..8e9422d7abf 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/core/errorreports/test/test_api.py @@ -11,26 +11,33 @@ class FrontendReportTestCase(TestCase): databases = "__all__" + data = { + "error_message": "Something went wrong", + "traceback": "Traceback information", + "context": { + "browser": "", + "component": "my component", + "device": { + "type": "desktop", + "platform": "Windows", + "screen": {"width": 303, "height": 230}, + }, + }, + } def setUp(self): self.client = APIClient() def test_frontend_report(self): url = reverse("kolibri:core:report") - data = { - "error_message": "Something went wrong", - "traceback": "Traceback information", - } - response = self.client.post(url, data, format="json") + response = self.client.post(url, self.data, format="json") self.assertEqual(response.status_code, HTTP_200_OK) def test_frontend_report_invalid_data(self): url = reverse("kolibri:core:report") - data = { - "error_message": "", - "traceback": "Traceback information", - } - response = self.client.post(url, data, format="json") + data = self.data.copy() + invalid_data = data.pop("context") + response = self.client.post(url, invalid_data, format="json") self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST) @patch( @@ -41,11 +48,7 @@ def test_frontend_report_server_error_attribute_error( self, mock_insert_or_update_error ): url = reverse("kolibri:core:report") - data = { - "error_message": "Something went wrong", - "traceback": "Traceback information", - } - response = self.client.post(url, data, format="json") + response = self.client.post(url, self.data, format="json") self.assertEqual(response.status_code, HTTP_500_INTERNAL_SERVER_ERROR) self.assertIn("error", response.data) @@ -57,11 +60,7 @@ def test_frontend_report_server_error_general_exception( self, mock_insert_or_update_error ): url = reverse("kolibri:core:report") - data = { - "error_message": "Something went wrong", - "traceback": "Traceback information", - } - response = self.client.post(url, data, format="json") + response = self.client.post(url, self.data, format="json") self.assertEqual(response.status_code, HTTP_500_INTERNAL_SERVER_ERROR) self.assertIn("error", response.data) @@ -74,10 +73,6 @@ def test_frontend_report_server_error_any_other_exception( ): # this is to check that anything other than AttributeError or Exception can be caught url = reverse("kolibri:core:report") - data = { - "error_message": "Something went wrong", - "traceback": "Traceback information", - } - response = self.client.post(url, data, format="json") + response = self.client.post(url, self.data, format="json") self.assertEqual(response.status_code, HTTP_500_INTERNAL_SERVER_ERROR) self.assertIn("error", response.data) From 44ce4e1b4bc2ae93d22023d3803517cbd5b44732 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 1 Jul 2024 14:02:40 +0530 Subject: [PATCH 41/74] update middleware to capture more fields --- kolibri/core/errorreports/middleware.py | 33 +++++++++++++++- .../core/errorreports/test/test_middleware.py | 39 +++++++++++++++++-- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index 0694fa9c167..bcaf1c1ea3b 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -1,12 +1,35 @@ import logging import traceback +from sys import version +import pkg_resources from django.db import IntegrityError from .constants import BACKEND from .models import ErrorReports +def get_request_info(request): + return { + "url": request.build_absolute_uri(), + "method": request.method, + "headers": dict(request.headers), + "body": request.body.decode("utf-8"), + } + + +def get_server_info(request): + return {"host": request.get_host(), "port": request.get_port()} + + +def get_packages(): + return {dist.project_name: dist.version for dist in pkg_resources.working_set} + + +def get_python_version(): + return version.split()[0] + + class ErrorReportingMiddleware: """ Middleware to log exceptions to the database. @@ -23,11 +46,19 @@ def __call__(self, request): def process_exception(self, request, exception): error_message = str(exception) traceback_info = traceback.format_exc() + context = { + "request_info": get_request_info(request), + "server": get_server_info(request), + "packages": get_packages(), + "python_version": get_python_version(), + } self.logger.error("Unexpected Error: %s", error_message) try: self.logger.error("Saving error report to the database.") - ErrorReports.insert_or_update_error(BACKEND, error_message, traceback_info) + ErrorReports.insert_or_update_error( + BACKEND, error_message, traceback_info, context_backend=context + ) self.logger.info("Error report saved to the database.") except IntegrityError as e: self.logger.error( diff --git a/kolibri/core/errorreports/test/test_middleware.py b/kolibri/core/errorreports/test/test_middleware.py index 019e8818110..32ef36bc01a 100644 --- a/kolibri/core/errorreports/test/test_middleware.py +++ b/kolibri/core/errorreports/test/test_middleware.py @@ -3,6 +3,7 @@ from unittest.mock import patch from django.db import IntegrityError +from django.test import RequestFactory from django.test import TestCase from ..constants import BACKEND @@ -11,11 +12,27 @@ class ErrorReportingMiddlewareTestCase(TestCase): + def setUp(self): + self.factory = RequestFactory() + + @patch( + "kolibri.core.errorreports.middleware.get_python_version", return_value="3.9.9" + ) + @patch( + "kolibri.core.errorreports.middleware.get_packages", + return_value={"Django": "3.2.25"}, + ) @patch.object(ErrorReports, "insert_or_update_error") @patch.object(logging.Logger, "error") - def test_process_exception(self, mock_logger_error, mock_insert_or_update_error): + def test_process_exception( + self, + mock_logger_error, + mock_insert_or_update_error, + mock_get_packages, + mock_get_python_version, + ): middleware = ErrorReportingMiddleware(lambda r: None) - request = self.client.request() + request = self.factory.get("/") exception = Exception("Test Exception") try: raise exception @@ -27,8 +44,22 @@ def test_process_exception(self, mock_logger_error, mock_insert_or_update_error) type(exception), exception, exception.__traceback__ ) ) + mock_insert_or_update_error.assert_called_once_with( - BACKEND, str(exception), expected_traceback_info + BACKEND, + str(exception), + expected_traceback_info, + context_backend={ + "request_info": { + "url": "http://testserver/", + "method": "GET", + "headers": dict(request.headers), + "body": "", + }, + "server": {"host": "testserver", "port": "80"}, + "packages": {"Django": 1.1}, + "python_version": "1.1.1", + }, ) @patch.object(ErrorReports, "insert_or_update_error") @@ -37,7 +68,7 @@ def test_process_exception_integrity_error( self, mock_logger_error, mock_insert_or_update_error ): middleware = ErrorReportingMiddleware(lambda r: None) - request = self.client.request() + request = self.factory.get("/") exception = Exception("Test Exception") mock_insert_or_update_error.side_effect = IntegrityError("Some Integrity Error") middleware.process_exception(request, exception) From 2798cdfdb70dd0ba8de2af3913466022091e3dcd Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 1 Jul 2024 14:04:50 +0530 Subject: [PATCH 42/74] update frontend erroreports to capture more fields --- .../__tests__/errorReport.test.js | 19 ++++++++--- kolibri/core/assets/src/core-app/index.js | 4 +-- .../core/assets/src/utils/errorReportUtils.js | 34 +++++++++++++++++-- 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js b/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js index 7dc0eecd5eb..c699e7ce3fd 100644 --- a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js +++ b/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js @@ -23,15 +23,20 @@ describe('Error Report', () => { it('should call api/core/report with VueErrorReport data', () => { const vueError = new Error('Vue error'); vueError.stack = 'My stack trace'; - const errorReport = new VueErrorReport(vueError); + const vm = { $options: { name: 'TestComponent' } }; + const errorReport = new VueErrorReport(vueError, vm); const expectedData = { error_message: 'Vue error', traceback: 'My stack trace', + context: { + component: 'TestComponent', + browser: errorReport.getBrowserInfo(), + device: errorReport.getDeviceInfo(), + }, }; Resource.client = jest.fn(); - Resource.report(errorReport); expect(Resource.client).toHaveBeenCalledWith({ @@ -52,10 +57,13 @@ describe('Error Report', () => { const expectedData = { error_message: 'Javascript error', traceback: 'My stack trace', + context: { + browser: errorReport.getBrowserInfo(), + device: errorReport.getDeviceInfo(), + }, }; Resource.client = jest.fn(); - Resource.report(errorReport); expect(Resource.client).toHaveBeenCalledWith({ @@ -76,10 +84,13 @@ describe('Error Report', () => { const expectedData = { error_message: 'Unhandled rejection', traceback: 'My stack trace', + context: { + browser: errorReport.getBrowserInfo(), + device: errorReport.getDeviceInfo(), + }, }; Resource.client = jest.fn(); - Resource.report(errorReport); expect(Resource.client).toHaveBeenCalledWith({ diff --git a/kolibri/core/assets/src/core-app/index.js b/kolibri/core/assets/src/core-app/index.js index 42ea08b554e..1eb79b4e519 100644 --- a/kolibri/core/assets/src/core-app/index.js +++ b/kolibri/core/assets/src/core-app/index.js @@ -78,9 +78,9 @@ heartbeat.startPolling(); i18nSetup().then(coreApp.ready); // these shall be responsibe for catching runtime errors -Vue.config.errorHandler = function (err) { +Vue.config.errorHandler = function(err, vm) { logging.error(`Unexpected Error: ${err}`); - const error = new VueErrorReport(err); + const error = new VueErrorReport(err, vm); ErrorReportResource.report(error); }; diff --git a/kolibri/core/assets/src/utils/errorReportUtils.js b/kolibri/core/assets/src/utils/errorReportUtils.js index e639623461f..6b018e6ac54 100644 --- a/kolibri/core/assets/src/utils/errorReportUtils.js +++ b/kolibri/core/assets/src/utils/errorReportUtils.js @@ -6,13 +6,35 @@ class ErrorReport { getErrorReport() { throw new Error('getErrorReport() method must be implemented.'); } + getDeviceInfo() { + return { + type: /Mobi|Android/i.test(navigator.userAgent) ? 'Mobile' : 'Desktop', + platform: navigator.platform, + screen: { + width: window.innerWidth, + height: window.innerHeight, + }, + }; + } + getBrowserInfo() { + return navigator.userAgent; + } } export class VueErrorReport extends ErrorReport { + constructor(e, vm) { + super(e); + this.vm = vm; + } getErrorReport() { return { error_message: this.e.message, traceback: this.e.stack, + context: { + component: this.vm.$options.name || this.vm.$options._componentTag || 'Unknown Component', + browser: this.getBrowserInfo(), + device: this.getDeviceInfo(), + }, }; } } @@ -22,6 +44,10 @@ export class JavascriptErrorReport extends ErrorReport { return { error_message: this.e.error.message, traceback: this.e.error.stack, + context: { + browser: this.getBrowserInfo(), + device: this.getDeviceInfo(), + }, }; } } @@ -29,8 +55,12 @@ export class JavascriptErrorReport extends ErrorReport { export class UnhandledRejectionErrorReport extends ErrorReport { getErrorReport() { return { - error_message: this.e.reason.message, - traceback: this.e.reason.stack, + error_message: this.e.reason ? this.e.reason.message : 'Unhandled Rejection', + traceback: this.e.reason ? this.e.reason.stack : 'No stack trace available', + context: { + browser: this.getBrowserInfo(), + device: this.getDeviceInfo(), + }, }; } } From bea2e64e18e92a8e08c21c1694e188c2d52d87d6 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 1 Jul 2024 14:06:03 +0530 Subject: [PATCH 43/74] update erroreports task to report more fields --- kolibri/core/errorreports/tasks.py | 12 +++++++----- kolibri/core/errorreports/test/test_tasks.py | 0 2 files changed, 7 insertions(+), 5 deletions(-) create mode 100644 kolibri/core/errorreports/test/test_tasks.py diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py index d2b7706e78e..5b9996436bb 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/errorreports/tasks.py @@ -1,8 +1,6 @@ -import json import logging import requests -from django.core.serializers.json import DjangoJSONEncoder from django.db import connection from requests.exceptions import ConnectionError from requests.exceptions import RequestException @@ -20,12 +18,16 @@ def serialize_error_reports_to_json_response(errors): for error in errors: errors_list.append( { - "error_from": error.error_from, + "category": error.category, "error_message": error.error_message, "traceback": error.traceback, "first_occurred": error.first_occurred, "last_occurred": error.last_occurred, - "no_of_errors": error.no_of_errors, + "reported": error.reported, + "events": error.events, + "release_version": error.release_version, + "context_frontend": error.context_frontend, + "context_backend": error.context_backend, } ) return json.dumps(errors_list, cls=DjangoJSONEncoder) @@ -34,7 +36,7 @@ def serialize_error_reports_to_json_response(errors): @register_task def ping_error_reports(server): try: - errors = ErrorReports.get_unsent_errors() + errors = ErrorReports.get_unreported_errors() errors_json = serialize_error_reports_to_json_response(errors) requests.post( join_url(server, "/api/v1/errors/"), diff --git a/kolibri/core/errorreports/test/test_tasks.py b/kolibri/core/errorreports/test/test_tasks.py new file mode 100644 index 00000000000..e69de29bb2d From afef5adc75b8a25f3ad1283910127b56ed0d01c3 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Tue, 2 Jul 2024 17:13:09 +0530 Subject: [PATCH 44/74] add test for tasks --- kolibri/core/errorreports/tasks.py | 9 ++- .../core/errorreports/test/test_middleware.py | 4 +- kolibri/core/errorreports/test/test_tasks.py | 80 +++++++++++++++++++ 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py index 5b9996436bb..47e386f77ca 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/errorreports/tasks.py @@ -1,6 +1,8 @@ +import json import logging import requests +from django.core.serializers.json import DjangoJSONEncoder from django.db import connection from requests.exceptions import ConnectionError from requests.exceptions import RequestException @@ -23,7 +25,6 @@ def serialize_error_reports_to_json_response(errors): "traceback": error.traceback, "first_occurred": error.first_occurred, "last_occurred": error.last_occurred, - "reported": error.reported, "events": error.events, "release_version": error.release_version, "context_frontend": error.context_frontend, @@ -37,13 +38,17 @@ def serialize_error_reports_to_json_response(errors): def ping_error_reports(server): try: errors = ErrorReports.get_unreported_errors() + errors_json = serialize_error_reports_to_json_response(errors) + requests.post( join_url(server, "/api/v1/errors/"), data=errors_json, headers={"Content-Type": "application/json"}, ) - errors.update(sent=True) + + errors.update(reported=True) + except ConnectionError: logger.warning("Reporting Error failed (could not connect).") raise diff --git a/kolibri/core/errorreports/test/test_middleware.py b/kolibri/core/errorreports/test/test_middleware.py index 32ef36bc01a..d1761a5b83d 100644 --- a/kolibri/core/errorreports/test/test_middleware.py +++ b/kolibri/core/errorreports/test/test_middleware.py @@ -57,8 +57,8 @@ def test_process_exception( "body": "", }, "server": {"host": "testserver", "port": "80"}, - "packages": {"Django": 1.1}, - "python_version": "1.1.1", + "packages": {"Django": "3.2.25"}, + "python_version": "3.9.9", }, ) diff --git a/kolibri/core/errorreports/test/test_tasks.py b/kolibri/core/errorreports/test/test_tasks.py index e69de29bb2d..6bcdbc394df 100644 --- a/kolibri/core/errorreports/test/test_tasks.py +++ b/kolibri/core/errorreports/test/test_tasks.py @@ -0,0 +1,80 @@ +from unittest.mock import patch + +import pytest +from django.test import TestCase +from requests.exceptions import ConnectionError +from requests.exceptions import RequestException +from requests.exceptions import Timeout + +from kolibri.core.errorreports.models import ErrorReports +from kolibri.core.errorreports.tasks import ping_error_reports + + +class TestPingErrorReports(TestCase): + databases = "__all__" + + def setUp(self): + ErrorReports.objects.create( + category="frontend", + error_message="Test Error", + traceback="Test Traceback", + context_frontend={ + "browser": "Chrome", + "component": "HeaderComponent", + "device": { + "type": "desktop", + "platform": "windows", + "screen": {"width": 1920, "height": 1080}, + }, + }, + ) + ErrorReports.objects.create( + category="backend", + error_message="Test Error", + traceback="Test Traceback", + context_backend={ + "request_info": { + "url": "/api/test", + "method": "GET", + "headers": {"User-Agent": "TestAgent"}, + "body": "", + }, + "server": {"host": "localhost", "port": 8000}, + "packages": {"django": "3.2", "kolibri": "0.15.8"}, + "python_version": "3.9.1", + }, + ) + + @patch("kolibri.core.errorreports.tasks.requests.post") + @patch( + "kolibri.core.errorreports.tasks.serialize_error_reports_to_json_response", + return_value="[]", + ) + def test_ping_error_reports(self, mock_serializer, mock_post): + ping_error_reports("http://testserver") + mock_post.assert_called_with( + "http://testserver/api/v1/errors/", + data="[]", + headers={"Content-Type": "application/json"}, + ) + self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 2) + + @patch("kolibri.core.errorreports.tasks.requests.post", side_effect=ConnectionError) + def test_ping_error_reports_connection_error(self, mock_post): + with pytest.raises(ConnectionError): + ping_error_reports("http://testserver") + self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 0) + + @patch("kolibri.core.errorreports.tasks.requests.post", side_effect=Timeout) + def test_ping_error_reports_timeout(self, mock_post): + with pytest.raises(Timeout): + ping_error_reports("http://testserver") + self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 0) + + @patch( + "kolibri.core.errorreports.tasks.requests.post", side_effect=RequestException + ) + def test_ping_error_reports_request_exception(self, mock_post): + with pytest.raises(RequestException): + ping_error_reports("http://testserver") + self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 0) From e3252eee44869427498d46f7c317b585029a54ec Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Wed, 3 Jul 2024 23:18:19 +0530 Subject: [PATCH 45/74] update schema of frontend --- ...namedfields.py => 0002_add_more_fields.py} | 27 ++++++++++++++----- kolibri/core/errorreports/models.py | 6 ++++- kolibri/core/errorreports/schemas.py | 21 ++++++++++----- 3 files changed, 40 insertions(+), 14 deletions(-) rename kolibri/core/errorreports/migrations/{0002_newfields_renamedfields.py => 0002_add_more_fields.py} (76%) diff --git a/kolibri/core/errorreports/migrations/0002_newfields_renamedfields.py b/kolibri/core/errorreports/migrations/0002_add_more_fields.py similarity index 76% rename from kolibri/core/errorreports/migrations/0002_newfields_renamedfields.py rename to kolibri/core/errorreports/migrations/0002_add_more_fields.py index 3954d34886b..bed77f59116 100644 --- a/kolibri/core/errorreports/migrations/0002_newfields_renamedfields.py +++ b/kolibri/core/errorreports/migrations/0002_add_more_fields.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-06-29 11:09 +# Generated by Django 3.2.25 on 2024-07-03 17:31 from django.db import migrations from django.db import models @@ -74,20 +74,29 @@ class Migration(migrations.Migration): name="context_frontend", field=kolibri.core.fields.JSONField( blank=True, - default={"browser": "", "component": "N/A", "device": {}}, + default={"browser": {}, "component": "", "device": {}}, null=True, validators=[ kolibri.core.utils.validators.JSON_Schema_Validator( { "properties": { - "browser": {"type": "string"}, + "browser": { + "properties": { + "major": {"optional": True, "type": "string"}, + "minor": {"optional": True, "type": "string"}, + "name": {"optional": True, "type": "string"}, + "patch": {"optional": True, "type": "string"}, + }, + "type": "object", + }, "component": {"optional": True, "type": "string"}, "device": { "properties": { - "platform": { + "is_touch_device": { "optional": True, - "type": "string", + "type": "boolean", }, + "model": {"optional": True, "type": "string"}, "screen": { "properties": { "height": {"type": "integer"}, @@ -95,7 +104,8 @@ class Migration(migrations.Migration): }, "type": "object", }, - "type": {"type": "string"}, + "type": {"optional": True, "type": "string"}, + "vendor": {"optional": True, "type": "string"}, }, "type": "object", }, @@ -106,6 +116,11 @@ class Migration(migrations.Migration): ], ), ), + migrations.AddField( + model_name="errorreports", + name="installation_type", + field=models.CharField(blank=True, max_length=64), + ), migrations.AddField( model_name="errorreports", name="release_version", diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index ebb80423f44..0a824c14728 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -13,6 +13,7 @@ from kolibri.core.fields import JSONField from kolibri.core.utils.validators import JSON_Schema_Validator from kolibri.deployment.default.sqlite_db_names import ERROR_REPORTS +from kolibri.utils.server import installation_type logger = logging.getLogger(__name__) @@ -64,6 +65,7 @@ class ErrorReports(models.Model): release_version = models.CharField( max_length=64, default=".".join(map(str, VERSION[:2])) ) + installation_type = models.CharField(max_length=64, blank=True) context_frontend = JSONField( null=True, blank=True, @@ -93,13 +95,15 @@ def insert_or_update_error( context_frontend=None, context_backend=None, ): - if not getattr(settings, "DEVELOPER_MODE", None): + if getattr(settings, "DEVELOPER_MODE", None): error, created = cls.objects.get_or_create( category=category, error_message=error_message, traceback=traceback, context_frontend=context_frontend, context_backend=context_backend, + release_version=".".join(map(str, VERSION[:2])), + installation_type=installation_type(), ) if not created: error.events += 1 diff --git a/kolibri/core/errorreports/schemas.py b/kolibri/core/errorreports/schemas.py index 84822e0abd6..09ed382deae 100644 --- a/kolibri/core/errorreports/schemas.py +++ b/kolibri/core/errorreports/schemas.py @@ -1,16 +1,23 @@ context_frontend_schema = { "type": "object", "properties": { - "browser": {"type": "string"}, + "browser": { + "type": "object", + "properties": { + "name": {"type": "string", "optional": True}, + "major": {"type": "string", "optional": True}, + "minor": {"type": "string", "optional": True}, + "patch": {"type": "string", "optional": True}, + }, + }, "component": {"type": "string", "optional": True}, "device": { "type": "object", "properties": { - "type": {"type": "string"}, # "desktop", "tablet", "mobile" - "platform": { - "type": "string", - "optional": True, - }, # "windows", "mac", "linux", "android", "ios" + "model": {"type": "string", "optional": True}, + "type": {"type": "string", "optional": True}, + "vendor": {"type": "string", "optional": True}, + "is_touch_device": {"type": "boolean", "optional": True}, "screen": { "type": "object", "properties": { @@ -22,7 +29,7 @@ }, }, } -default_context_frontend_schema = {"browser": "", "component": "N/A", "device": {}} +default_context_frontend_schema = {"browser": {}, "component": "", "device": {}} context_backend_schema = { "type": "object", "properties": { From 01474a27e8f563b67b552b4a99dd83f673a187b5 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Wed, 3 Jul 2024 23:19:12 +0530 Subject: [PATCH 46/74] add installation_type in tasks --- kolibri/core/errorreports/tasks.py | 1 + 1 file changed, 1 insertion(+) diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py index 47e386f77ca..68376fe40cd 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/errorreports/tasks.py @@ -27,6 +27,7 @@ def serialize_error_reports_to_json_response(errors): "last_occurred": error.last_occurred, "events": error.events, "release_version": error.release_version, + "installation_type": error.installation_type, "context_frontend": error.context_frontend, "context_backend": error.context_backend, } From d530b3a4d2abf562ddde55da9dae27503ae52b2d Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Wed, 3 Jul 2024 23:20:47 +0530 Subject: [PATCH 47/74] use ua-parser for the device and os --- .../__tests__/errorReport.test.js | 13 ++----- kolibri/core/assets/src/utils/browserInfo.js | 10 +++++ .../core/assets/src/utils/errorReportUtils.js | 38 +++++++++---------- kolibri/core/errorreports/test/test_api.py | 17 ++++++--- 4 files changed, 42 insertions(+), 36 deletions(-) diff --git a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js b/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js index c699e7ce3fd..8c91dcd35fb 100644 --- a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js +++ b/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js @@ -30,9 +30,8 @@ describe('Error Report', () => { error_message: 'Vue error', traceback: 'My stack trace', context: { + ...errorReport.getContext(), component: 'TestComponent', - browser: errorReport.getBrowserInfo(), - device: errorReport.getDeviceInfo(), }, }; @@ -57,10 +56,7 @@ describe('Error Report', () => { const expectedData = { error_message: 'Javascript error', traceback: 'My stack trace', - context: { - browser: errorReport.getBrowserInfo(), - device: errorReport.getDeviceInfo(), - }, + context: errorReport.getContext(), }; Resource.client = jest.fn(); @@ -84,10 +80,7 @@ describe('Error Report', () => { const expectedData = { error_message: 'Unhandled rejection', traceback: 'My stack trace', - context: { - browser: errorReport.getBrowserInfo(), - device: errorReport.getDeviceInfo(), - }, + context: errorReport.getContext(), }; Resource.client = jest.fn(); diff --git a/kolibri/core/assets/src/utils/browserInfo.js b/kolibri/core/assets/src/utils/browserInfo.js index 516024db0fe..34bf0761906 100644 --- a/kolibri/core/assets/src/utils/browserInfo.js +++ b/kolibri/core/assets/src/utils/browserInfo.js @@ -70,6 +70,16 @@ export const isTouchDevice = window.navigator?.maxTouchPoints > 0 || window.navigator?.msMaxTouchPoints > 0; +const device = info.device; + +// this is better than undefined +device.type = device.type || 'unknown'; +device.model = device.model || 'unknown'; +device.vendor = device.vendor || 'unknown'; +export const deviceWithTouch = { + ...device, + isTouchDevice, +}; function handlePointerDown(event) { if (event.pointerType === 'mouse') { localStorage.setItem('mouseUsed', 'true'); diff --git a/kolibri/core/assets/src/utils/errorReportUtils.js b/kolibri/core/assets/src/utils/errorReportUtils.js index 6b018e6ac54..7279577ca4e 100644 --- a/kolibri/core/assets/src/utils/errorReportUtils.js +++ b/kolibri/core/assets/src/utils/errorReportUtils.js @@ -1,3 +1,5 @@ +import { browser, os, deviceWithTouch } from './browserInfo'; + class ErrorReport { constructor(e) { this.e = e; @@ -6,19 +8,20 @@ class ErrorReport { getErrorReport() { throw new Error('getErrorReport() method must be implemented.'); } - getDeviceInfo() { + + getContext() { return { - type: /Mobi|Android/i.test(navigator.userAgent) ? 'Mobile' : 'Desktop', - platform: navigator.platform, - screen: { - width: window.innerWidth, - height: window.innerHeight, + browser: browser, + os: os, + device: { + ...deviceWithTouch, + screen: { + width: window.innerWidth, + height: window.innerHeight, + }, }, }; } - getBrowserInfo() { - return navigator.userAgent; - } } export class VueErrorReport extends ErrorReport { @@ -31,9 +34,8 @@ export class VueErrorReport extends ErrorReport { error_message: this.e.message, traceback: this.e.stack, context: { + ...this.getContext(), component: this.vm.$options.name || this.vm.$options._componentTag || 'Unknown Component', - browser: this.getBrowserInfo(), - device: this.getDeviceInfo(), }, }; } @@ -44,10 +46,7 @@ export class JavascriptErrorReport extends ErrorReport { return { error_message: this.e.error.message, traceback: this.e.error.stack, - context: { - browser: this.getBrowserInfo(), - device: this.getDeviceInfo(), - }, + context: this.getContext(), }; } } @@ -55,12 +54,9 @@ export class JavascriptErrorReport extends ErrorReport { export class UnhandledRejectionErrorReport extends ErrorReport { getErrorReport() { return { - error_message: this.e.reason ? this.e.reason.message : 'Unhandled Rejection', - traceback: this.e.reason ? this.e.reason.stack : 'No stack trace available', - context: { - browser: this.getBrowserInfo(), - device: this.getDeviceInfo(), - }, + error_message: this.e.reason.message, + traceback: this.e.reason.stack, + context: this.getContext(), }; } } diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py index 8e9422d7abf..605b6ba46ad 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/core/errorreports/test/test_api.py @@ -15,12 +15,19 @@ class FrontendReportTestCase(TestCase): "error_message": "Something went wrong", "traceback": "Traceback information", "context": { - "browser": "", - "component": "my component", + "browser": { + "name": "Chrome", + "major": "1", + "minor": "2", + "patch": "3", + }, + "component": "component", "device": { - "type": "desktop", - "platform": "Windows", - "screen": {"width": 303, "height": 230}, + "model": "", + "type": "type", + "vendor": "vendor", + "is_touch_device": True, + "screen": {"width": 100, "height": 200}, }, }, } From 7056944849d15fd1405d8f2ef89e3f2b78e1da74 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Wed, 3 Jul 2024 23:28:26 +0530 Subject: [PATCH 48/74] add os in context_frontend --- .../errorreports/migrations/0002_add_more_fields.py | 11 ++++++++++- kolibri/core/errorreports/schemas.py | 9 +++++++++ kolibri/core/errorreports/test/test_api.py | 6 ++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/kolibri/core/errorreports/migrations/0002_add_more_fields.py b/kolibri/core/errorreports/migrations/0002_add_more_fields.py index bed77f59116..6bdbe08268f 100644 --- a/kolibri/core/errorreports/migrations/0002_add_more_fields.py +++ b/kolibri/core/errorreports/migrations/0002_add_more_fields.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-07-03 17:31 +# Generated by Django 3.2.25 on 2024-07-03 17:57 from django.db import migrations from django.db import models @@ -109,6 +109,15 @@ class Migration(migrations.Migration): }, "type": "object", }, + "os": { + "properties": { + "major": {"optional": True, "type": "string"}, + "minor": {"optional": True, "type": "string"}, + "name": {"optional": True, "type": "string"}, + "patch": {"optional": True, "type": "string"}, + }, + "type": "object", + }, }, "type": "object", } diff --git a/kolibri/core/errorreports/schemas.py b/kolibri/core/errorreports/schemas.py index 09ed382deae..ff63a11f56c 100644 --- a/kolibri/core/errorreports/schemas.py +++ b/kolibri/core/errorreports/schemas.py @@ -11,6 +11,15 @@ }, }, "component": {"type": "string", "optional": True}, + "os": { + "type": "object", + "properties": { + "name": {"type": "string", "optional": True}, + "major": {"type": "string", "optional": True}, + "minor": {"type": "string", "optional": True}, + "patch": {"type": "string", "optional": True}, + }, + }, "device": { "type": "object", "properties": { diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py index 605b6ba46ad..59709c3b323 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/core/errorreports/test/test_api.py @@ -21,6 +21,12 @@ class FrontendReportTestCase(TestCase): "minor": "2", "patch": "3", }, + "os": { + "name": "OS", + "major": "1", + "minor": "2", + "patch": "3", + }, "component": "component", "device": { "model": "", From 109acd2b86bf6860bca5616881a49852b2353945 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Wed, 3 Jul 2024 23:29:17 +0530 Subject: [PATCH 49/74] revert --- kolibri/core/errorreports/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index 0a824c14728..c8ad3e17a95 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -95,7 +95,7 @@ def insert_or_update_error( context_frontend=None, context_backend=None, ): - if getattr(settings, "DEVELOPER_MODE", None): + if not getattr(settings, "DEVELOPER_MODE", None): error, created = cls.objects.get_or_create( category=category, error_message=error_message, From cd373c4088e73c0dc03505192991694edcc09996 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Thu, 11 Jul 2024 17:37:24 +0530 Subject: [PATCH 50/74] clarity --- kolibri/core/errorreports/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py index 68376fe40cd..1408acd5a32 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/errorreports/tasks.py @@ -43,7 +43,7 @@ def ping_error_reports(server): errors_json = serialize_error_reports_to_json_response(errors) requests.post( - join_url(server, "/api/v1/errors/"), + join_url(server, "/api/v1/errors/report/"), data=errors_json, headers={"Content-Type": "application/json"}, ) From a558f37539010738ceb6cdacc6af5a43271fa09d Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Thu, 11 Jul 2024 17:43:50 +0530 Subject: [PATCH 51/74] format python version --- kolibri/core/errorreports/middleware.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index bcaf1c1ea3b..87fd1562bad 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -1,6 +1,6 @@ import logging import traceback -from sys import version +from sys import version_info import pkg_resources from django.db import IntegrityError @@ -27,7 +27,9 @@ def get_packages(): def get_python_version(): - return version.split()[0] + return "{major}.{minor}.{micro}".format( + major=version_info.major, minor=version_info.minor, micro=version_info.micro + ) class ErrorReportingMiddleware: From 6c4ce3a490d3f7a95bc6a299d9588dab24eed3f3 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 15 Jul 2024 13:15:59 +0530 Subject: [PATCH 52/74] use definations for schema --- .../migrations/0002_add_more_fields.py | 43 ++++++++++++------- kolibri/core/errorreports/schemas.py | 43 +++++++++++++------ 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/kolibri/core/errorreports/migrations/0002_add_more_fields.py b/kolibri/core/errorreports/migrations/0002_add_more_fields.py index 6bdbe08268f..5de1b8b2ccf 100644 --- a/kolibri/core/errorreports/migrations/0002_add_more_fields.py +++ b/kolibri/core/errorreports/migrations/0002_add_more_fields.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-07-03 17:57 +# Generated by Django 3.2.25 on 2024-07-15 07:30 from django.db import migrations from django.db import models @@ -36,8 +36,13 @@ class Migration(migrations.Migration): default={ "packages": {}, "python_version": "", - "request_info": {}, - "server": {}, + "request_info": { + "body": "", + "headers": {}, + "method": "", + "url": "", + }, + "server": {"host": "", "port": 0}, }, null=True, validators=[ @@ -74,13 +79,24 @@ class Migration(migrations.Migration): name="context_frontend", field=kolibri.core.fields.JSONField( blank=True, - default={"browser": {}, "component": "", "device": {}}, + default={ + "browser": {"major": "", "minor": "", "name": "", "patch": ""}, + "component": "", + "device": { + "is_touch_device": False, + "model": "", + "screen": {"height": 0, "width": 0}, + "type": "", + "vendor": "", + }, + "os": {"major": "", "minor": "", "name": "", "patch": ""}, + }, null=True, validators=[ kolibri.core.utils.validators.JSON_Schema_Validator( { - "properties": { - "browser": { + "definitions": { + "versionInfo": { "properties": { "major": {"optional": True, "type": "string"}, "minor": {"optional": True, "type": "string"}, @@ -88,7 +104,10 @@ class Migration(migrations.Migration): "patch": {"optional": True, "type": "string"}, }, "type": "object", - }, + } + }, + "properties": { + "browser": {"$ref": "#/definitions/versionInfo"}, "component": {"optional": True, "type": "string"}, "device": { "properties": { @@ -109,15 +128,7 @@ class Migration(migrations.Migration): }, "type": "object", }, - "os": { - "properties": { - "major": {"optional": True, "type": "string"}, - "minor": {"optional": True, "type": "string"}, - "name": {"optional": True, "type": "string"}, - "patch": {"optional": True, "type": "string"}, - }, - "type": "object", - }, + "os": {"$ref": "#/definitions/versionInfo"}, }, "type": "object", } diff --git a/kolibri/core/errorreports/schemas.py b/kolibri/core/errorreports/schemas.py index ff63a11f56c..691a67ce272 100644 --- a/kolibri/core/errorreports/schemas.py +++ b/kolibri/core/errorreports/schemas.py @@ -1,7 +1,7 @@ context_frontend_schema = { "type": "object", - "properties": { - "browser": { + "definitions": { + "versionInfo": { "type": "object", "properties": { "name": {"type": "string", "optional": True}, @@ -9,16 +9,15 @@ "minor": {"type": "string", "optional": True}, "patch": {"type": "string", "optional": True}, }, + } + }, + "properties": { + "browser": { + "$ref": "#/definitions/versionInfo", }, "component": {"type": "string", "optional": True}, "os": { - "type": "object", - "properties": { - "name": {"type": "string", "optional": True}, - "major": {"type": "string", "optional": True}, - "minor": {"type": "string", "optional": True}, - "patch": {"type": "string", "optional": True}, - }, + "$ref": "#/definitions/versionInfo", }, "device": { "type": "object", @@ -38,7 +37,22 @@ }, }, } -default_context_frontend_schema = {"browser": {}, "component": "", "device": {}} +default_version_info = {"name": "", "major": "", "minor": "", "patch": ""} +default_context_frontend_schema = { + "browser": default_version_info, + "component": "", + "os": default_version_info, + "device": { + "model": "", + "type": "", + "vendor": "", + "is_touch_device": False, + "screen": { + "width": 0, + "height": 0, + }, + }, +} context_backend_schema = { "type": "object", "properties": { @@ -63,8 +77,13 @@ }, } default_context_backend_schema = { - "request_info": {}, - "server": {}, + "request_info": { + "url": "", + "method": "", + "headers": {}, + "body": "", + }, + "server": {"host": "", "port": 0}, "packages": {}, "python_version": "", } From 1cc20dc948aa5300fe2f218427bd8a7b6736a791 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 15 Jul 2024 13:17:00 +0530 Subject: [PATCH 53/74] seeprate device and isTouchDevice --- kolibri/core/assets/src/utils/browserInfo.js | 17 +++++++---------- .../core/assets/src/utils/errorReportUtils.js | 5 +++-- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/kolibri/core/assets/src/utils/browserInfo.js b/kolibri/core/assets/src/utils/browserInfo.js index 34bf0761906..52bc9d5bfd0 100644 --- a/kolibri/core/assets/src/utils/browserInfo.js +++ b/kolibri/core/assets/src/utils/browserInfo.js @@ -64,22 +64,19 @@ export const os = { patch: osVersion[2], }; +// Device info +export const device = { + type: info.device.type || 'desktop', + model: info.device.model, + vendor: info.device.vendor, +}; + // Check for presence of the touch event in DOM or multi-touch capabilities export const isTouchDevice = 'ontouchstart' in window || window.navigator?.maxTouchPoints > 0 || window.navigator?.msMaxTouchPoints > 0; -const device = info.device; - -// this is better than undefined -device.type = device.type || 'unknown'; -device.model = device.model || 'unknown'; -device.vendor = device.vendor || 'unknown'; -export const deviceWithTouch = { - ...device, - isTouchDevice, -}; function handlePointerDown(event) { if (event.pointerType === 'mouse') { localStorage.setItem('mouseUsed', 'true'); diff --git a/kolibri/core/assets/src/utils/errorReportUtils.js b/kolibri/core/assets/src/utils/errorReportUtils.js index 7279577ca4e..9c8bc157f8b 100644 --- a/kolibri/core/assets/src/utils/errorReportUtils.js +++ b/kolibri/core/assets/src/utils/errorReportUtils.js @@ -1,4 +1,4 @@ -import { browser, os, deviceWithTouch } from './browserInfo'; +import { browser, os, device, isTouchDevice } from './browserInfo'; class ErrorReport { constructor(e) { @@ -14,7 +14,8 @@ class ErrorReport { browser: browser, os: os, device: { - ...deviceWithTouch, + ...device, + is_touch_device: isTouchDevice, screen: { width: window.innerWidth, height: window.innerHeight, From a7414c8f0af7d6bf68b2c387390abcbac8945326 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Fri, 19 Jul 2024 01:09:39 +0530 Subject: [PATCH 54/74] change screen object and move getContext to parent method --- kolibri/core/assets/src/utils/errorReportUtils.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/kolibri/core/assets/src/utils/errorReportUtils.js b/kolibri/core/assets/src/utils/errorReportUtils.js index 9c8bc157f8b..5b47b817b67 100644 --- a/kolibri/core/assets/src/utils/errorReportUtils.js +++ b/kolibri/core/assets/src/utils/errorReportUtils.js @@ -3,6 +3,7 @@ import { browser, os, device, isTouchDevice } from './browserInfo'; class ErrorReport { constructor(e) { this.e = e; + this.context = this.getContext(); } getErrorReport() { @@ -17,8 +18,10 @@ class ErrorReport { ...device, is_touch_device: isTouchDevice, screen: { - width: window.innerWidth, - height: window.innerHeight, + width: window.screen.width, + height: window.screen.height, + available_width: window.screen.availWidth, + available_height: window.screen.availHeight, }, }, }; @@ -35,7 +38,7 @@ export class VueErrorReport extends ErrorReport { error_message: this.e.message, traceback: this.e.stack, context: { - ...this.getContext(), + ...this.context, component: this.vm.$options.name || this.vm.$options._componentTag || 'Unknown Component', }, }; @@ -47,7 +50,7 @@ export class JavascriptErrorReport extends ErrorReport { return { error_message: this.e.error.message, traceback: this.e.error.stack, - context: this.getContext(), + context: this.context, }; } } @@ -57,7 +60,7 @@ export class UnhandledRejectionErrorReport extends ErrorReport { return { error_message: this.e.reason.message, traceback: this.e.reason.stack, - context: this.getContext(), + context: this.context, }; } } From dea1d32c5389f708281fa388fa404517fd2acaac Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Fri, 19 Jul 2024 01:13:08 +0530 Subject: [PATCH 55/74] changes: single context instead of two, full version instead of parsed, remove mark_as_reported --- .../migrations/0002_add_more_fields.py | 114 +----------------- kolibri/core/errorreports/models.py | 46 +++---- kolibri/core/errorreports/test/test_models.py | 49 +++----- 3 files changed, 38 insertions(+), 171 deletions(-) diff --git a/kolibri/core/errorreports/migrations/0002_add_more_fields.py b/kolibri/core/errorreports/migrations/0002_add_more_fields.py index 5de1b8b2ccf..173dcd1701f 100644 --- a/kolibri/core/errorreports/migrations/0002_add_more_fields.py +++ b/kolibri/core/errorreports/migrations/0002_add_more_fields.py @@ -1,9 +1,8 @@ -# Generated by Django 3.2.25 on 2024-07-15 07:30 +# Generated by Django 3.2.25 on 2024-07-18 19:40 from django.db import migrations from django.db import models import kolibri.core.fields -import kolibri.core.utils.validators class Migration(migrations.Migration): @@ -30,111 +29,8 @@ class Migration(migrations.Migration): ), migrations.AddField( model_name="errorreports", - name="context_backend", - field=kolibri.core.fields.JSONField( - blank=True, - default={ - "packages": {}, - "python_version": "", - "request_info": { - "body": "", - "headers": {}, - "method": "", - "url": "", - }, - "server": {"host": "", "port": 0}, - }, - null=True, - validators=[ - kolibri.core.utils.validators.JSON_Schema_Validator( - { - "properties": { - "packages": {"optional": True, "type": "object"}, - "python_version": {"optional": True, "type": "string"}, - "request_info": { - "properties": { - "body": {"optional": True, "type": "string"}, - "headers": {"optional": True, "type": "object"}, - "method": {"optional": True, "type": "string"}, - "url": {"optional": True, "type": "string"}, - }, - "type": "object", - }, - "server": { - "properties": { - "host": {"optional": True, "type": "string"}, - "port": {"optional": True, "type": "integer"}, - }, - "type": "object", - }, - }, - "type": "object", - } - ) - ], - ), - ), - migrations.AddField( - model_name="errorreports", - name="context_frontend", - field=kolibri.core.fields.JSONField( - blank=True, - default={ - "browser": {"major": "", "minor": "", "name": "", "patch": ""}, - "component": "", - "device": { - "is_touch_device": False, - "model": "", - "screen": {"height": 0, "width": 0}, - "type": "", - "vendor": "", - }, - "os": {"major": "", "minor": "", "name": "", "patch": ""}, - }, - null=True, - validators=[ - kolibri.core.utils.validators.JSON_Schema_Validator( - { - "definitions": { - "versionInfo": { - "properties": { - "major": {"optional": True, "type": "string"}, - "minor": {"optional": True, "type": "string"}, - "name": {"optional": True, "type": "string"}, - "patch": {"optional": True, "type": "string"}, - }, - "type": "object", - } - }, - "properties": { - "browser": {"$ref": "#/definitions/versionInfo"}, - "component": {"optional": True, "type": "string"}, - "device": { - "properties": { - "is_touch_device": { - "optional": True, - "type": "boolean", - }, - "model": {"optional": True, "type": "string"}, - "screen": { - "properties": { - "height": {"type": "integer"}, - "width": {"type": "integer"}, - }, - "type": "object", - }, - "type": {"optional": True, "type": "string"}, - "vendor": {"optional": True, "type": "string"}, - }, - "type": "object", - }, - "os": {"$ref": "#/definitions/versionInfo"}, - }, - "type": "object", - } - ) - ], - ), + name="context", + field=kolibri.core.fields.JSONField(blank=True, null=True), ), migrations.AddField( model_name="errorreports", @@ -144,6 +40,8 @@ class Migration(migrations.Migration): migrations.AddField( model_name="errorreports", name="release_version", - field=models.CharField(default="0.16", max_length=64), + field=models.CharField( + blank=True, default="0.16.2b2.dev0+git.1204.g57aa39d3", max_length=128 + ), ), ] diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index c8ad3e17a95..6806bd79788 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -4,12 +4,12 @@ from django.db import models from django.utils import timezone +from .constants import BACKEND +from .constants import FRONTEND from .constants import POSSIBLE_ERRORS from .schemas import context_backend_schema from .schemas import context_frontend_schema -from .schemas import default_context_backend_schema -from .schemas import default_context_frontend_schema -from kolibri import VERSION +from kolibri import __version__ from kolibri.core.fields import JSONField from kolibri.core.utils.validators import JSON_Schema_Validator from kolibri.deployment.default.sqlite_db_names import ERROR_REPORTS @@ -62,47 +62,35 @@ class ErrorReports(models.Model): last_occurred = models.DateTimeField(default=timezone.now) reported = models.BooleanField(default=False) events = models.IntegerField(default=1) - release_version = models.CharField( - max_length=64, default=".".join(map(str, VERSION[:2])) - ) + release_version = models.CharField(max_length=128, default=__version__, blank=True) installation_type = models.CharField(max_length=64, blank=True) - context_frontend = JSONField( + context = JSONField( null=True, blank=True, - validators=[JSON_Schema_Validator(context_frontend_schema)], - default=default_context_frontend_schema, - ) - context_backend = JSONField( - null=True, - blank=True, - validators=[JSON_Schema_Validator(context_backend_schema)], - default=default_context_backend_schema, ) def __str__(self): return f"{self.error_message} ({self.category})" - def mark_reported(self): - self.reported = True - self.save() + def clean(self): + if self.category == FRONTEND: + JSON_Schema_Validator(context_frontend_schema)(self.context) + elif self.category == BACKEND: + JSON_Schema_Validator(context_backend_schema)(self.context) + + def save(self, *args, **kwargs): + self.full_clean() + super().save(*args, **kwargs) @classmethod - def insert_or_update_error( - cls, - category, - error_message, - traceback, - context_frontend=None, - context_backend=None, - ): + def insert_or_update_error(cls, category, error_message, traceback, context): if not getattr(settings, "DEVELOPER_MODE", None): error, created = cls.objects.get_or_create( category=category, error_message=error_message, traceback=traceback, - context_frontend=context_frontend, - context_backend=context_backend, - release_version=".".join(map(str, VERSION[:2])), + context=context, + release_version=__version__, installation_type=installation_type(), ) if not created: diff --git a/kolibri/core/errorreports/test/test_models.py b/kolibri/core/errorreports/test/test_models.py index a823e3e8431..3ae8d3a09b5 100644 --- a/kolibri/core/errorreports/test/test_models.py +++ b/kolibri/core/errorreports/test/test_models.py @@ -16,12 +16,17 @@ def setUp(self): self.error_message = "Test Error" self.traceback = "Test Traceback" self.context_frontend = { - "browser": "Chrome", + "browser": {}, + "os": {}, "component": "HeaderComponent", "device": { - "type": "desktop", - "platform": "windows", - "screen": {"width": 1920, "height": 1080}, + "is_touch_device": True, + "screen": { + "width": 1920, + "height": 1080, + "available_width": 1920, + "available_height": 1040, + }, }, } self.context_backend = { @@ -31,7 +36,7 @@ def setUp(self): "headers": {"User-Agent": "TestAgent"}, "body": "", }, - "server": {"host": "localhost", "port": 8000}, + "server": {"host": "localhost", "port": "8000"}, "packages": {"django": "3.2", "kolibri": "0.15.8"}, "python_version": "3.9.1", } @@ -41,16 +46,14 @@ def create_error( category, error_message, traceback, - context_frontend, - context_backend, + context, reported=False, ): return ErrorReports.objects.create( category=category, error_message=error_message, traceback=traceback, - context_frontend=context_frontend, - context_backend=context_backend, + context=context, reported=reported, ) @@ -61,13 +64,11 @@ def test_insert_or_update_error_prod_mode(self): self.error_message, self.traceback, self.context_frontend, - self.context_backend, ) self.assertEqual(error.category, self.category_frontend) self.assertEqual(error.error_message, self.error_message) self.assertEqual(error.traceback, self.traceback) - self.assertEqual(error.context_frontend, self.context_frontend) - self.assertEqual(error.context_backend, self.context_backend) + self.assertEqual(error.context, self.context_frontend) self.assertEqual(error.events, 1) self.assertFalse(error.reported) self.assertLess( @@ -83,13 +84,11 @@ def test_insert_or_update_error_prod_mode(self): self.error_message, self.traceback, self.context_frontend, - self.context_backend, ) self.assertEqual(error.category, self.category_frontend) self.assertEqual(error.error_message, self.error_message) self.assertEqual(error.traceback, self.traceback) - self.assertEqual(error.context_frontend, self.context_frontend) - self.assertEqual(error.context_backend, self.context_backend) + self.assertEqual(error.context, self.context_frontend) self.assertEqual(error.events, 2) self.assertFalse(error.reported) self.assertLess( @@ -102,10 +101,9 @@ def test_insert_or_update_error_prod_mode(self): @override_settings(DEVELOPER_MODE=True) def test_insert_or_update_error_dev_mode(self): error = ErrorReports.insert_or_update_error( - self.category_frontend, + self.category_backend, self.error_message, self.traceback, - self.context_frontend, self.context_backend, ) self.assertIsNone(error) @@ -116,14 +114,12 @@ def test_get_unreported_errors(self): "Error 1", "Traceback 1", self.context_frontend, - self.context_backend, reported=False, ) self.create_error( self.category_backend, "Error 2", "Traceback 2", - self.context_frontend, self.context_backend, reported=False, ) @@ -131,7 +127,6 @@ def test_get_unreported_errors(self): self.category_backend, "Error 3", "Traceback 3", - self.context_frontend, self.context_backend, reported=True, ) @@ -141,17 +136,3 @@ def test_get_unreported_errors(self): self.assertEqual(unreported_errors.count(), 2) self.assertFalse(unreported_errors[0].reported) self.assertFalse(unreported_errors[1].reported) - - def test_mark_reported(self): - error = self.create_error( - self.category_frontend, - self.error_message, - self.traceback, - self.context_frontend, - self.context_backend, - reported=False, - ) - # First check error is unreported, then set to True and assert again - self.assertFalse(error.reported) - error.mark_reported() - self.assertTrue(error.reported) From 86159d08537254451fc7e6f5fcf112affc2185af Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Fri, 19 Jul 2024 01:14:34 +0530 Subject: [PATCH 56/74] changes: importlib instead of pkg_resources and pass context to the save method --- kolibri/core/errorreports/middleware.py | 14 +++++++++----- kolibri/core/errorreports/test/test_middleware.py | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index 87fd1562bad..5fe5e2dba6e 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -1,8 +1,9 @@ +import importlib.metadata import logging import traceback from sys import version_info -import pkg_resources +from django.core.exceptions import ValidationError from django.db import IntegrityError from .constants import BACKEND @@ -23,7 +24,10 @@ def get_server_info(request): def get_packages(): - return {dist.project_name: dist.version for dist in pkg_resources.working_set} + return { + dist.metadata["Name"]: dist.version + for dist in importlib.metadata.distributions() + } def get_python_version(): @@ -54,15 +58,15 @@ def process_exception(self, request, exception): "packages": get_packages(), "python_version": get_python_version(), } - self.logger.error("Unexpected Error: %s", error_message) + self.logger.error("Unexpected Error: %s", error_message) try: self.logger.error("Saving error report to the database.") ErrorReports.insert_or_update_error( - BACKEND, error_message, traceback_info, context_backend=context + BACKEND, error_message, traceback_info, context ) self.logger.info("Error report saved to the database.") - except IntegrityError as e: + except (IntegrityError, ValidationError) as e: self.logger.error( "Error occurred while saving error report to the database: %s", str(e) ) diff --git a/kolibri/core/errorreports/test/test_middleware.py b/kolibri/core/errorreports/test/test_middleware.py index d1761a5b83d..927983c808b 100644 --- a/kolibri/core/errorreports/test/test_middleware.py +++ b/kolibri/core/errorreports/test/test_middleware.py @@ -49,7 +49,7 @@ def test_process_exception( BACKEND, str(exception), expected_traceback_info, - context_backend={ + { "request_info": { "url": "http://testserver/", "method": "GET", From 22cfce2ead8eedbd8968c0b63f30150374b6fb61 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Fri, 19 Jul 2024 01:24:30 +0530 Subject: [PATCH 57/74] changes: modelserializer instead of regular, pass context to the save method --- kolibri/core/errorreports/api.py | 26 +++++----------------- kolibri/core/errorreports/serializers.py | 15 ++++++------- kolibri/core/errorreports/test/test_api.py | 7 +++++- 3 files changed, 18 insertions(+), 30 deletions(-) diff --git a/kolibri/core/errorreports/api.py b/kolibri/core/errorreports/api.py index 52d4f312059..7a3be6ea517 100644 --- a/kolibri/core/errorreports/api.py +++ b/kolibri/core/errorreports/api.py @@ -1,5 +1,6 @@ import logging +from django.core.exceptions import ValidationError from rest_framework import status from rest_framework.decorators import api_view from rest_framework.response import Response @@ -8,29 +9,12 @@ from .models import ErrorReports from .serializers import ErrorReportSerializer + logger = logging.getLogger(__name__) @api_view(["POST"]) def report(request): - """ - test with: - curl -X POST http://localhost:8000/api/errorreports/report/ -H "Content-Type: application/json" -d '{ - "error_message": "An example error occurred", - "traceback": "Traceback (most recent call last): ..." - "context": { - "browser":"", - "component":"my component", - "device":{ - "type": "desktop", - "platform":"Windows", - "screen": { - "width":545, - "height":858 - } - } - }' - """ serializer = ErrorReportSerializer(data=request.data) if serializer.is_valid(): data = serializer.validated_data @@ -39,16 +23,16 @@ def report(request): FRONTEND, data["error_message"], data["traceback"], - context_frontend=data["context"], + context=data["context"], ) return Response( {"error_id": error.id if error else None}, status=status.HTTP_200_OK ) - except (AttributeError, Exception) as e: + except (AttributeError, ValidationError, Exception) as e: logger.error("Error while saving error report: {}".format(e)) return Response( - {"error": "Error while saving error report"}, + {"error": "Error while saving error report: {}".format(e)}, status=status.HTTP_500_INTERNAL_SERVER_ERROR, ) else: diff --git a/kolibri/core/errorreports/serializers.py b/kolibri/core/errorreports/serializers.py index b682b8eebeb..765653e39b2 100644 --- a/kolibri/core/errorreports/serializers.py +++ b/kolibri/core/errorreports/serializers.py @@ -1,12 +1,11 @@ from rest_framework import serializers -from .schemas import context_frontend_schema -from kolibri.core.utils.validators import JSON_Schema_Validator +from .models import ErrorReports -class ErrorReportSerializer(serializers.Serializer): - error_message = serializers.CharField(max_length=255) - traceback = serializers.CharField() - context = serializers.JSONField( - validators=[JSON_Schema_Validator(context_frontend_schema)] - ) +class ErrorReportSerializer(serializers.ModelSerializer): + context = serializers.JSONField() + + class Meta: + model = ErrorReports + fields = ["error_message", "traceback", "context"] diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py index 59709c3b323..b6b91065488 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/core/errorreports/test/test_api.py @@ -33,7 +33,12 @@ class FrontendReportTestCase(TestCase): "type": "type", "vendor": "vendor", "is_touch_device": True, - "screen": {"width": 100, "height": 200}, + "screen": { + "width": 100, + "height": 200, + "available_width": 100, + "available_height": 200, + }, }, }, } From 15e34ba599a20bd995ed979222c468b13c94b54c Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Fri, 19 Jul 2024 01:25:07 +0530 Subject: [PATCH 58/74] use single context --- kolibri/core/errorreports/tasks.py | 3 +-- kolibri/core/errorreports/test/test_tasks.py | 21 ++++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py index 1408acd5a32..6827d77f412 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/errorreports/tasks.py @@ -28,8 +28,7 @@ def serialize_error_reports_to_json_response(errors): "events": error.events, "release_version": error.release_version, "installation_type": error.installation_type, - "context_frontend": error.context_frontend, - "context_backend": error.context_backend, + "context": error.context, } ) return json.dumps(errors_list, cls=DjangoJSONEncoder) diff --git a/kolibri/core/errorreports/test/test_tasks.py b/kolibri/core/errorreports/test/test_tasks.py index 6bcdbc394df..597e7005c38 100644 --- a/kolibri/core/errorreports/test/test_tasks.py +++ b/kolibri/core/errorreports/test/test_tasks.py @@ -18,13 +18,18 @@ def setUp(self): category="frontend", error_message="Test Error", traceback="Test Traceback", - context_frontend={ - "browser": "Chrome", + context={ + "browser": {}, + "os": {}, "component": "HeaderComponent", "device": { - "type": "desktop", - "platform": "windows", - "screen": {"width": 1920, "height": 1080}, + "is_touch_device": True, + "screen": { + "width": 1920, + "height": 1080, + "available_width": 1920, + "available_height": 1040, + }, }, }, ) @@ -32,14 +37,14 @@ def setUp(self): category="backend", error_message="Test Error", traceback="Test Traceback", - context_backend={ + context={ "request_info": { "url": "/api/test", "method": "GET", "headers": {"User-Agent": "TestAgent"}, "body": "", }, - "server": {"host": "localhost", "port": 8000}, + "server": {"host": "localhost", "port": "8000"}, "packages": {"django": "3.2", "kolibri": "0.15.8"}, "python_version": "3.9.1", }, @@ -53,7 +58,7 @@ def setUp(self): def test_ping_error_reports(self, mock_serializer, mock_post): ping_error_reports("http://testserver") mock_post.assert_called_with( - "http://testserver/api/v1/errors/", + "http://testserver/api/v1/errors/report/", data="[]", headers={"Content-Type": "application/json"}, ) From 85cf9c0a8e3b984a79694a764685b214569e9e7c Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Fri, 19 Jul 2024 01:25:45 +0530 Subject: [PATCH 59/74] add more screen info in schemas and remove default schemas --- kolibri/core/errorreports/schemas.py | 35 ++++------------------------ 1 file changed, 5 insertions(+), 30 deletions(-) diff --git a/kolibri/core/errorreports/schemas.py b/kolibri/core/errorreports/schemas.py index 691a67ce272..cdc163dc8ef 100644 --- a/kolibri/core/errorreports/schemas.py +++ b/kolibri/core/errorreports/schemas.py @@ -29,30 +29,16 @@ "screen": { "type": "object", "properties": { - "width": {"type": "integer"}, - "height": {"type": "integer"}, + "width": {"type": "integer", "optional": True}, + "height": {"type": "integer", "optional": True}, + "available_width": {"type": "integer", "optional": True}, + "available_height": {"type": "integer", "optional": True}, }, }, }, }, }, } -default_version_info = {"name": "", "major": "", "minor": "", "patch": ""} -default_context_frontend_schema = { - "browser": default_version_info, - "component": "", - "os": default_version_info, - "device": { - "model": "", - "type": "", - "vendor": "", - "is_touch_device": False, - "screen": { - "width": 0, - "height": 0, - }, - }, -} context_backend_schema = { "type": "object", "properties": { @@ -69,21 +55,10 @@ "type": "object", "properties": { "host": {"type": "string", "optional": True}, - "port": {"type": "integer", "optional": True}, + "port": {"type": "string", "optional": True}, }, }, "packages": {"type": "object", "optional": True}, "python_version": {"type": "string", "optional": True}, }, } -default_context_backend_schema = { - "request_info": { - "url": "", - "method": "", - "headers": {}, - "body": "", - }, - "server": {"host": "", "port": 0}, - "packages": {}, - "python_version": "", -} From c5eb2ae89abccda58e70350005caacc80d391ac3 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Fri, 26 Jul 2024 00:21:15 +0530 Subject: [PATCH 60/74] add query_params and improve packages retreival --- kolibri/core/errorreports/middleware.py | 13 ++++++++----- .../errorreports/migrations/0002_add_more_fields.py | 4 ++-- kolibri/core/errorreports/schemas.py | 3 ++- kolibri/core/errorreports/test/test_middleware.py | 5 +++-- kolibri/core/errorreports/test/test_models.py | 3 ++- kolibri/core/errorreports/test/test_tasks.py | 3 ++- 6 files changed, 19 insertions(+), 12 deletions(-) diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index 5fe5e2dba6e..9dc99871fc1 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -1,8 +1,12 @@ -import importlib.metadata import logging import traceback from sys import version_info +if version_info < (3, 10): + from importlib_metadata import distributions +else: + from importlib.metadata import distributions + from django.core.exceptions import ValidationError from django.db import IntegrityError @@ -16,6 +20,7 @@ def get_request_info(request): "method": request.method, "headers": dict(request.headers), "body": request.body.decode("utf-8"), + "query_params": dict(request.GET), } @@ -24,10 +29,8 @@ def get_server_info(request): def get_packages(): - return { - dist.metadata["Name"]: dist.version - for dist in importlib.metadata.distributions() - } + packages = [f"{dist.metadata['Name']}=={dist.version}" for dist in distributions()] + return packages def get_python_version(): diff --git a/kolibri/core/errorreports/migrations/0002_add_more_fields.py b/kolibri/core/errorreports/migrations/0002_add_more_fields.py index 173dcd1701f..8ffa8835b63 100644 --- a/kolibri/core/errorreports/migrations/0002_add_more_fields.py +++ b/kolibri/core/errorreports/migrations/0002_add_more_fields.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-07-18 19:40 +# Generated by Django 3.2.25 on 2024-07-25 18:19 from django.db import migrations from django.db import models @@ -41,7 +41,7 @@ class Migration(migrations.Migration): model_name="errorreports", name="release_version", field=models.CharField( - blank=True, default="0.16.2b2.dev0+git.1204.g57aa39d3", max_length=128 + blank=True, default="0.16.2b2.dev0+git.1209.g319b0288", max_length=128 ), ), ] diff --git a/kolibri/core/errorreports/schemas.py b/kolibri/core/errorreports/schemas.py index cdc163dc8ef..546d3bc01cd 100644 --- a/kolibri/core/errorreports/schemas.py +++ b/kolibri/core/errorreports/schemas.py @@ -49,6 +49,7 @@ "method": {"type": "string", "optional": True}, "headers": {"type": "object", "optional": True}, "body": {"type": "string", "optional": True}, + "query_params": {"type": "object", "optional": True}, }, }, "server": { @@ -58,7 +59,7 @@ "port": {"type": "string", "optional": True}, }, }, - "packages": {"type": "object", "optional": True}, + "packages": {"type": "array", "optional": True}, "python_version": {"type": "string", "optional": True}, }, } diff --git a/kolibri/core/errorreports/test/test_middleware.py b/kolibri/core/errorreports/test/test_middleware.py index 927983c808b..f63a954ee05 100644 --- a/kolibri/core/errorreports/test/test_middleware.py +++ b/kolibri/core/errorreports/test/test_middleware.py @@ -20,7 +20,7 @@ def setUp(self): ) @patch( "kolibri.core.errorreports.middleware.get_packages", - return_value={"Django": "3.2.25"}, + return_value=["Django==3.2.25"], ) @patch.object(ErrorReports, "insert_or_update_error") @patch.object(logging.Logger, "error") @@ -55,9 +55,10 @@ def test_process_exception( "method": "GET", "headers": dict(request.headers), "body": "", + "query_params": {}, }, "server": {"host": "testserver", "port": "80"}, - "packages": {"Django": "3.2.25"}, + "packages": ["Django==3.2.25"], "python_version": "3.9.9", }, ) diff --git a/kolibri/core/errorreports/test/test_models.py b/kolibri/core/errorreports/test/test_models.py index 3ae8d3a09b5..03c65910153 100644 --- a/kolibri/core/errorreports/test/test_models.py +++ b/kolibri/core/errorreports/test/test_models.py @@ -35,9 +35,10 @@ def setUp(self): "method": "GET", "headers": {"User-Agent": "TestAgent"}, "body": "", + "query_params": {"test": "true"}, }, "server": {"host": "localhost", "port": "8000"}, - "packages": {"django": "3.2", "kolibri": "0.15.8"}, + "packages": ["django==3.2", "kolibri==0.15.8"], "python_version": "3.9.1", } diff --git a/kolibri/core/errorreports/test/test_tasks.py b/kolibri/core/errorreports/test/test_tasks.py index 597e7005c38..edb8af77e78 100644 --- a/kolibri/core/errorreports/test/test_tasks.py +++ b/kolibri/core/errorreports/test/test_tasks.py @@ -43,9 +43,10 @@ def setUp(self): "method": "GET", "headers": {"User-Agent": "TestAgent"}, "body": "", + "query_params": {"test": "true"}, }, "server": {"host": "localhost", "port": "8000"}, - "packages": {"django": "3.2", "kolibri": "0.15.8"}, + "packages": ["Django==3.2.25"], "python_version": "3.9.1", }, ) From 14ceceba4fdfaffa70d09ba6f0e46a747e5dbec6 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Wed, 31 Jul 2024 22:04:56 +0530 Subject: [PATCH 61/74] Add pingback_id and request_time --- kolibri/core/analytics/tasks.py | 5 ++-- kolibri/core/analytics/utils.py | 1 + kolibri/core/errorreports/middleware.py | 25 +++++++++++++++++-- .../migrations/0002_add_more_fields.py | 9 +++++-- kolibri/core/errorreports/models.py | 13 +++++++++- kolibri/core/errorreports/tasks.py | 7 +++--- .../core/errorreports/test/test_middleware.py | 7 ++++++ kolibri/core/errorreports/test/test_tasks.py | 8 +++--- kolibri/deployment/default/settings/base.py | 1 + 9 files changed, 62 insertions(+), 14 deletions(-) diff --git a/kolibri/core/analytics/tasks.py b/kolibri/core/analytics/tasks.py index 19e71ce6cd6..aecc8d4a283 100644 --- a/kolibri/core/analytics/tasks.py +++ b/kolibri/core/analytics/tasks.py @@ -25,8 +25,9 @@ @register_task(job_id=DEFAULT_PING_JOB_ID) def _ping(started, server, checkrate): try: - ping_once(started, server=server) - ping_error_reports.enqueue(args=(server,)) + pingback_id = ping_once(started, server=server) + if pingback_id: + ping_error_reports.enqueue(args=(server, pingback_id)) except ConnectionError: logger.warning( "Ping failed (could not connect). Trying again in {} minutes.".format( diff --git a/kolibri/core/analytics/utils.py b/kolibri/core/analytics/utils.py index d08d5be7298..c5db074f617 100644 --- a/kolibri/core/analytics/utils.py +++ b/kolibri/core/analytics/utils.py @@ -471,3 +471,4 @@ def ping_once(started, server=DEFAULT_SERVER_URL): if "id" in data: stat_data = perform_statistics(server, data["id"]) create_and_update_notifications(stat_data, nutrition_endpoints.STATISTICS) + return data["id"] diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index 9dc99871fc1..73a5487a66a 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -1,4 +1,5 @@ import logging +import time import traceback from sys import version_info @@ -39,6 +40,10 @@ def get_python_version(): ) +def get_request_time_to_error(request): + return time.time() - request.start_time + + class ErrorReportingMiddleware: """ Middleware to log exceptions to the database. @@ -55,21 +60,37 @@ def __call__(self, request): def process_exception(self, request, exception): error_message = str(exception) traceback_info = traceback.format_exc() + request_time_to_error = get_request_time_to_error(request) context = { "request_info": get_request_info(request), "server": get_server_info(request), "packages": get_packages(), "python_version": get_python_version(), } - self.logger.error("Unexpected Error: %s", error_message) try: self.logger.error("Saving error report to the database.") ErrorReports.insert_or_update_error( - BACKEND, error_message, traceback_info, context + BACKEND, + error_message, + traceback_info, + context, + request_time_to_error=request_time_to_error, ) self.logger.info("Error report saved to the database.") except (IntegrityError, ValidationError) as e: self.logger.error( "Error occurred while saving error report to the database: %s", str(e) ) + + +class PreRequestMiddleware: + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + response = self.get_response(request) + return response + + def process_view(self, request, view_func, view_args, view_kwargs): + request.start_time = time.time() diff --git a/kolibri/core/errorreports/migrations/0002_add_more_fields.py b/kolibri/core/errorreports/migrations/0002_add_more_fields.py index 8ffa8835b63..f4d2157bd67 100644 --- a/kolibri/core/errorreports/migrations/0002_add_more_fields.py +++ b/kolibri/core/errorreports/migrations/0002_add_more_fields.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-07-25 18:19 +# Generated by Django 3.2.25 on 2024-07-31 16:20 from django.db import migrations from django.db import models @@ -27,6 +27,11 @@ class Migration(migrations.Migration): old_name="sent", new_name="reported", ), + migrations.AddField( + model_name="errorreports", + name="avg_request_time_to_error", + field=models.FloatField(blank=True, null=True), + ), migrations.AddField( model_name="errorreports", name="context", @@ -41,7 +46,7 @@ class Migration(migrations.Migration): model_name="errorreports", name="release_version", field=models.CharField( - blank=True, default="0.16.2b2.dev0+git.1209.g319b0288", max_length=128 + blank=True, default="0.16.2b2.dev0+git.1211.gc5eb2ae8", max_length=128 ), ), ] diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index 6806bd79788..d8349bfa11b 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -68,6 +68,7 @@ class ErrorReports(models.Model): null=True, blank=True, ) + avg_request_time_to_error = models.FloatField(null=True, blank=True) # in seconds def __str__(self): return f"{self.error_message} ({self.category})" @@ -83,7 +84,9 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) @classmethod - def insert_or_update_error(cls, category, error_message, traceback, context): + def insert_or_update_error( + cls, category, error_message, traceback, context, request_time_to_error=None + ): if not getattr(settings, "DEVELOPER_MODE", None): error, created = cls.objects.get_or_create( category=category, @@ -96,6 +99,14 @@ def insert_or_update_error(cls, category, error_message, traceback, context): if not created: error.events += 1 error.last_occurred = timezone.now() + if error.avg_request_time_to_error is not None: + # see the proof: https://math.stackexchange.com/a/106314 + error.avg_request_time_to_error = ( + error.avg_request_time_to_error * (error.events - 1) + + request_time_to_error + ) / error.events + else: + error.avg_request_time_to_error = request_time_to_error error.save() logger.error("ErrorReports: Database updated.") return error diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py index 6827d77f412..bd4f94e95d6 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/errorreports/tasks.py @@ -15,7 +15,7 @@ logger = logging.getLogger(__name__) -def serialize_error_reports_to_json_response(errors): +def serialize_error_reports_to_json_response(errors, pingback_id): errors_list = [] for error in errors: errors_list.append( @@ -29,17 +29,18 @@ def serialize_error_reports_to_json_response(errors): "release_version": error.release_version, "installation_type": error.installation_type, "context": error.context, + "pingback_id": pingback_id, } ) return json.dumps(errors_list, cls=DjangoJSONEncoder) @register_task -def ping_error_reports(server): +def ping_error_reports(server, pingback_id): try: errors = ErrorReports.get_unreported_errors() - errors_json = serialize_error_reports_to_json_response(errors) + errors_json = serialize_error_reports_to_json_response(errors, pingback_id) requests.post( join_url(server, "/api/v1/errors/report/"), diff --git a/kolibri/core/errorreports/test/test_middleware.py b/kolibri/core/errorreports/test/test_middleware.py index f63a954ee05..ecb4c562e36 100644 --- a/kolibri/core/errorreports/test/test_middleware.py +++ b/kolibri/core/errorreports/test/test_middleware.py @@ -15,6 +15,10 @@ class ErrorReportingMiddlewareTestCase(TestCase): def setUp(self): self.factory = RequestFactory() + @patch( + "kolibri.core.errorreports.middleware.get_request_time_to_error", + return_value=0.0, + ) @patch( "kolibri.core.errorreports.middleware.get_python_version", return_value="3.9.9" ) @@ -30,6 +34,7 @@ def test_process_exception( mock_insert_or_update_error, mock_get_packages, mock_get_python_version, + mock_get_request_time_to_error, ): middleware = ErrorReportingMiddleware(lambda r: None) request = self.factory.get("/") @@ -61,6 +66,7 @@ def test_process_exception( "packages": ["Django==3.2.25"], "python_version": "3.9.9", }, + request_time_to_error=0.0, ) @patch.object(ErrorReports, "insert_or_update_error") @@ -70,6 +76,7 @@ def test_process_exception_integrity_error( ): middleware = ErrorReportingMiddleware(lambda r: None) request = self.factory.get("/") + request.start_time = 0.0 exception = Exception("Test Exception") mock_insert_or_update_error.side_effect = IntegrityError("Some Integrity Error") middleware.process_exception(request, exception) diff --git a/kolibri/core/errorreports/test/test_tasks.py b/kolibri/core/errorreports/test/test_tasks.py index edb8af77e78..331de6d7ff5 100644 --- a/kolibri/core/errorreports/test/test_tasks.py +++ b/kolibri/core/errorreports/test/test_tasks.py @@ -57,7 +57,7 @@ def setUp(self): return_value="[]", ) def test_ping_error_reports(self, mock_serializer, mock_post): - ping_error_reports("http://testserver") + ping_error_reports("http://testserver", "test-pingback-id") mock_post.assert_called_with( "http://testserver/api/v1/errors/report/", data="[]", @@ -68,13 +68,13 @@ def test_ping_error_reports(self, mock_serializer, mock_post): @patch("kolibri.core.errorreports.tasks.requests.post", side_effect=ConnectionError) def test_ping_error_reports_connection_error(self, mock_post): with pytest.raises(ConnectionError): - ping_error_reports("http://testserver") + ping_error_reports("http://testserver", "test-pingback-id") self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 0) @patch("kolibri.core.errorreports.tasks.requests.post", side_effect=Timeout) def test_ping_error_reports_timeout(self, mock_post): with pytest.raises(Timeout): - ping_error_reports("http://testserver") + ping_error_reports("http://testserver", "test-pingback-id") self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 0) @patch( @@ -82,5 +82,5 @@ def test_ping_error_reports_timeout(self, mock_post): ) def test_ping_error_reports_request_exception(self, mock_post): with pytest.raises(RequestException): - ping_error_reports("http://testserver") + ping_error_reports("http://testserver", "test-pingback-id") self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 0) diff --git a/kolibri/deployment/default/settings/base.py b/kolibri/deployment/default/settings/base.py index a92e2e3bef5..882984941f9 100644 --- a/kolibri/deployment/default/settings/base.py +++ b/kolibri/deployment/default/settings/base.py @@ -105,6 +105,7 @@ "django.middleware.csrf.CsrfViewMiddleware", "kolibri.core.auth.middleware.CustomAuthenticationMiddleware", "kolibri.core.errorreports.middleware.ErrorReportingMiddleware", + "kolibri.core.errorreports.middleware.PreRequestMiddleware", "django.contrib.messages.middleware.MessageMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware", "django.middleware.security.SecurityMiddleware", From ee7eb3e75cf6a2af60cba432f5a0e52debda64be Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Fri, 2 Aug 2024 11:10:24 +0530 Subject: [PATCH 62/74] raise 400 instead of 500 --- kolibri/core/errorreports/api.py | 4 ++-- kolibri/core/errorreports/test/test_api.py | 24 +++++----------------- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/kolibri/core/errorreports/api.py b/kolibri/core/errorreports/api.py index 7a3be6ea517..b9103d7a0a9 100644 --- a/kolibri/core/errorreports/api.py +++ b/kolibri/core/errorreports/api.py @@ -29,11 +29,11 @@ def report(request): {"error_id": error.id if error else None}, status=status.HTTP_200_OK ) - except (AttributeError, ValidationError, Exception) as e: + except (AttributeError, ValidationError) as e: logger.error("Error while saving error report: {}".format(e)) return Response( {"error": "Error while saving error report: {}".format(e)}, - status=status.HTTP_500_INTERNAL_SERVER_ERROR, + status=status.HTTP_400_BAD_REQUEST, ) else: return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/core/errorreports/test/test_api.py index b6b91065488..e8924cddf9b 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/core/errorreports/test/test_api.py @@ -1,11 +1,10 @@ from unittest.mock import patch -from django.db.utils import IntegrityError +from django.core.exceptions import ValidationError from django.test import TestCase from django.urls import reverse from rest_framework.status import HTTP_200_OK from rest_framework.status import HTTP_400_BAD_REQUEST -from rest_framework.status import HTTP_500_INTERNAL_SERVER_ERROR from rest_framework.test import APIClient @@ -67,30 +66,17 @@ def test_frontend_report_server_error_attribute_error( ): url = reverse("kolibri:core:report") response = self.client.post(url, self.data, format="json") - self.assertEqual(response.status_code, HTTP_500_INTERNAL_SERVER_ERROR) - self.assertIn("error", response.data) - - @patch( - "kolibri.core.errorreports.models.ErrorReports.insert_or_update_error", - side_effect=Exception("Mocked exception"), - ) - def test_frontend_report_server_error_general_exception( - self, mock_insert_or_update_error - ): - url = reverse("kolibri:core:report") - response = self.client.post(url, self.data, format="json") - self.assertEqual(response.status_code, HTTP_500_INTERNAL_SERVER_ERROR) + self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST) self.assertIn("error", response.data) @patch( "kolibri.core.errorreports.models.ErrorReports.insert_or_update_error", - side_effect=IntegrityError("Mocked exception integrity error"), + side_effect=ValidationError("Mocked exception"), ) - def test_frontend_report_server_error_any_other_exception( + def test_frontend_report_server_error_validation_error( self, mock_insert_or_update_error ): - # this is to check that anything other than AttributeError or Exception can be caught url = reverse("kolibri:core:report") response = self.client.post(url, self.data, format="json") - self.assertEqual(response.status_code, HTTP_500_INTERNAL_SERVER_ERROR) + self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST) self.assertIn("error", response.data) From 780d544ed8259c2428623ec2f3164fc341b98faf Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Wed, 7 Aug 2024 15:00:45 +0000 Subject: [PATCH 63/74] [pre-commit.ci lite] apply automatic fixes --- kolibri/core/assets/src/core-app/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/assets/src/core-app/index.js b/kolibri/core/assets/src/core-app/index.js index 1eb79b4e519..4e69dd2291b 100644 --- a/kolibri/core/assets/src/core-app/index.js +++ b/kolibri/core/assets/src/core-app/index.js @@ -78,7 +78,7 @@ heartbeat.startPolling(); i18nSetup().then(coreApp.ready); // these shall be responsibe for catching runtime errors -Vue.config.errorHandler = function(err, vm) { +Vue.config.errorHandler = function (err, vm) { logging.error(`Unexpected Error: ${err}`); const error = new VueErrorReport(err, vm); ErrorReportResource.report(error); From 4f104ef17d6a65710eef18e100e93eb9d002224f Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Sat, 10 Aug 2024 02:17:39 +0300 Subject: [PATCH 64/74] reruns migrations --- .../errorreports/migrations/0001_initial.py | 21 ++++++-- .../migrations/0002_add_more_fields.py | 52 ------------------- 2 files changed, 17 insertions(+), 56 deletions(-) delete mode 100644 kolibri/core/errorreports/migrations/0002_add_more_fields.py diff --git a/kolibri/core/errorreports/migrations/0001_initial.py b/kolibri/core/errorreports/migrations/0001_initial.py index c25dded817d..d705ea4d498 100644 --- a/kolibri/core/errorreports/migrations/0001_initial.py +++ b/kolibri/core/errorreports/migrations/0001_initial.py @@ -1,8 +1,10 @@ -# Generated by Django 3.2.25 on 2024-06-07 09:20 +# Generated by Django 3.2.25 on 2024-08-09 23:13 import django.utils.timezone from django.db import migrations from django.db import models +import kolibri.core.fields + class Migration(migrations.Migration): @@ -24,7 +26,7 @@ class Migration(migrations.Migration): ), ), ( - "error_from", + "category", models.CharField( choices=[("frontend", "Frontend"), ("backend", "Backend")], max_length=10, @@ -40,8 +42,19 @@ class Migration(migrations.Migration): "last_occurred", models.DateTimeField(default=django.utils.timezone.now), ), - ("sent", models.BooleanField(default=False)), - ("no_of_errors", models.IntegerField(default=1)), + ("reported", models.BooleanField(default=False)), + ("events", models.IntegerField(default=1)), + ( + "release_version", + models.CharField( + blank=True, + default="0.16.2b2.dev0+git.1215.g780d544e", + max_length=128, + ), + ), + ("installation_type", models.CharField(blank=True, max_length=64)), + ("context", kolibri.core.fields.JSONField(blank=True, null=True)), + ("avg_request_time_to_error", models.FloatField(blank=True, null=True)), ], ), ] diff --git a/kolibri/core/errorreports/migrations/0002_add_more_fields.py b/kolibri/core/errorreports/migrations/0002_add_more_fields.py deleted file mode 100644 index f4d2157bd67..00000000000 --- a/kolibri/core/errorreports/migrations/0002_add_more_fields.py +++ /dev/null @@ -1,52 +0,0 @@ -# Generated by Django 3.2.25 on 2024-07-31 16:20 -from django.db import migrations -from django.db import models - -import kolibri.core.fields - - -class Migration(migrations.Migration): - - dependencies = [ - ("errorreports", "0001_initial"), - ] - - operations = [ - migrations.RenameField( - model_name="errorreports", - old_name="error_from", - new_name="category", - ), - migrations.RenameField( - model_name="errorreports", - old_name="no_of_errors", - new_name="events", - ), - migrations.RenameField( - model_name="errorreports", - old_name="sent", - new_name="reported", - ), - migrations.AddField( - model_name="errorreports", - name="avg_request_time_to_error", - field=models.FloatField(blank=True, null=True), - ), - migrations.AddField( - model_name="errorreports", - name="context", - field=kolibri.core.fields.JSONField(blank=True, null=True), - ), - migrations.AddField( - model_name="errorreports", - name="installation_type", - field=models.CharField(blank=True, max_length=64), - ), - migrations.AddField( - model_name="errorreports", - name="release_version", - field=models.CharField( - blank=True, default="0.16.2b2.dev0+git.1211.gc5eb2ae8", max_length=128 - ), - ), - ] From 085cf88540766b1cdaf984e08d91c51e6b489cd7 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Sat, 10 Aug 2024 02:27:43 +0300 Subject: [PATCH 65/74] reruns migrations --- kolibri/core/errorreports/migrations/0001_initial.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kolibri/core/errorreports/migrations/0001_initial.py b/kolibri/core/errorreports/migrations/0001_initial.py index d705ea4d498..c046c7b4d58 100644 --- a/kolibri/core/errorreports/migrations/0001_initial.py +++ b/kolibri/core/errorreports/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-08-09 23:13 +# Generated by Django 3.2.25 on 2024-08-09 23:25 import django.utils.timezone from django.db import migrations from django.db import models @@ -48,7 +48,7 @@ class Migration(migrations.Migration): "release_version", models.CharField( blank=True, - default="0.16.2b2.dev0+git.1215.g780d544e", + default="0.16.2b2.dev0+git.1216.g4f104ef1", max_length=128, ), ), From 5b909bc08a7579f291e42640d7f34b358b4fe435 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Sat, 10 Aug 2024 02:30:45 +0300 Subject: [PATCH 66/74] reruns migrations --- ...0002_alter_errorreports_release_version.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 kolibri/core/errorreports/migrations/0002_alter_errorreports_release_version.py diff --git a/kolibri/core/errorreports/migrations/0002_alter_errorreports_release_version.py b/kolibri/core/errorreports/migrations/0002_alter_errorreports_release_version.py new file mode 100644 index 00000000000..12d169b179f --- /dev/null +++ b/kolibri/core/errorreports/migrations/0002_alter_errorreports_release_version.py @@ -0,0 +1,20 @@ +# Generated by Django 3.2.25 on 2024-08-09 23:27 +from django.db import migrations +from django.db import models + + +class Migration(migrations.Migration): + + dependencies = [ + ("errorreports", "0001_initial"), + ] + + operations = [ + migrations.AlterField( + model_name="errorreports", + name="release_version", + field=models.CharField( + blank=True, default="0.16.2b2.dev0+git.1217.g085cf885", max_length=128 + ), + ), + ] From fd9e9251dcd1fd38566ee617a94595adde07bfd8 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Sat, 10 Aug 2024 02:49:14 +0300 Subject: [PATCH 67/74] reruns migrations --- .../errorreports/migrations/0001_initial.py | 4 ++-- ...0002_alter_errorreports_release_version.py | 20 ------------------- yarn.lock | 9 --------- 3 files changed, 2 insertions(+), 31 deletions(-) delete mode 100644 kolibri/core/errorreports/migrations/0002_alter_errorreports_release_version.py diff --git a/kolibri/core/errorreports/migrations/0001_initial.py b/kolibri/core/errorreports/migrations/0001_initial.py index c046c7b4d58..de26918ca04 100644 --- a/kolibri/core/errorreports/migrations/0001_initial.py +++ b/kolibri/core/errorreports/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-08-09 23:25 +# Generated by Django 3.2.25 on 2024-08-09 23:47 import django.utils.timezone from django.db import migrations from django.db import models @@ -48,7 +48,7 @@ class Migration(migrations.Migration): "release_version", models.CharField( blank=True, - default="0.16.2b2.dev0+git.1216.g4f104ef1", + default="0.16.2b2.dev0+git.1218.g5b909bc0", max_length=128, ), ), diff --git a/kolibri/core/errorreports/migrations/0002_alter_errorreports_release_version.py b/kolibri/core/errorreports/migrations/0002_alter_errorreports_release_version.py deleted file mode 100644 index 12d169b179f..00000000000 --- a/kolibri/core/errorreports/migrations/0002_alter_errorreports_release_version.py +++ /dev/null @@ -1,20 +0,0 @@ -# Generated by Django 3.2.25 on 2024-08-09 23:27 -from django.db import migrations -from django.db import models - - -class Migration(migrations.Migration): - - dependencies = [ - ("errorreports", "0001_initial"), - ] - - operations = [ - migrations.AlterField( - model_name="errorreports", - name="release_version", - field=models.CharField( - blank=True, default="0.16.2b2.dev0+git.1217.g085cf885", max_length=128 - ), - ), - ] diff --git a/yarn.lock b/yarn.lock index 21d577c750b..273163724f4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2438,15 +2438,6 @@ aphrodite@1.1.0: asap "^2.0.3" inline-style-prefixer "^2.0.0" -"aphrodite@git+https://github.com/learningequality/aphrodite.git": - version "2.2.3" - uid fdc8d7be8912a5cf17f74ff10f124013c52c3e32 - resolved "git+https://github.com/learningequality/aphrodite.git#fdc8d7be8912a5cf17f74ff10f124013c52c3e32" - dependencies: - asap "^2.0.3" - inline-style-prefixer "^4.0.2" - string-hash "^1.1.3" - "aphrodite@https://github.com/learningequality/aphrodite/": version "2.2.3" resolved "https://github.com/learningequality/aphrodite/#fdc8d7be8912a5cf17f74ff10f124013c52c3e32" From ac07e7ae088dfda89a6e74b9a8d11f64018d89f5 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Sat, 10 Aug 2024 02:56:46 +0300 Subject: [PATCH 68/74] Removes information exposure through exception --- kolibri/core/errorreports/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/errorreports/api.py b/kolibri/core/errorreports/api.py index b9103d7a0a9..ac814fee3e0 100644 --- a/kolibri/core/errorreports/api.py +++ b/kolibri/core/errorreports/api.py @@ -32,7 +32,7 @@ def report(request): except (AttributeError, ValidationError) as e: logger.error("Error while saving error report: {}".format(e)) return Response( - {"error": "Error while saving error report: {}".format(e)}, + {"error": "An error occurred while saving reporting errors."}, status=status.HTTP_400_BAD_REQUEST, ) else: From 6939d0e0f2ce2184d6c2b5eb5a470bfa33f2637d Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Sat, 10 Aug 2024 02:57:28 +0300 Subject: [PATCH 69/74] Removes information exposure through exception --- kolibri/core/errorreports/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/errorreports/api.py b/kolibri/core/errorreports/api.py index ac814fee3e0..c27b1c70066 100644 --- a/kolibri/core/errorreports/api.py +++ b/kolibri/core/errorreports/api.py @@ -32,7 +32,7 @@ def report(request): except (AttributeError, ValidationError) as e: logger.error("Error while saving error report: {}".format(e)) return Response( - {"error": "An error occurred while saving reporting errors."}, + {"error": "An error occurred while saving errors."}, status=status.HTTP_400_BAD_REQUEST, ) else: From b64c4c2b039d8eee781acdab456401b81329088b Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Sat, 14 Sep 2024 20:17:54 +0530 Subject: [PATCH 70/74] refactor stuffs remove installation_type and release_version\n move request_time_to_error to context\n remove sensitive info from the requests info\n only use traceback and error_msg to fingerprint an error_report --- kolibri/core/errorreports/middleware.py | 16 ++++-- .../errorreports/migrations/0001_initial.py | 12 +---- kolibri/core/errorreports/models.py | 52 +++++++++---------- kolibri/core/errorreports/schemas.py | 1 + kolibri/core/errorreports/tasks.py | 2 - .../core/errorreports/test/test_middleware.py | 4 +- kolibri/core/errorreports/test/test_tasks.py | 1 + 7 files changed, 40 insertions(+), 48 deletions(-) diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index 73a5487a66a..98d10771ca6 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -16,12 +16,20 @@ def get_request_info(request): + # checked the codebase and found these are the sensitive headers + request_headers = dict(request.headers) + request_headers.pop("X-Csrftoken", None) + request_headers.pop("Cookie", None) + + request_get = dict(request.GET) + request_get.pop("token", None) + return { "url": request.build_absolute_uri(), "method": request.method, - "headers": dict(request.headers), + "headers": request_headers, "body": request.body.decode("utf-8"), - "query_params": dict(request.GET), + "query_params": request_get, } @@ -60,12 +68,12 @@ def __call__(self, request): def process_exception(self, request, exception): error_message = str(exception) traceback_info = traceback.format_exc() - request_time_to_error = get_request_time_to_error(request) context = { "request_info": get_request_info(request), "server": get_server_info(request), "packages": get_packages(), "python_version": get_python_version(), + "avg_request_time_to_error": get_request_time_to_error(request), } self.logger.error("Unexpected Error: %s", error_message) try: @@ -75,9 +83,7 @@ def process_exception(self, request, exception): error_message, traceback_info, context, - request_time_to_error=request_time_to_error, ) - self.logger.info("Error report saved to the database.") except (IntegrityError, ValidationError) as e: self.logger.error( "Error occurred while saving error report to the database: %s", str(e) diff --git a/kolibri/core/errorreports/migrations/0001_initial.py b/kolibri/core/errorreports/migrations/0001_initial.py index de26918ca04..ee57340edd7 100644 --- a/kolibri/core/errorreports/migrations/0001_initial.py +++ b/kolibri/core/errorreports/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-08-09 23:47 +# Generated by Django 3.2.25 on 2024-09-14 14:20 import django.utils.timezone from django.db import migrations from django.db import models @@ -44,17 +44,7 @@ class Migration(migrations.Migration): ), ("reported", models.BooleanField(default=False)), ("events", models.IntegerField(default=1)), - ( - "release_version", - models.CharField( - blank=True, - default="0.16.2b2.dev0+git.1218.g5b909bc0", - max_length=128, - ), - ), - ("installation_type", models.CharField(blank=True, max_length=64)), ("context", kolibri.core.fields.JSONField(blank=True, null=True)), - ("avg_request_time_to_error", models.FloatField(blank=True, null=True)), ], ), ] diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index d8349bfa11b..3439ac3517c 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -9,11 +9,9 @@ from .constants import POSSIBLE_ERRORS from .schemas import context_backend_schema from .schemas import context_frontend_schema -from kolibri import __version__ from kolibri.core.fields import JSONField from kolibri.core.utils.validators import JSON_Schema_Validator from kolibri.deployment.default.sqlite_db_names import ERROR_REPORTS -from kolibri.utils.server import installation_type logger = logging.getLogger(__name__) @@ -62,13 +60,10 @@ class ErrorReports(models.Model): last_occurred = models.DateTimeField(default=timezone.now) reported = models.BooleanField(default=False) events = models.IntegerField(default=1) - release_version = models.CharField(max_length=128, default=__version__, blank=True) - installation_type = models.CharField(max_length=64, blank=True) context = JSONField( null=True, blank=True, ) - avg_request_time_to_error = models.FloatField(null=True, blank=True) # in seconds def __str__(self): return f"{self.error_message} ({self.category})" @@ -84,34 +79,35 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) @classmethod - def insert_or_update_error( - cls, category, error_message, traceback, context, request_time_to_error=None - ): - if not getattr(settings, "DEVELOPER_MODE", None): - error, created = cls.objects.get_or_create( + def insert_or_update_error(cls, category, error_message, traceback, context): + if getattr(settings, "DEVELOPER_MODE", False): + logger.info( + "ErrorReports: Database not updated, as DEVELOPER_MODE is True." + ) + return + error_report = cls.objects.filter( + category=category, error_message=error_message, traceback=traceback + ).first() + if error_report is not None: + error_report.events += 1 + error_report.last_occurred = timezone.now() + if error_report.context.get("avg_request_time_to_error", None): + context["avg_request_time_to_error"] = ( + error_report.context["avg_request_time_to_error"] + * (error_report.events - 1) + + context["avg_request_time_to_error"] + ) / error_report.events + error_report.context = context + error_report.save() + else: + error_report = cls.objects.create( category=category, error_message=error_message, traceback=traceback, context=context, - release_version=__version__, - installation_type=installation_type(), ) - if not created: - error.events += 1 - error.last_occurred = timezone.now() - if error.avg_request_time_to_error is not None: - # see the proof: https://math.stackexchange.com/a/106314 - error.avg_request_time_to_error = ( - error.avg_request_time_to_error * (error.events - 1) - + request_time_to_error - ) / error.events - else: - error.avg_request_time_to_error = request_time_to_error - error.save() - logger.error("ErrorReports: Database updated.") - return error - logger.error("ErrorReports: Database not updated, as DEVELOPER_MODE is True.") - return None + logger.error("ErrorReports: Database updated.") + return error_report @classmethod def get_unreported_errors(cls): diff --git a/kolibri/core/errorreports/schemas.py b/kolibri/core/errorreports/schemas.py index 546d3bc01cd..cf8328d9711 100644 --- a/kolibri/core/errorreports/schemas.py +++ b/kolibri/core/errorreports/schemas.py @@ -61,5 +61,6 @@ }, "packages": {"type": "array", "optional": True}, "python_version": {"type": "string", "optional": True}, + "avg_request_time_to_error": {"type": "number", "optional": True}, }, } diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/errorreports/tasks.py index bd4f94e95d6..a54a502557f 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/errorreports/tasks.py @@ -26,8 +26,6 @@ def serialize_error_reports_to_json_response(errors, pingback_id): "first_occurred": error.first_occurred, "last_occurred": error.last_occurred, "events": error.events, - "release_version": error.release_version, - "installation_type": error.installation_type, "context": error.context, "pingback_id": pingback_id, } diff --git a/kolibri/core/errorreports/test/test_middleware.py b/kolibri/core/errorreports/test/test_middleware.py index ecb4c562e36..f1624dba85b 100644 --- a/kolibri/core/errorreports/test/test_middleware.py +++ b/kolibri/core/errorreports/test/test_middleware.py @@ -58,15 +58,15 @@ def test_process_exception( "request_info": { "url": "http://testserver/", "method": "GET", - "headers": dict(request.headers), + "headers": {}, # checking whether cookies are removed "body": "", "query_params": {}, }, "server": {"host": "testserver", "port": "80"}, "packages": ["Django==3.2.25"], "python_version": "3.9.9", + "avg_request_time_to_error": 0.0, }, - request_time_to_error=0.0, ) @patch.object(ErrorReports, "insert_or_update_error") diff --git a/kolibri/core/errorreports/test/test_tasks.py b/kolibri/core/errorreports/test/test_tasks.py index 331de6d7ff5..a880dba2d05 100644 --- a/kolibri/core/errorreports/test/test_tasks.py +++ b/kolibri/core/errorreports/test/test_tasks.py @@ -48,6 +48,7 @@ def setUp(self): "server": {"host": "localhost", "port": "8000"}, "packages": ["Django==3.2.25"], "python_version": "3.9.1", + "request_time_to_error": 0.0, }, ) From 340f7141d1d858614223fdc4e928759676558df7 Mon Sep 17 00:00:00 2001 From: Sujai Kumar Gupta Date: Mon, 23 Sep 2024 12:30:29 +0530 Subject: [PATCH 71/74] use get_or_create() with defaults arg --- kolibri/core/errorreports/models.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index 3439ac3517c..1e7318fbea4 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -85,9 +85,12 @@ def insert_or_update_error(cls, category, error_message, traceback, context): "ErrorReports: Database not updated, as DEVELOPER_MODE is True." ) return - error_report = cls.objects.filter( - category=category, error_message=error_message, traceback=traceback - ).first() + error_report, _ = cls.objects.get_or_create( + category=category, + error_message=error_message, + traceback=traceback, + defaults={"context": context}, + ) if error_report is not None: error_report.events += 1 error_report.last_occurred = timezone.now() @@ -98,14 +101,8 @@ def insert_or_update_error(cls, category, error_message, traceback, context): + context["avg_request_time_to_error"] ) / error_report.events error_report.context = context - error_report.save() - else: - error_report = cls.objects.create( - category=category, - error_message=error_message, - traceback=traceback, - context=context, - ) + + error_report.save() logger.error("ErrorReports: Database updated.") return error_report From 8ab29d4e3f502152b843e04242f21a391fa43d72 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 25 Sep 2024 16:10:00 -0700 Subject: [PATCH 72/74] Make error reporting pluggable. --- kolibri/__init__.py | 1 + kolibri/core/api_urls.py | 1 - .../assets/src/api-resources/errorReport.js | 15 ---- .../core/assets/src/api-resources/index.js | 1 - kolibri/core/assets/src/core-app/index.js | 28 +------ kolibri/core/errorreports/middleware.py | 8 ++ kolibri/plugins/error_reports/__init__.py | 0 .../error_reports}/api.py | 4 +- .../error_reports}/api_urls.py | 0 .../assets/src}/__tests__/errorReport.test.js | 39 +++++----- .../plugins/error_reports/assets/src/index.js | 32 ++++++++ .../plugins/error_reports/assets/src/utils.js | 74 +++++++++++++++++++ kolibri/plugins/error_reports/buildConfig.js | 8 ++ .../plugins/error_reports/kolibri_plugin.py | 22 ++++++ .../error_reports}/serializers.py | 2 +- .../error_reports}/test/test_api.py | 8 +- kolibri/utils/build_config/default_plugins.py | 1 + 17 files changed, 174 insertions(+), 70 deletions(-) delete mode 100644 kolibri/core/assets/src/api-resources/errorReport.js create mode 100644 kolibri/plugins/error_reports/__init__.py rename kolibri/{core/errorreports => plugins/error_reports}/api.py (91%) rename kolibri/{core/errorreports => plugins/error_reports}/api_urls.py (100%) rename kolibri/{core/assets/src/api-resources => plugins/error_reports/assets/src}/__tests__/errorReport.test.js (64%) create mode 100644 kolibri/plugins/error_reports/assets/src/index.js create mode 100644 kolibri/plugins/error_reports/assets/src/utils.js create mode 100644 kolibri/plugins/error_reports/buildConfig.js create mode 100644 kolibri/plugins/error_reports/kolibri_plugin.py rename kolibri/{core/errorreports => plugins/error_reports}/serializers.py (80%) rename kolibri/{core/errorreports => plugins/error_reports}/test/test_api.py (90%) diff --git a/kolibri/__init__.py b/kolibri/__init__.py index 8aa9879b173..6c7b49bf316 100755 --- a/kolibri/__init__.py +++ b/kolibri/__init__.py @@ -27,6 +27,7 @@ "kolibri.plugins.demo_server", "kolibri.plugins.device", "kolibri.plugins.epub_viewer", + "kolibri.plugins.error_reports", "kolibri.plugins.html5_viewer", "kolibri.plugins.facility", "kolibri.plugins.learn", diff --git a/kolibri/core/api_urls.py b/kolibri/core/api_urls.py index 5e112955cfc..7681b05498f 100644 --- a/kolibri/core/api_urls.py +++ b/kolibri/core/api_urls.py @@ -13,5 +13,4 @@ re_path(r"^discovery/", include("kolibri.core.discovery.api_urls")), re_path(r"^notifications/", include("kolibri.core.analytics.api_urls")), re_path(r"^public/", include("kolibri.core.public.api_urls")), - re_path(r"^errorreports/", include("kolibri.core.errorreports.api_urls")), ] diff --git a/kolibri/core/assets/src/api-resources/errorReport.js b/kolibri/core/assets/src/api-resources/errorReport.js deleted file mode 100644 index 11289ee9c6a..00000000000 --- a/kolibri/core/assets/src/api-resources/errorReport.js +++ /dev/null @@ -1,15 +0,0 @@ -import { Resource } from 'kolibri.lib.apiResource'; -import urls from 'kolibri.urls'; - -export default new Resource({ - name: 'errorreports', - report(error) { - const url = urls['kolibri:core:report'](); - const data = error.getErrorReport(); - return this.client({ - url, - method: 'post', - data: data, - }); - }, -}); diff --git a/kolibri/core/assets/src/api-resources/index.js b/kolibri/core/assets/src/api-resources/index.js index 8e0b2331b31..2fd46b3e9da 100644 --- a/kolibri/core/assets/src/api-resources/index.js +++ b/kolibri/core/assets/src/api-resources/index.js @@ -24,7 +24,6 @@ export { default as UserSyncStatusResource } from './userSyncStatus'; export { default as ContentRequestResource } from './contentRequest'; export { default as ContentNodeProgressResource } from './contentNodeProgress'; export { default as DevicePermissionsResource } from './devicePermissions'; -export { default as ErrorReportResource } from './errorReport'; export { default as RemoteChannelResource } from './remoteChannel'; export { default as LessonResource } from './lesson'; export { default as AttemptLogResource } from './attemptLog'; diff --git a/kolibri/core/assets/src/core-app/index.js b/kolibri/core/assets/src/core-app/index.js index 4e69dd2291b..10e451939c4 100644 --- a/kolibri/core/assets/src/core-app/index.js +++ b/kolibri/core/assets/src/core-app/index.js @@ -16,14 +16,8 @@ import heartbeat from 'kolibri.heartbeat'; import ContentRenderer from '../views/ContentRenderer'; import initializeTheme from '../styles/initializeTheme'; import { i18nSetup } from '../utils/i18n'; -import { ErrorReportResource } from '../api-resources'; -import { - VueErrorReport, - JavascriptErrorReport, - UnhandledRejectionErrorReport, -} from '../utils/errorReportUtils'; -import apiSpec from './apiSpec'; import setupPluginMediator from './pluginMediator'; +import apiSpec from './apiSpec'; // Do this before any async imports to ensure that public paths // are set correctly @@ -77,26 +71,6 @@ heartbeat.startPolling(); i18nSetup().then(coreApp.ready); -// these shall be responsibe for catching runtime errors -Vue.config.errorHandler = function (err, vm) { - logging.error(`Unexpected Error: ${err}`); - const error = new VueErrorReport(err, vm); - ErrorReportResource.report(error); -}; - -window.addEventListener('error', e => { - logging.error(`Unexpected Error: ${e.error}`); - const error = new JavascriptErrorReport(e); - ErrorReportResource.report(error); -}); - -window.addEventListener('unhandledrejection', event => { - event.preventDefault(); - logging.error(`Unhandled Rejection: ${event.reason}`); - const error = new UnhandledRejectionErrorReport(event); - ErrorReportResource.report(error); -}); - // This is exported by webpack as the kolibriCoreAppGlobal object, due to the 'output.library' flag // which exports the coreApp at the bottom of this file as a named global variable: // https://webpack.github.io/docs/configuration.html#output-library diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index 98d10771ca6..242f4dfac9a 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -8,12 +8,16 @@ else: from importlib.metadata import distributions +from django.core.exceptions import MiddlewareNotUsed from django.core.exceptions import ValidationError from django.db import IntegrityError from .constants import BACKEND from .models import ErrorReports +from kolibri.plugins.error_reports.kolibri_plugin import ErrorReportsPlugin +from kolibri.plugins.registry import registered_plugins + def get_request_info(request): # checked the codebase and found these are the sensitive headers @@ -58,6 +62,8 @@ class ErrorReportingMiddleware: """ def __init__(self, get_response): + if ErrorReportsPlugin not in registered_plugins: + raise MiddlewareNotUsed("ErrorReportsPlugin is not enabled.") self.get_response = get_response self.logger = logging.getLogger(__name__) @@ -92,6 +98,8 @@ def process_exception(self, request, exception): class PreRequestMiddleware: def __init__(self, get_response): + if ErrorReportsPlugin not in registered_plugins: + raise MiddlewareNotUsed("ErrorReportsPlugin is not enabled.") self.get_response = get_response def __call__(self, request): diff --git a/kolibri/plugins/error_reports/__init__.py b/kolibri/plugins/error_reports/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/kolibri/core/errorreports/api.py b/kolibri/plugins/error_reports/api.py similarity index 91% rename from kolibri/core/errorreports/api.py rename to kolibri/plugins/error_reports/api.py index c27b1c70066..db6fdf7c61d 100644 --- a/kolibri/core/errorreports/api.py +++ b/kolibri/plugins/error_reports/api.py @@ -5,9 +5,9 @@ from rest_framework.decorators import api_view from rest_framework.response import Response -from .constants import FRONTEND -from .models import ErrorReports from .serializers import ErrorReportSerializer +from kolibri.core.errorreports.constants import FRONTEND +from kolibri.core.errorreports.models import ErrorReports logger = logging.getLogger(__name__) diff --git a/kolibri/core/errorreports/api_urls.py b/kolibri/plugins/error_reports/api_urls.py similarity index 100% rename from kolibri/core/errorreports/api_urls.py rename to kolibri/plugins/error_reports/api_urls.py diff --git a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js b/kolibri/plugins/error_reports/assets/src/__tests__/errorReport.test.js similarity index 64% rename from kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js rename to kolibri/plugins/error_reports/assets/src/__tests__/errorReport.test.js index 8c91dcd35fb..1a863a63c0d 100644 --- a/kolibri/core/assets/src/api-resources/__tests__/errorReport.test.js +++ b/kolibri/plugins/error_reports/assets/src/__tests__/errorReport.test.js @@ -1,26 +1,30 @@ +import client from 'kolibri.client'; import urls from 'kolibri.urls'; -import Resource from '../errorReport'; import { + report, VueErrorReport, JavascriptErrorReport, UnhandledRejectionErrorReport, -} from '../../utils/errorReportUtils'; +} from '../utils'; /* eslint-env jest */ jest.mock('kolibri.urls', () => ({ - 'kolibri:core:report': jest.fn(), + 'kolibri:kolibri.plugins.error_reports:report': jest.fn(), })); +jest.mock('kolibri.client', () => jest.fn()); describe('Error Report', () => { beforeEach(() => { - urls['kolibri:core:report'].mockReturnValue('/api/core/report'); + urls['kolibri:kolibri.plugins.error_reports:report'].mockReturnValue( + '/error_reports/api/report' + ); }); afterEach(() => { jest.clearAllMocks(); }); - it('should call api/core/report with VueErrorReport data', () => { + it('should call /error_reports/api/report with VueErrorReport data', () => { const vueError = new Error('Vue error'); vueError.stack = 'My stack trace'; const vm = { $options: { name: 'TestComponent' } }; @@ -35,17 +39,16 @@ describe('Error Report', () => { }, }; - Resource.client = jest.fn(); - Resource.report(errorReport); + report(errorReport); - expect(Resource.client).toHaveBeenCalledWith({ - url: '/api/core/report', + expect(client).toHaveBeenCalledWith({ + url: '/error_reports/api/report', method: 'post', data: expectedData, }); }); - it('should call api/core/report with JavascriptErrorReport data', () => { + it('should call /error_reports/api/report with JavascriptErrorReport data', () => { const jsErrorEvent = { error: new Error('Javascript error'), }; @@ -59,17 +62,16 @@ describe('Error Report', () => { context: errorReport.getContext(), }; - Resource.client = jest.fn(); - Resource.report(errorReport); + report(errorReport); - expect(Resource.client).toHaveBeenCalledWith({ - url: '/api/core/report', + expect(client).toHaveBeenCalledWith({ + url: '/error_reports/api/report', method: 'post', data: expectedData, }); }); - it('should call api/core/report with UnhandledRejectionErrorReport data', () => { + it('should call /error_reports/api/report with UnhandledRejectionErrorReport data', () => { const rejectionEvent = { reason: new Error('Unhandled rejection'), }; @@ -83,11 +85,10 @@ describe('Error Report', () => { context: errorReport.getContext(), }; - Resource.client = jest.fn(); - Resource.report(errorReport); + report(errorReport); - expect(Resource.client).toHaveBeenCalledWith({ - url: '/api/core/report', + expect(client).toHaveBeenCalledWith({ + url: '/error_reports/api/report', method: 'post', data: expectedData, }); diff --git a/kolibri/plugins/error_reports/assets/src/index.js b/kolibri/plugins/error_reports/assets/src/index.js new file mode 100644 index 00000000000..d6408297759 --- /dev/null +++ b/kolibri/plugins/error_reports/assets/src/index.js @@ -0,0 +1,32 @@ +import Vue from 'vue'; +import logging from 'kolibri.lib.logging'; +import { + report, + VueErrorReport, + JavascriptErrorReport, + UnhandledRejectionErrorReport, +} from './utils'; + +const logger = logging.getLogger(__filename); + +// these shall be responsibe for catching runtime errors +Vue.config.errorHandler = function(err, vm) { + logger.debug(`Unexpected Error: ${err}`); + const error = new VueErrorReport(err, vm); + report(error); +}; + +window.addEventListener('error', e => { + logger.debug(`Unexpected Error: ${e.error}`); + const error = new JavascriptErrorReport(e); + report(error); +}); + +window.addEventListener('unhandledrejection', event => { + if (process.env.NODE_ENV === 'production') { + event.preventDefault(); + } + logger.debug(`Unhandled Rejection: ${event.reason}`); + const error = new UnhandledRejectionErrorReport(event); + report(error); +}); diff --git a/kolibri/plugins/error_reports/assets/src/utils.js b/kolibri/plugins/error_reports/assets/src/utils.js new file mode 100644 index 00000000000..e242a44a8b5 --- /dev/null +++ b/kolibri/plugins/error_reports/assets/src/utils.js @@ -0,0 +1,74 @@ +import client from 'kolibri.client'; +import urls from 'kolibri.urls'; +import { browser, os, device, isTouchDevice } from 'kolibri.utils.browserInfo'; + +export function report(error) { + const url = urls['kolibri:kolibri.plugins.error_reports:report'](); + const data = error.getErrorReport(); + return client({ + url, + method: 'post', + data: data, + }); +} + +class ErrorReport { + constructor(e) { + this.message = e?.message || 'Unknown Error'; + this.stack = e?.stack || 'No stack trace available'; + } + + getErrorReport() { + return { + error_message: this.message, + traceback: this.stack, + context: this.getContext(), + }; + } + + getContext() { + return { + browser: browser, + os: os, + device: { + ...device, + is_touch_device: isTouchDevice, + screen: { + width: window.screen.width, + height: window.screen.height, + available_width: window.screen.availWidth, + available_height: window.screen.availHeight, + }, + }, + ...this.getExtraContext(), + }; + } + + getExtraContext() { + return {}; + } +} + +export class VueErrorReport extends ErrorReport { + constructor(e, vm) { + super(e); + this.vm = vm; + } + getExtraContext() { + return { + component: this.vm.$options.name || this.vm.$options._componentTag || 'Unknown Component', + }; + } +} + +export class JavascriptErrorReport extends ErrorReport { + constructor(e) { + super(e.error || { message: e.message }); + } +} + +export class UnhandledRejectionErrorReport extends ErrorReport { + constructor(e) { + super(e.reason); + } +} diff --git a/kolibri/plugins/error_reports/buildConfig.js b/kolibri/plugins/error_reports/buildConfig.js new file mode 100644 index 00000000000..502e9e18dd0 --- /dev/null +++ b/kolibri/plugins/error_reports/buildConfig.js @@ -0,0 +1,8 @@ +module.exports = [ + { + bundle_id: 'main', + webpack_config: { + entry: './assets/src/index.js', + }, + }, +]; diff --git a/kolibri/plugins/error_reports/kolibri_plugin.py b/kolibri/plugins/error_reports/kolibri_plugin.py new file mode 100644 index 00000000000..50e9cbdc524 --- /dev/null +++ b/kolibri/plugins/error_reports/kolibri_plugin.py @@ -0,0 +1,22 @@ +from kolibri.core.hooks import FrontEndBaseSyncHook +from kolibri.core.webpack.hooks import WebpackBundleHook +from kolibri.plugins import KolibriPluginBase +from kolibri.plugins.hooks import register_hook + + +class ErrorReportsPlugin(KolibriPluginBase): + """ + A plugin to capture and report errors in Kolibri. + """ + + untranslated_view_urls = "api_urls" + + +@register_hook +class ErrorReportsPluginAsset(WebpackBundleHook): + bundle_id = "main" + + +@register_hook +class ErrorReportsPluginInclusionHook(FrontEndBaseSyncHook): + bundle_class = ErrorReportsPluginAsset diff --git a/kolibri/core/errorreports/serializers.py b/kolibri/plugins/error_reports/serializers.py similarity index 80% rename from kolibri/core/errorreports/serializers.py rename to kolibri/plugins/error_reports/serializers.py index 765653e39b2..880e0551137 100644 --- a/kolibri/core/errorreports/serializers.py +++ b/kolibri/plugins/error_reports/serializers.py @@ -1,6 +1,6 @@ from rest_framework import serializers -from .models import ErrorReports +from kolibri.core.errorreports.models import ErrorReports class ErrorReportSerializer(serializers.ModelSerializer): diff --git a/kolibri/core/errorreports/test/test_api.py b/kolibri/plugins/error_reports/test/test_api.py similarity index 90% rename from kolibri/core/errorreports/test/test_api.py rename to kolibri/plugins/error_reports/test/test_api.py index e8924cddf9b..04ae67825e1 100644 --- a/kolibri/core/errorreports/test/test_api.py +++ b/kolibri/plugins/error_reports/test/test_api.py @@ -46,12 +46,12 @@ def setUp(self): self.client = APIClient() def test_frontend_report(self): - url = reverse("kolibri:core:report") + url = reverse("kolibri:kolibri.plugins.error_reports:report") response = self.client.post(url, self.data, format="json") self.assertEqual(response.status_code, HTTP_200_OK) def test_frontend_report_invalid_data(self): - url = reverse("kolibri:core:report") + url = reverse("kolibri:kolibri.plugins.error_reports:report") data = self.data.copy() invalid_data = data.pop("context") response = self.client.post(url, invalid_data, format="json") @@ -64,7 +64,7 @@ def test_frontend_report_invalid_data(self): def test_frontend_report_server_error_attribute_error( self, mock_insert_or_update_error ): - url = reverse("kolibri:core:report") + url = reverse("kolibri:kolibri.plugins.error_reports:report") response = self.client.post(url, self.data, format="json") self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST) self.assertIn("error", response.data) @@ -76,7 +76,7 @@ def test_frontend_report_server_error_attribute_error( def test_frontend_report_server_error_validation_error( self, mock_insert_or_update_error ): - url = reverse("kolibri:core:report") + url = reverse("kolibri:kolibri.plugins.error_reports:report") response = self.client.post(url, self.data, format="json") self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST) self.assertIn("error", response.data) diff --git a/kolibri/utils/build_config/default_plugins.py b/kolibri/utils/build_config/default_plugins.py index c2892a53e50..54d7bec0b8d 100644 --- a/kolibri/utils/build_config/default_plugins.py +++ b/kolibri/utils/build_config/default_plugins.py @@ -3,6 +3,7 @@ "kolibri.plugins.default_theme", "kolibri.plugins.device", "kolibri.plugins.epub_viewer", + "kolibri.plugins.error_reports", "kolibri.plugins.html5_viewer", "kolibri.plugins.facility", "kolibri.plugins.learn", From 39d07ad1b0553d46879a5593178ef3d4098103d5 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 25 Sep 2024 18:05:47 -0700 Subject: [PATCH 73/74] Add error capturing for tasks. --- kolibri/core/error_reports/middleware.py | 108 ++++++++++++++++++ kolibri/core/errorreports/constants.py | 2 + kolibri/core/errorreports/middleware.py | 6 +- .../0002_alter_errorreports_category.py | 25 ++++ kolibri/core/errorreports/models.py | 17 ++- kolibri/core/errorreports/schemas.py | 42 +++++++ kolibri/core/errorreports/test/test_models.py | 103 ++++++++++++++++- .../plugins/error_reports/kolibri_plugin.py | 49 ++++++++ 8 files changed, 337 insertions(+), 15 deletions(-) create mode 100644 kolibri/core/error_reports/middleware.py create mode 100644 kolibri/core/errorreports/migrations/0002_alter_errorreports_category.py diff --git a/kolibri/core/error_reports/middleware.py b/kolibri/core/error_reports/middleware.py new file mode 100644 index 00000000000..ddafef4d397 --- /dev/null +++ b/kolibri/core/error_reports/middleware.py @@ -0,0 +1,108 @@ +import logging +import time +import traceback +from sys import version_info + +if version_info < (3, 10): + from importlib_metadata import distributions +else: + from importlib.metadata import distributions + +from django.core.exceptions import MiddlewareNotUsed +from django.core.exceptions import ValidationError +from django.db import IntegrityError + +from .constants import BACKEND +from .models import ErrorReport + +from kolibri.plugins.error_reports.kolibri_plugin import ErrorReportsPlugin +from kolibri.plugins.registry import registered_plugins + + +def get_request_info(request): + # checked the codebase and found these are the sensitive headers + request_headers = dict(request.headers) + request_headers.pop("X-Csrftoken", None) + request_headers.pop("Cookie", None) + + request_get = dict(request.GET) + request_get.pop("token", None) + + return { + "url": request.build_absolute_uri(), + "method": request.method, + "headers": request_headers, + "body": request.body.decode("utf-8"), + "query_params": request_get, + } + + +def get_server_info(request): + return {"host": request.get_host(), "port": request.get_port()} + + +def get_packages(): + packages = [f"{dist.metadata['Name']}=={dist.version}" for dist in distributions()] + return packages + + +def get_python_version(): + return ".".join(str(v) for v in version_info[:3]) + + +def get_request_time_to_error(request): + return time.time() - request.start_time + + +class ErrorReportingMiddleware: + """ + Middleware to log exceptions to the database. + """ + + def __init__(self, get_response): + if ErrorReportsPlugin not in registered_plugins: + raise MiddlewareNotUsed("ErrorReportsPlugin is not enabled.") + self.get_response = get_response + self.logger = logging.getLogger(__name__) + + def __call__(self, request): + response = self.get_response(request) + return response + + def process_exception(self, request, exception): + error_message = str(exception) + traceback_info = traceback.format_exc() + context = { + "request_info": get_request_info(request), + "server": get_server_info(request), + "packages": get_packages(), + "python_version": get_python_version(), + "avg_request_time_to_error": get_request_time_to_error(request), + } + self.logger.error("Unexpected Error: %s", error_message) + try: + self.logger.error("Saving error report to the database.") + ErrorReport.insert_or_update_error( + BACKEND, + error_message, + traceback_info, + context, + ) + except (IntegrityError, ValidationError) as e: + self.logger.error( + "Error occurred while saving error report to the database: %s", str(e) + ) + + +class PreRequestMiddleware: + def __init__(self, get_response): + if ErrorReportsPlugin not in registered_plugins: + raise MiddlewareNotUsed("ErrorReportsPlugin is not enabled.") + self.get_response = get_response + + def __call__(self, request): + response = self.get_response(request) + return response + + def process_view(self, request, view_func, view_args, view_kwargs): + request.start_time = time.time() diff --git a/kolibri/core/errorreports/constants.py b/kolibri/core/errorreports/constants.py index b32b9331260..1042ac7c300 100644 --- a/kolibri/core/errorreports/constants.py +++ b/kolibri/core/errorreports/constants.py @@ -1,7 +1,9 @@ FRONTEND = "frontend" BACKEND = "backend" +TASK = "task" POSSIBLE_ERRORS = [ (FRONTEND, "Frontend"), (BACKEND, "Backend"), + (TASK, "Task"), ] diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py index 242f4dfac9a..5cb57637891 100644 --- a/kolibri/core/errorreports/middleware.py +++ b/kolibri/core/errorreports/middleware.py @@ -47,9 +47,7 @@ def get_packages(): def get_python_version(): - return "{major}.{minor}.{micro}".format( - major=version_info.major, minor=version_info.minor, micro=version_info.micro - ) + return ".".join(str(v) for v in version_info[:3]) def get_request_time_to_error(request): @@ -78,7 +76,7 @@ def process_exception(self, request, exception): "request_info": get_request_info(request), "server": get_server_info(request), "packages": get_packages(), - "python_version": get_python_version(), + "python_version": ".".join(str(v) for v in version_info[:3]), "avg_request_time_to_error": get_request_time_to_error(request), } self.logger.error("Unexpected Error: %s", error_message) diff --git a/kolibri/core/errorreports/migrations/0002_alter_errorreports_category.py b/kolibri/core/errorreports/migrations/0002_alter_errorreports_category.py new file mode 100644 index 00000000000..1650c79e017 --- /dev/null +++ b/kolibri/core/errorreports/migrations/0002_alter_errorreports_category.py @@ -0,0 +1,25 @@ +# Generated by Django 3.2.25 on 2024-09-26 01:05 +from django.db import migrations +from django.db import models + + +class Migration(migrations.Migration): + + dependencies = [ + ("errorreports", "0001_initial"), + ] + + operations = [ + migrations.AlterField( + model_name="errorreports", + name="category", + field=models.CharField( + choices=[ + ("frontend", "Frontend"), + ("backend", "Backend"), + ("task", "Task"), + ], + max_length=10, + ), + ), + ] diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/errorreports/models.py index 1e7318fbea4..af3df365236 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/errorreports/models.py @@ -4,11 +4,8 @@ from django.db import models from django.utils import timezone -from .constants import BACKEND -from .constants import FRONTEND from .constants import POSSIBLE_ERRORS -from .schemas import context_backend_schema -from .schemas import context_frontend_schema +from .schemas import SCHEMA_MAP from kolibri.core.fields import JSONField from kolibri.core.utils.validators import JSON_Schema_Validator from kolibri.deployment.default.sqlite_db_names import ERROR_REPORTS @@ -69,10 +66,10 @@ def __str__(self): return f"{self.error_message} ({self.category})" def clean(self): - if self.category == FRONTEND: - JSON_Schema_Validator(context_frontend_schema)(self.context) - elif self.category == BACKEND: - JSON_Schema_Validator(context_backend_schema)(self.context) + schema = SCHEMA_MAP.get(self.category, None) + if schema is None: + raise ValueError("Category not found in SCHEMA_MAP") + JSON_Schema_Validator(schema)(self.context) def save(self, *args, **kwargs): self.full_clean() @@ -85,13 +82,13 @@ def insert_or_update_error(cls, category, error_message, traceback, context): "ErrorReports: Database not updated, as DEVELOPER_MODE is True." ) return - error_report, _ = cls.objects.get_or_create( + error_report, created = cls.objects.get_or_create( category=category, error_message=error_message, traceback=traceback, defaults={"context": context}, ) - if error_report is not None: + if not created: error_report.events += 1 error_report.last_occurred = timezone.now() if error_report.context.get("avg_request_time_to_error", None): diff --git a/kolibri/core/errorreports/schemas.py b/kolibri/core/errorreports/schemas.py index cf8328d9711..4210d40983a 100644 --- a/kolibri/core/errorreports/schemas.py +++ b/kolibri/core/errorreports/schemas.py @@ -1,3 +1,8 @@ +from .constants import BACKEND +from .constants import FRONTEND +from .constants import TASK + + context_frontend_schema = { "type": "object", "definitions": { @@ -64,3 +69,40 @@ "avg_request_time_to_error": {"type": "number", "optional": True}, }, } + +context_task_schema = { + "type": "object", + "properties": { + "job_info": { + "type": "object", + "properties": { + "job_id": {"type": "string", "optional": True}, + "func": {"type": "string", "optional": True}, + "facility_id": {"type": ["string", "null"], "optional": True}, + "args": {"type": "array", "optional": True}, + "kwargs": {"type": "object", "optional": True}, + "progress": {"type": "integer", "optional": True}, + "total_progress": {"type": "integer", "optional": True}, + "extra_metadata": {"type": "object", "optional": True}, + }, + }, + "worker_info": { + "type": "object", + "properties": { + "worker_host": {"type": ["string", "null"], "optional": True}, + "worker_process": {"type": ["string", "null"], "optional": True}, + "worker_thread": {"type": ["string", "null"], "optional": True}, + "worker_extra": {"type": ["string", "null"], "optional": True}, + }, + }, + "packages": {"type": "array", "optional": True}, + "python_version": {"type": "string", "optional": True}, + }, +} + + +SCHEMA_MAP = { + FRONTEND: context_frontend_schema, + BACKEND: context_backend_schema, + TASK: context_task_schema, +} diff --git a/kolibri/core/errorreports/test/test_models.py b/kolibri/core/errorreports/test/test_models.py index 03c65910153..98f525494fe 100644 --- a/kolibri/core/errorreports/test/test_models.py +++ b/kolibri/core/errorreports/test/test_models.py @@ -4,6 +4,7 @@ from ..constants import BACKEND from ..constants import FRONTEND +from ..constants import TASK from kolibri.core.errorreports.models import ErrorReports @@ -41,6 +42,24 @@ def setUp(self): "packages": ["django==3.2", "kolibri==0.15.8"], "python_version": "3.9.1", } + self.context_task = { + "job_info": { + "job_id": "1", + "func": "test_func", + "facility_id": None, + "args": ["test"], + "kwargs": {"test": "test"}, + "progress": 0, + "total_progress": 0, + "extra_metadata": {}, + }, + "worker_info": { + "worker_host": "localhost", + "worker_process": "1", + "worker_thread": "1", + "worker_extra": None, + }, + } def create_error( self, @@ -59,7 +78,7 @@ def create_error( ) @override_settings(DEVELOPER_MODE=False) - def test_insert_or_update_error_prod_mode(self): + def test_insert_or_update_frontend_error_prod_mode(self): error = ErrorReports.insert_or_update_error( self.category_frontend, self.error_message, @@ -99,6 +118,88 @@ def test_insert_or_update_error_prod_mode(self): timezone.now() - error.last_occurred, timezone.timedelta(seconds=1) ) + @override_settings(DEVELOPER_MODE=False) + def test_insert_or_update_backend_error_prod_mode(self): + error = ErrorReports.insert_or_update_error( + self.category_backend, + self.error_message, + self.traceback, + self.context_backend, + ) + self.assertEqual(error.category, self.category_backend) + self.assertEqual(error.error_message, self.error_message) + self.assertEqual(error.traceback, self.traceback) + self.assertEqual(error.context, self.context_backend) + self.assertEqual(error.events, 1) + self.assertFalse(error.reported) + self.assertLess( + timezone.now() - error.first_occurred, timezone.timedelta(seconds=1) + ) + self.assertLess( + timezone.now() - error.last_occurred, timezone.timedelta(seconds=1) + ) + + # Creating the error again, so this time it should update the error + error = ErrorReports.insert_or_update_error( + self.category_backend, + self.error_message, + self.traceback, + self.context_backend, + ) + self.assertEqual(error.category, self.category_backend) + self.assertEqual(error.error_message, self.error_message) + self.assertEqual(error.traceback, self.traceback) + self.assertEqual(error.context, self.context_backend) + self.assertEqual(error.events, 2) + self.assertFalse(error.reported) + self.assertLess( + timezone.now() - error.first_occurred, timezone.timedelta(seconds=1) + ) + self.assertLess( + timezone.now() - error.last_occurred, timezone.timedelta(seconds=1) + ) + + @override_settings(DEVELOPER_MODE=False) + def test_insert_or_update_task_error_prod_mode(self): + error = ErrorReports.insert_or_update_error( + TASK, + self.error_message, + self.traceback, + self.context_task, + ) + self.assertEqual(error.category, TASK) + self.assertEqual(error.error_message, self.error_message) + self.assertEqual(error.traceback, self.traceback) + self.assertEqual(error.context, self.context_task) + self.assertEqual(error.events, 1) + self.assertFalse(error.reported) + self.assertLess( + timezone.now() - error.first_occurred, timezone.timedelta(seconds=1) + ) + self.assertLess( + timezone.now() - error.last_occurred, timezone.timedelta(seconds=1) + ) + + # Creating the error again, so this time it should update the error + error = ErrorReports.insert_or_update_error( + TASK, + self.error_message, + self.traceback, + self.context_task, + ) + self.assertEqual(error.category, TASK) + self.assertEqual(error.error_message, self.error_message) + self.assertEqual(error.traceback, self.traceback) + self.assertEqual(error.context, self.context_task) + self.assertEqual(error.events, 2) + self.assertFalse(error.reported) + self.assertLess( + timezone.now() - error.first_occurred, timezone.timedelta(seconds=1) + ) + self.assertLess( + timezone.now() - error.last_occurred, timezone.timedelta(seconds=1) + ) + @override_settings(DEVELOPER_MODE=True) def test_insert_or_update_error_dev_mode(self): error = ErrorReports.insert_or_update_error( diff --git a/kolibri/plugins/error_reports/kolibri_plugin.py b/kolibri/plugins/error_reports/kolibri_plugin.py index 50e9cbdc524..fb5914718b3 100644 --- a/kolibri/plugins/error_reports/kolibri_plugin.py +++ b/kolibri/plugins/error_reports/kolibri_plugin.py @@ -1,8 +1,15 @@ +import logging + +from kolibri.core.errorreports.constants import TASK from kolibri.core.hooks import FrontEndBaseSyncHook +from kolibri.core.tasks.hooks import StorageHook +from kolibri.core.tasks.job import State from kolibri.core.webpack.hooks import WebpackBundleHook from kolibri.plugins import KolibriPluginBase from kolibri.plugins.hooks import register_hook +logger = logging.getLogger(__name__) + class ErrorReportsPlugin(KolibriPluginBase): """ @@ -20,3 +27,45 @@ class ErrorReportsPluginAsset(WebpackBundleHook): @register_hook class ErrorReportsPluginInclusionHook(FrontEndBaseSyncHook): bundle_class = ErrorReportsPluginAsset + + +@register_hook +class ErrorReportsPluginStorageHook(StorageHook): + def schedule(self, job, orm_job): + pass + + def update(self, job, orm_job, state=None, **kwargs): + if state == State.FAILED: + # Importing here to avoid importing models at the top level + from kolibri.core.errorreports.middleware import get_packages + from kolibri.core.errorreports.middleware import get_python_version + from kolibri.core.errorreports.models import ErrorReports + + ErrorReports.insert_or_update_error( + TASK, + job.exception, + job.traceback, + context={ + "job_info": { + "job_id": job.job_id, + "func": job.func, + "facility_id": job.facility_id, + "args": job.args, + "kwargs": job.kwargs, + "progress": job.progress, + "total_progress": job.total_progress, + "extra_metadata": job.extra_metadata, + }, + "worker_info": { + "worker_host": orm_job.worker_host, + "worker_process": orm_job.worker_process, + "worker_thread": orm_job.worker_thread, + "worker_extra": orm_job.worker_extra, + }, + "packages": get_packages(), + "python_version": get_python_version(), + }, + ) + + def clear(self, job, orm_job): + pass From f9968885397a66c374fda5e3ed442f179ccbee12 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 26 Sep 2024 08:26:32 -0700 Subject: [PATCH 74/74] Refactor to standardize naming of core app and model. --- kolibri/core/analytics/tasks.py | 2 +- .../__init__.py | 0 kolibri/core/error_reports/apps.py | 7 ++ .../constants.py | 0 .../migrations/0001_initial.py | 10 +- .../migrations/__init__.py | 0 .../{errorreports => error_reports}/models.py | 20 ++-- .../schemas.py | 0 .../{errorreports => error_reports}/tasks.py | 4 +- .../test/__init__.py | 0 .../test/test_middleware.py | 12 +- .../test/test_models.py | 22 ++-- .../test/test_tasks.py | 28 ++--- kolibri/core/errorreports/apps.py | 7 -- kolibri/core/errorreports/middleware.py | 108 ------------------ .../0002_alter_errorreports_category.py | 25 ---- kolibri/deployment/default/settings/base.py | 8 +- kolibri/deployment/default/sqlite_db_names.py | 2 +- kolibri/plugins/error_reports/api.py | 6 +- .../plugins/error_reports/kolibri_plugin.py | 10 +- kolibri/plugins/error_reports/serializers.py | 4 +- .../plugins/error_reports/test/test_api.py | 4 +- 22 files changed, 75 insertions(+), 204 deletions(-) rename kolibri/core/{errorreports => error_reports}/__init__.py (100%) create mode 100644 kolibri/core/error_reports/apps.py rename kolibri/core/{errorreports => error_reports}/constants.py (100%) rename kolibri/core/{errorreports => error_reports}/migrations/0001_initial.py (82%) rename kolibri/core/{errorreports => error_reports}/migrations/__init__.py (100%) rename kolibri/core/{errorreports => error_reports}/models.py (84%) rename kolibri/core/{errorreports => error_reports}/schemas.py (100%) rename kolibri/core/{errorreports => error_reports}/tasks.py (95%) rename kolibri/core/{errorreports => error_reports}/test/__init__.py (100%) rename kolibri/core/{errorreports => error_reports}/test/test_middleware.py (87%) rename kolibri/core/{errorreports => error_reports}/test/test_models.py (93%) rename kolibri/core/{errorreports => error_reports}/test/test_tasks.py (72%) delete mode 100644 kolibri/core/errorreports/apps.py delete mode 100644 kolibri/core/errorreports/middleware.py delete mode 100644 kolibri/core/errorreports/migrations/0002_alter_errorreports_category.py diff --git a/kolibri/core/analytics/tasks.py b/kolibri/core/analytics/tasks.py index aecc8d4a283..4864e361592 100644 --- a/kolibri/core/analytics/tasks.py +++ b/kolibri/core/analytics/tasks.py @@ -7,7 +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.error_reports.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 diff --git a/kolibri/core/errorreports/__init__.py b/kolibri/core/error_reports/__init__.py similarity index 100% rename from kolibri/core/errorreports/__init__.py rename to kolibri/core/error_reports/__init__.py diff --git a/kolibri/core/error_reports/apps.py b/kolibri/core/error_reports/apps.py new file mode 100644 index 00000000000..64bd043a1ad --- /dev/null +++ b/kolibri/core/error_reports/apps.py @@ -0,0 +1,7 @@ +from django.apps import AppConfig + + +class KolibriErrorConfig(AppConfig): + name = "kolibri.core.error_reports" + label = "error_reports" + verbose_name = "Kolibri Error Reports" diff --git a/kolibri/core/errorreports/constants.py b/kolibri/core/error_reports/constants.py similarity index 100% rename from kolibri/core/errorreports/constants.py rename to kolibri/core/error_reports/constants.py diff --git a/kolibri/core/errorreports/migrations/0001_initial.py b/kolibri/core/error_reports/migrations/0001_initial.py similarity index 82% rename from kolibri/core/errorreports/migrations/0001_initial.py rename to kolibri/core/error_reports/migrations/0001_initial.py index ee57340edd7..b1feb4e183a 100644 --- a/kolibri/core/errorreports/migrations/0001_initial.py +++ b/kolibri/core/error_reports/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-09-14 14:20 +# Generated by Django 3.2.25 on 2024-09-26 01:14 import django.utils.timezone from django.db import migrations from django.db import models @@ -14,7 +14,7 @@ class Migration(migrations.Migration): operations = [ migrations.CreateModel( - name="ErrorReports", + name="ErrorReport", fields=[ ( "id", @@ -28,7 +28,11 @@ class Migration(migrations.Migration): ( "category", models.CharField( - choices=[("frontend", "Frontend"), ("backend", "Backend")], + choices=[ + ("frontend", "Frontend"), + ("backend", "Backend"), + ("task", "Task"), + ], max_length=10, ), ), diff --git a/kolibri/core/errorreports/migrations/__init__.py b/kolibri/core/error_reports/migrations/__init__.py similarity index 100% rename from kolibri/core/errorreports/migrations/__init__.py rename to kolibri/core/error_reports/migrations/__init__.py diff --git a/kolibri/core/errorreports/models.py b/kolibri/core/error_reports/models.py similarity index 84% rename from kolibri/core/errorreports/models.py rename to kolibri/core/error_reports/models.py index af3df365236..190b017de38 100644 --- a/kolibri/core/errorreports/models.py +++ b/kolibri/core/error_reports/models.py @@ -20,28 +20,28 @@ class ErrorReportsRouter(object): """ def db_for_read(self, model, **hints): - if model._meta.app_label == "errorreports": + if model._meta.app_label == "error_reports": return ERROR_REPORTS return None def db_for_write(self, model, **hints): - if model._meta.app_label == "errorreports": + if model._meta.app_label == "error_reports": return ERROR_REPORTS return None def allow_relation(self, obj1, obj2, **hints): if ( - obj1._meta.app_label == "errorreports" - and obj2._meta.app_label == "errorreports" + obj1._meta.app_label == "error_reports" + and obj2._meta.app_label == "error_reports" ): return True - elif "errorreports" not in [obj1._meta.app_label, obj2._meta.app_label]: + elif "error_reports" not in [obj1._meta.app_label, obj2._meta.app_label]: return None return False def allow_migrate(self, db, app_label, model_name=None, **hints): - if app_label == "errorreports": + if app_label == "error_reports": return db == ERROR_REPORTS elif db == ERROR_REPORTS: return False @@ -49,7 +49,7 @@ def allow_migrate(self, db, app_label, model_name=None, **hints): return None -class ErrorReports(models.Model): +class ErrorReport(models.Model): category = models.CharField(max_length=10, choices=POSSIBLE_ERRORS) error_message = models.CharField(max_length=255) traceback = models.TextField() @@ -78,9 +78,7 @@ def save(self, *args, **kwargs): @classmethod def insert_or_update_error(cls, category, error_message, traceback, context): if getattr(settings, "DEVELOPER_MODE", False): - logger.info( - "ErrorReports: Database not updated, as DEVELOPER_MODE is True." - ) + logger.info("ErrorReport: Database not updated, as DEVELOPER_MODE is True.") return error_report, created = cls.objects.get_or_create( category=category, @@ -100,7 +98,7 @@ def insert_or_update_error(cls, category, error_message, traceback, context): error_report.context = context error_report.save() - logger.error("ErrorReports: Database updated.") + logger.error("ErrorReport: Database updated.") return error_report @classmethod diff --git a/kolibri/core/errorreports/schemas.py b/kolibri/core/error_reports/schemas.py similarity index 100% rename from kolibri/core/errorreports/schemas.py rename to kolibri/core/error_reports/schemas.py diff --git a/kolibri/core/errorreports/tasks.py b/kolibri/core/error_reports/tasks.py similarity index 95% rename from kolibri/core/errorreports/tasks.py rename to kolibri/core/error_reports/tasks.py index a54a502557f..c94d442791b 100644 --- a/kolibri/core/errorreports/tasks.py +++ b/kolibri/core/error_reports/tasks.py @@ -8,7 +8,7 @@ from requests.exceptions import RequestException from requests.exceptions import Timeout -from .models import ErrorReports +from .models import ErrorReport from kolibri.core.tasks.decorators import register_task from kolibri.core.utils.urls import join_url @@ -36,7 +36,7 @@ def serialize_error_reports_to_json_response(errors, pingback_id): @register_task def ping_error_reports(server, pingback_id): try: - errors = ErrorReports.get_unreported_errors() + errors = ErrorReport.get_unreported_errors() errors_json = serialize_error_reports_to_json_response(errors, pingback_id) diff --git a/kolibri/core/errorreports/test/__init__.py b/kolibri/core/error_reports/test/__init__.py similarity index 100% rename from kolibri/core/errorreports/test/__init__.py rename to kolibri/core/error_reports/test/__init__.py diff --git a/kolibri/core/errorreports/test/test_middleware.py b/kolibri/core/error_reports/test/test_middleware.py similarity index 87% rename from kolibri/core/errorreports/test/test_middleware.py rename to kolibri/core/error_reports/test/test_middleware.py index f1624dba85b..bfa4a345ef0 100644 --- a/kolibri/core/errorreports/test/test_middleware.py +++ b/kolibri/core/error_reports/test/test_middleware.py @@ -8,7 +8,7 @@ from ..constants import BACKEND from ..middleware import ErrorReportingMiddleware -from ..models import ErrorReports +from ..models import ErrorReport class ErrorReportingMiddlewareTestCase(TestCase): @@ -16,17 +16,17 @@ def setUp(self): self.factory = RequestFactory() @patch( - "kolibri.core.errorreports.middleware.get_request_time_to_error", + "kolibri.core.error_reports.middleware.get_request_time_to_error", return_value=0.0, ) @patch( - "kolibri.core.errorreports.middleware.get_python_version", return_value="3.9.9" + "kolibri.core.error_reports.middleware.get_python_version", return_value="3.9.9" ) @patch( - "kolibri.core.errorreports.middleware.get_packages", + "kolibri.core.error_reports.middleware.get_packages", return_value=["Django==3.2.25"], ) - @patch.object(ErrorReports, "insert_or_update_error") + @patch.object(ErrorReport, "insert_or_update_error") @patch.object(logging.Logger, "error") def test_process_exception( self, @@ -69,7 +69,7 @@ def test_process_exception( }, ) - @patch.object(ErrorReports, "insert_or_update_error") + @patch.object(ErrorReport, "insert_or_update_error") @patch.object(logging.Logger, "error") def test_process_exception_integrity_error( self, mock_logger_error, mock_insert_or_update_error diff --git a/kolibri/core/errorreports/test/test_models.py b/kolibri/core/error_reports/test/test_models.py similarity index 93% rename from kolibri/core/errorreports/test/test_models.py rename to kolibri/core/error_reports/test/test_models.py index 98f525494fe..7bc0af229e0 100644 --- a/kolibri/core/errorreports/test/test_models.py +++ b/kolibri/core/error_reports/test/test_models.py @@ -5,10 +5,10 @@ from ..constants import BACKEND from ..constants import FRONTEND from ..constants import TASK -from kolibri.core.errorreports.models import ErrorReports +from kolibri.core.error_reports.models import ErrorReport -class ErrorReportsTestCase(TestCase): +class ErrorReportTestCase(TestCase): databases = "__all__" def setUp(self): @@ -69,7 +69,7 @@ def create_error( context, reported=False, ): - return ErrorReports.objects.create( + return ErrorReport.objects.create( category=category, error_message=error_message, traceback=traceback, @@ -79,7 +79,7 @@ def create_error( @override_settings(DEVELOPER_MODE=False) def test_insert_or_update_frontend_error_prod_mode(self): - error = ErrorReports.insert_or_update_error( + error = ErrorReport.insert_or_update_error( self.category_frontend, self.error_message, self.traceback, @@ -99,7 +99,7 @@ def test_insert_or_update_frontend_error_prod_mode(self): ) # Creating the error again, so this time it should update the error - error = ErrorReports.insert_or_update_error( + error = ErrorReport.insert_or_update_error( self.category_frontend, self.error_message, self.traceback, @@ -120,7 +120,7 @@ def test_insert_or_update_frontend_error_prod_mode(self): @override_settings(DEVELOPER_MODE=False) def test_insert_or_update_backend_error_prod_mode(self): - error = ErrorReports.insert_or_update_error( + error = ErrorReport.insert_or_update_error( self.category_backend, self.error_message, self.traceback, @@ -140,7 +140,7 @@ def test_insert_or_update_backend_error_prod_mode(self): ) # Creating the error again, so this time it should update the error - error = ErrorReports.insert_or_update_error( + error = ErrorReport.insert_or_update_error( self.category_backend, self.error_message, self.traceback, @@ -161,7 +161,7 @@ def test_insert_or_update_backend_error_prod_mode(self): @override_settings(DEVELOPER_MODE=False) def test_insert_or_update_task_error_prod_mode(self): - error = ErrorReports.insert_or_update_error( + error = ErrorReport.insert_or_update_error( TASK, self.error_message, self.traceback, @@ -181,7 +181,7 @@ def test_insert_or_update_task_error_prod_mode(self): ) # Creating the error again, so this time it should update the error - error = ErrorReports.insert_or_update_error( + error = ErrorReport.insert_or_update_error( TASK, self.error_message, self.traceback, @@ -202,7 +202,7 @@ def test_insert_or_update_task_error_prod_mode(self): @override_settings(DEVELOPER_MODE=True) def test_insert_or_update_error_dev_mode(self): - error = ErrorReports.insert_or_update_error( + error = ErrorReport.insert_or_update_error( self.category_backend, self.error_message, self.traceback, @@ -234,7 +234,7 @@ def test_get_unreported_errors(self): ) # Get unreported errors, should be only 2 as out of 3, 1 is reported - unreported_errors = ErrorReports.get_unreported_errors() + unreported_errors = ErrorReport.get_unreported_errors() self.assertEqual(unreported_errors.count(), 2) self.assertFalse(unreported_errors[0].reported) self.assertFalse(unreported_errors[1].reported) diff --git a/kolibri/core/errorreports/test/test_tasks.py b/kolibri/core/error_reports/test/test_tasks.py similarity index 72% rename from kolibri/core/errorreports/test/test_tasks.py rename to kolibri/core/error_reports/test/test_tasks.py index a880dba2d05..b10477733e3 100644 --- a/kolibri/core/errorreports/test/test_tasks.py +++ b/kolibri/core/error_reports/test/test_tasks.py @@ -6,15 +6,15 @@ from requests.exceptions import RequestException from requests.exceptions import Timeout -from kolibri.core.errorreports.models import ErrorReports -from kolibri.core.errorreports.tasks import ping_error_reports +from kolibri.core.error_reports.models import ErrorReport +from kolibri.core.error_reports.tasks import ping_error_reports class TestPingErrorReports(TestCase): databases = "__all__" def setUp(self): - ErrorReports.objects.create( + ErrorReport.objects.create( category="frontend", error_message="Test Error", traceback="Test Traceback", @@ -33,7 +33,7 @@ def setUp(self): }, }, ) - ErrorReports.objects.create( + ErrorReport.objects.create( category="backend", error_message="Test Error", traceback="Test Traceback", @@ -52,9 +52,9 @@ def setUp(self): }, ) - @patch("kolibri.core.errorreports.tasks.requests.post") + @patch("kolibri.core.error_reports.tasks.requests.post") @patch( - "kolibri.core.errorreports.tasks.serialize_error_reports_to_json_response", + "kolibri.core.error_reports.tasks.serialize_error_reports_to_json_response", return_value="[]", ) def test_ping_error_reports(self, mock_serializer, mock_post): @@ -64,24 +64,26 @@ def test_ping_error_reports(self, mock_serializer, mock_post): data="[]", headers={"Content-Type": "application/json"}, ) - self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 2) + self.assertEqual(ErrorReport.objects.filter(reported=True).count(), 2) - @patch("kolibri.core.errorreports.tasks.requests.post", side_effect=ConnectionError) + @patch( + "kolibri.core.error_reports.tasks.requests.post", side_effect=ConnectionError + ) def test_ping_error_reports_connection_error(self, mock_post): with pytest.raises(ConnectionError): ping_error_reports("http://testserver", "test-pingback-id") - self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 0) + self.assertEqual(ErrorReport.objects.filter(reported=True).count(), 0) - @patch("kolibri.core.errorreports.tasks.requests.post", side_effect=Timeout) + @patch("kolibri.core.error_reports.tasks.requests.post", side_effect=Timeout) def test_ping_error_reports_timeout(self, mock_post): with pytest.raises(Timeout): ping_error_reports("http://testserver", "test-pingback-id") - self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 0) + self.assertEqual(ErrorReport.objects.filter(reported=True).count(), 0) @patch( - "kolibri.core.errorreports.tasks.requests.post", side_effect=RequestException + "kolibri.core.error_reports.tasks.requests.post", side_effect=RequestException ) def test_ping_error_reports_request_exception(self, mock_post): with pytest.raises(RequestException): ping_error_reports("http://testserver", "test-pingback-id") - self.assertEqual(ErrorReports.objects.filter(reported=True).count(), 0) + self.assertEqual(ErrorReport.objects.filter(reported=True).count(), 0) diff --git a/kolibri/core/errorreports/apps.py b/kolibri/core/errorreports/apps.py deleted file mode 100644 index ae71f967170..00000000000 --- a/kolibri/core/errorreports/apps.py +++ /dev/null @@ -1,7 +0,0 @@ -from django.apps import AppConfig - - -class KolibriErrorConfig(AppConfig): - name = "kolibri.core.errorreports" - label = "errorreports" - verbose_name = "Kolibri ErrorReports" diff --git a/kolibri/core/errorreports/middleware.py b/kolibri/core/errorreports/middleware.py deleted file mode 100644 index 5cb57637891..00000000000 --- a/kolibri/core/errorreports/middleware.py +++ /dev/null @@ -1,108 +0,0 @@ -import logging -import time -import traceback -from sys import version_info - -if version_info < (3, 10): - from importlib_metadata import distributions -else: - from importlib.metadata import distributions - -from django.core.exceptions import MiddlewareNotUsed -from django.core.exceptions import ValidationError -from django.db import IntegrityError - -from .constants import BACKEND -from .models import ErrorReports - -from kolibri.plugins.error_reports.kolibri_plugin import ErrorReportsPlugin -from kolibri.plugins.registry import registered_plugins - - -def get_request_info(request): - # checked the codebase and found these are the sensitive headers - request_headers = dict(request.headers) - request_headers.pop("X-Csrftoken", None) - request_headers.pop("Cookie", None) - - request_get = dict(request.GET) - request_get.pop("token", None) - - return { - "url": request.build_absolute_uri(), - "method": request.method, - "headers": request_headers, - "body": request.body.decode("utf-8"), - "query_params": request_get, - } - - -def get_server_info(request): - return {"host": request.get_host(), "port": request.get_port()} - - -def get_packages(): - packages = [f"{dist.metadata['Name']}=={dist.version}" for dist in distributions()] - return packages - - -def get_python_version(): - return ".".join(str(v) for v in version_info[:3]) - - -def get_request_time_to_error(request): - return time.time() - request.start_time - - -class ErrorReportingMiddleware: - """ - Middleware to log exceptions to the database. - """ - - def __init__(self, get_response): - if ErrorReportsPlugin not in registered_plugins: - raise MiddlewareNotUsed("ErrorReportsPlugin is not enabled.") - self.get_response = get_response - self.logger = logging.getLogger(__name__) - - def __call__(self, request): - response = self.get_response(request) - return response - - def process_exception(self, request, exception): - error_message = str(exception) - traceback_info = traceback.format_exc() - context = { - "request_info": get_request_info(request), - "server": get_server_info(request), - "packages": get_packages(), - "python_version": ".".join(str(v) for v in version_info[:3]), - "avg_request_time_to_error": get_request_time_to_error(request), - } - self.logger.error("Unexpected Error: %s", error_message) - try: - self.logger.error("Saving error report to the database.") - ErrorReports.insert_or_update_error( - BACKEND, - error_message, - traceback_info, - context, - ) - except (IntegrityError, ValidationError) as e: - self.logger.error( - "Error occurred while saving error report to the database: %s", str(e) - ) - - -class PreRequestMiddleware: - def __init__(self, get_response): - if ErrorReportsPlugin not in registered_plugins: - raise MiddlewareNotUsed("ErrorReportsPlugin is not enabled.") - self.get_response = get_response - - def __call__(self, request): - response = self.get_response(request) - return response - - def process_view(self, request, view_func, view_args, view_kwargs): - request.start_time = time.time() diff --git a/kolibri/core/errorreports/migrations/0002_alter_errorreports_category.py b/kolibri/core/errorreports/migrations/0002_alter_errorreports_category.py deleted file mode 100644 index 1650c79e017..00000000000 --- a/kolibri/core/errorreports/migrations/0002_alter_errorreports_category.py +++ /dev/null @@ -1,25 +0,0 @@ -# Generated by Django 3.2.25 on 2024-09-26 01:05 -from django.db import migrations -from django.db import models - - -class Migration(migrations.Migration): - - dependencies = [ - ("errorreports", "0001_initial"), - ] - - operations = [ - migrations.AlterField( - model_name="errorreports", - name="category", - field=models.CharField( - choices=[ - ("frontend", "Frontend"), - ("backend", "Backend"), - ("task", "Task"), - ], - max_length=10, - ), - ), - ] diff --git a/kolibri/deployment/default/settings/base.py b/kolibri/deployment/default/settings/base.py index 882984941f9..06cb70dbe95 100644 --- a/kolibri/deployment/default/settings/base.py +++ b/kolibri/deployment/default/settings/base.py @@ -76,7 +76,7 @@ "kolibri.core.auth.apps.KolibriAuthConfig", "kolibri.core.bookmarks", "kolibri.core.content", - "kolibri.core.errorreports", + "kolibri.core.error_reports", "kolibri.core.logger", "kolibri.core.notifications.apps.KolibriNotificationsConfig", "kolibri.core.tasks.apps.KolibriTasksConfig", @@ -104,8 +104,8 @@ "django.middleware.common.CommonMiddleware", "django.middleware.csrf.CsrfViewMiddleware", "kolibri.core.auth.middleware.CustomAuthenticationMiddleware", - "kolibri.core.errorreports.middleware.ErrorReportingMiddleware", - "kolibri.core.errorreports.middleware.PreRequestMiddleware", + "kolibri.core.error_reports.middleware.ErrorReportingMiddleware", + "kolibri.core.error_reports.middleware.PreRequestMiddleware", "django.contrib.messages.middleware.MessageMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware", "django.middleware.security.SecurityMiddleware", @@ -167,7 +167,7 @@ "kolibri.core.notifications.models.NotificationsRouter", "kolibri.core.device.models.SyncQueueRouter", "kolibri.core.discovery.models.NetworkLocationRouter", - "kolibri.core.errorreports.models.ErrorReportsRouter", + "kolibri.core.error_reports.models.ErrorReportsRouter", ) elif conf.OPTIONS["Database"]["DATABASE_ENGINE"] == "postgres": diff --git a/kolibri/deployment/default/sqlite_db_names.py b/kolibri/deployment/default/sqlite_db_names.py index 46f54ea8885..d8cae6a8989 100644 --- a/kolibri/deployment/default/sqlite_db_names.py +++ b/kolibri/deployment/default/sqlite_db_names.py @@ -10,7 +10,7 @@ NOTIFICATIONS = "notifications" -ERROR_REPORTS = "errorreports" +ERROR_REPORTS = "error_reports" ADDITIONAL_SQLITE_DATABASES = ( diff --git a/kolibri/plugins/error_reports/api.py b/kolibri/plugins/error_reports/api.py index db6fdf7c61d..e8ac8def089 100644 --- a/kolibri/plugins/error_reports/api.py +++ b/kolibri/plugins/error_reports/api.py @@ -6,8 +6,8 @@ from rest_framework.response import Response from .serializers import ErrorReportSerializer -from kolibri.core.errorreports.constants import FRONTEND -from kolibri.core.errorreports.models import ErrorReports +from kolibri.core.error_reports.constants import FRONTEND +from kolibri.core.error_reports.models import ErrorReport logger = logging.getLogger(__name__) @@ -19,7 +19,7 @@ def report(request): if serializer.is_valid(): data = serializer.validated_data try: - error = ErrorReports.insert_or_update_error( + error = ErrorReport.insert_or_update_error( FRONTEND, data["error_message"], data["traceback"], diff --git a/kolibri/plugins/error_reports/kolibri_plugin.py b/kolibri/plugins/error_reports/kolibri_plugin.py index fb5914718b3..6ac41135543 100644 --- a/kolibri/plugins/error_reports/kolibri_plugin.py +++ b/kolibri/plugins/error_reports/kolibri_plugin.py @@ -1,6 +1,6 @@ import logging -from kolibri.core.errorreports.constants import TASK +from kolibri.core.error_reports.constants import TASK from kolibri.core.hooks import FrontEndBaseSyncHook from kolibri.core.tasks.hooks import StorageHook from kolibri.core.tasks.job import State @@ -37,11 +37,11 @@ def schedule(self, job, orm_job): def update(self, job, orm_job, state=None, **kwargs): if state == State.FAILED: # Importing here to avoid importing models at the top level - from kolibri.core.errorreports.middleware import get_packages - from kolibri.core.errorreports.middleware import get_python_version - from kolibri.core.errorreports.models import ErrorReports + from kolibri.core.error_reports.middleware import get_packages + from kolibri.core.error_reports.middleware import get_python_version + from kolibri.core.error_reports.models import ErrorReport - ErrorReports.insert_or_update_error( + ErrorReport.insert_or_update_error( TASK, job.exception, job.traceback, diff --git a/kolibri/plugins/error_reports/serializers.py b/kolibri/plugins/error_reports/serializers.py index 880e0551137..80a8e78ed3c 100644 --- a/kolibri/plugins/error_reports/serializers.py +++ b/kolibri/plugins/error_reports/serializers.py @@ -1,11 +1,11 @@ from rest_framework import serializers -from kolibri.core.errorreports.models import ErrorReports +from kolibri.core.error_reports.models import ErrorReport class ErrorReportSerializer(serializers.ModelSerializer): context = serializers.JSONField() class Meta: - model = ErrorReports + model = ErrorReport fields = ["error_message", "traceback", "context"] diff --git a/kolibri/plugins/error_reports/test/test_api.py b/kolibri/plugins/error_reports/test/test_api.py index 04ae67825e1..519444297e9 100644 --- a/kolibri/plugins/error_reports/test/test_api.py +++ b/kolibri/plugins/error_reports/test/test_api.py @@ -58,7 +58,7 @@ def test_frontend_report_invalid_data(self): self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST) @patch( - "kolibri.core.errorreports.models.ErrorReports.insert_or_update_error", + "kolibri.core.error_reports.models.ErrorReport.insert_or_update_error", side_effect=AttributeError("Mocked AttributeError"), ) def test_frontend_report_server_error_attribute_error( @@ -70,7 +70,7 @@ def test_frontend_report_server_error_attribute_error( self.assertIn("error", response.data) @patch( - "kolibri.core.errorreports.models.ErrorReports.insert_or_update_error", + "kolibri.core.error_reports.models.ErrorReport.insert_or_update_error", side_effect=ValidationError("Mocked exception"), ) def test_frontend_report_server_error_validation_error(