fix: Guess most likely exception source (#21827)
This commit is contained in:
parent
35c929afdb
commit
6e94cd2eb9
5 changed files with 91 additions and 6 deletions
|
|
@ -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 []
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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 += "<br>";
|
||||
dialog_html += `Possible source of error: ${data._exc_source.bold()} `;
|
||||
}
|
||||
|
||||
frappe.error_dialog.$body.html(dialog_html);
|
||||
frappe.error_dialog.show();
|
||||
}
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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 `<serverscript>`
|
||||
|
||||
"""
|
||||
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<app_name>\w+)/\1/")
|
||||
SERVER_SCRIPT_FRAME = re.compile(r".*<serverscript>")
|
||||
|
||||
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)"
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue