From 6e94cd2eb9a500a509331fcbe5eb55ca5c5148a4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 27 Jul 2023 17:30:04 +0530 Subject: [PATCH] fix: Guess most likely exception source (#21827) --- frappe/__init__.py | 6 +-- .../core/doctype/error_log/test_error_log.py | 45 ++++++++++++++++++- frappe/public/js/frappe/request.js | 9 +++- frappe/utils/error.py | 33 ++++++++++++++ frappe/utils/response.py | 4 ++ 5 files changed, 91 insertions(+), 6 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index fd77930816..9a83443b02 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1460,13 +1460,11 @@ def get_all_apps(with_internal_apps=True, sites_path=None): @request_cache -def get_installed_apps(*, _ensure_on_bench=False): +def get_installed_apps(*, _ensure_on_bench=False) -> list[str]: """ Get list of installed apps in current site. - :param sort: [DEPRECATED] Sort installed apps based on the sequence in sites/apps.txt - :param frappe_last: [DEPRECATED] Keep frappe last. Do not use this, reverse the app list instead. - :param ensure_on_bench: Only return apps that are present on bench. + :param _ensure_on_bench: Only return apps that are present on bench. """ if getattr(flags, "in_install_db", True): return [] diff --git a/frappe/core/doctype/error_log/test_error_log.py b/frappe/core/doctype/error_log/test_error_log.py index 3f19a6dd0c..22eeea329e 100644 --- a/frappe/core/doctype/error_log/test_error_log.py +++ b/frappe/core/doctype/error_log/test_error_log.py @@ -1,10 +1,12 @@ # Copyright (c) 2015, Frappe Technologies and Contributors # License: MIT. See LICENSE +from unittest.mock import patch + from ldap3.core.exceptions import LDAPException, LDAPInappropriateAuthenticationResult import frappe from frappe.tests.utils import FrappeTestCase -from frappe.utils.error import _is_ldap_exception +from frappe.utils.error import _is_ldap_exception, guess_exception_source # test_records = frappe.get_test_records('Error Log') @@ -21,3 +23,44 @@ class TestErrorLog(FrappeTestCase): for e in exc: self.assertTrue(_is_ldap_exception(e())) + + +_RAW_EXC = """ + File "apps/frappe/frappe/model/document.py", line 1284, in runner + add_to_return_value(self, fn(self, *args, **kwargs)) + ^^^^^^^^^^^^^^^^^^^^^^^^^ + File "apps/frappe/frappe/model/document.py", line 933, in fn + return method_object(*args, **kwargs) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + File "apps/erpnext/erpnext/selling/doctype/sales_order/sales_order.py", line 58, in onload + raise Exception("what") + Exception: what +""" + +_THROW_EXC = """ + File "apps/frappe/frappe/model/document.py", line 933, in fn + return method_object(*args, **kwargs) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + File "apps/erpnext/erpnext/selling/doctype/sales_order/sales_order.py", line 58, in onload + frappe.throw("what") + File "apps/frappe/frappe/__init__.py", line 550, in throw + msgprint( + File "apps/frappe/frappe/__init__.py", line 518, in msgprint + _raise_exception() + File "apps/frappe/frappe/__init__.py", line 467, in _raise_exception + raise raise_exception(msg) + frappe.exceptions.ValidationError: what +""" + +TEST_EXCEPTIONS = { + "erpnext (app)": _RAW_EXC, + "erpnext (app)": _THROW_EXC, +} + + +class TestExceptionSourceGuessing(FrappeTestCase): + @patch.object(frappe, "get_installed_apps", return_value=["frappe", "erpnext", "3pa"]) + def test_exc_source_guessing(self, _installed_apps): + for source, exc in TEST_EXCEPTIONS.items(): + result = guess_exception_source(exc) + self.assertEqual(result, source) diff --git a/frappe/public/js/frappe/request.js b/frappe/public/js/frappe/request.js index 6278d991c1..7e7e592669 100644 --- a/frappe/public/js/frappe/request.js +++ b/frappe/public/js/frappe/request.js @@ -604,7 +604,14 @@ frappe.request.report_error = function (xhr, request_opts) { let parts = strip(exc).split("\n"); - frappe.error_dialog.$body.html(parts[parts.length - 1]); + let dialog_html = parts[parts.length - 1]; + + if (data._exc_source) { + dialog_html += "
"; + dialog_html += `Possible source of error: ${data._exc_source.bold()} `; + } + + frappe.error_dialog.$body.html(dialog_html); frappe.error_dialog.show(); } }; diff --git a/frappe/utils/error.py b/frappe/utils/error.py index d44bdef47f..47902176ea 100644 --- a/frappe/utils/error.py +++ b/frappe/utils/error.py @@ -3,6 +3,9 @@ import functools import inspect +import re +from collections import Counter +from contextlib import suppress import frappe @@ -126,3 +129,33 @@ def raise_error_on_no_output(error_message, error_type=None, keep_quiet=None): return wrapper_raise_error_on_no_output return decorator_raise_error_on_no_output + + +def guess_exception_source(exception: str) -> str | None: + """Attempts to guess source of error based on traceback. + + E.g. + + - For unhandled exception last python file from apps folder is responsible. + - For frappe.throws the exception source is possibly present after skipping frappe.throw frames + - For server script the file name is `` + + """ + with suppress(Exception): + installed_apps = frappe.get_installed_apps() + app_priority = {app: installed_apps.index(app) for app in installed_apps} + + APP_NAME_REGEX = re.compile(r".*File.*apps/(?P\w+)/\1/") + SERVER_SCRIPT_FRAME = re.compile(r".*") + + apps = Counter() + for line in reversed(exception.splitlines()): + if SERVER_SCRIPT_FRAME.match(line): + return "Server Script" + + if matches := APP_NAME_REGEX.match(line): + app_name = matches.group("app_name") + apps[app_name] += app_priority.get(app_name, 0) + + if probably_source := apps.most_common(1): + return f"{probably_source[0][0]} (app)" diff --git a/frappe/utils/response.py b/frappe/utils/response.py index 0cdfd5b422..3ed3fe96d7 100644 --- a/frappe/utils/response.py +++ b/frappe/utils/response.py @@ -133,10 +133,14 @@ def as_binary(): def make_logs(response=None): """make strings for msgprint and errprint""" + from frappe.utils.error import guess_exception_source + if not response: response = frappe.local.response if frappe.error_log: + if source := guess_exception_source(frappe.local.error_log and frappe.local.error_log[0]["exc"]): + response["_exc_source"] = source response["exc"] = json.dumps([frappe.utils.cstr(d["exc"]) for d in frappe.local.error_log]) if frappe.local.message_log: