From 4e533682ba2ee3d822016ded2222bcbe4453a70a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 13:26:12 +0530 Subject: [PATCH 1/4] feat: get_traceback with context --- frappe/__init__.py | 4 ++-- frappe/utils/__init__.py | 14 ++++++++++---- requirements.txt | 1 + 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 97e605394b..257d02a09a 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -354,11 +354,11 @@ def cache() -> "RedisWrapper": return redis_server -def get_traceback(): +def get_traceback(with_context=False): """Returns error traceback.""" from frappe.utils import get_traceback - return get_traceback() + return get_traceback(with_context=with_context) def errprint(msg): diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 3e62589664..d0dfe44760 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -18,6 +18,7 @@ from typing import Generator, Iterable from urllib.parse import quote, urlparse from redis.exceptions import ConnectionError +from traceback_with_variables import iter_exc_lines from werkzeug.test import Client import frappe @@ -255,7 +256,7 @@ def get_gravatar(email): return gravatar_url -def get_traceback() -> str: +def get_traceback(with_context=False) -> str: """ Returns the traceback of the Exception """ @@ -264,10 +265,15 @@ def get_traceback() -> str: if not any([exc_type, exc_value, exc_tb]): return "" - trace_list = traceback.format_exception(exc_type, exc_value, exc_tb) - bench_path = get_bench_path() + "/" + if with_context: + trace_list = iter_exc_lines() + tb = "\n".join(trace_list) + else: + trace_list = traceback.format_exception(exc_type, exc_value, exc_tb) + tb = "".join(cstr(t) for t in trace_list) - return "".join(cstr(t) for t in trace_list).replace(bench_path, "") + bench_path = get_bench_path() + "/" + return tb.replace(bench_path, "") def log(event, details): diff --git a/requirements.txt b/requirements.txt index c77ab1d424..495574e05b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -63,6 +63,7 @@ semantic-version~=2.8.5 sqlparse~=0.4.1 stripe~=2.56.0 terminaltables~=3.1.0 +traceback-with-variables~=2.0.4 urllib3~=1.26.4 Werkzeug~=2.0.3 Whoosh~=2.7.4 From 5ecc9fe4ffec55a70fde9f4196ce74c15630f758 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 13:27:01 +0530 Subject: [PATCH 2/4] refactor(log_error): de-clutter & log context with traceback --- frappe/__init__.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 257d02a09a..4653845c3e 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -2069,7 +2069,6 @@ def logger( def log_error(title=None, message=None, reference_doctype=None, reference_name=None): """Log error to Error Log""" - # Parameter ALERT: # the title and message may be swapped # the better API for this is log_error(title, message), and used in many cases this way @@ -2082,20 +2081,15 @@ def log_error(title=None, message=None, reference_doctype=None, reference_name=N else: traceback = message - if not traceback: - traceback = get_traceback() - - if not title: - title = "Error" + title = title or "Error" + traceback = as_unicode(traceback or get_traceback(with_context=True)) return get_doc( - dict( - doctype="Error Log", - error=as_unicode(traceback), - method=title, - reference_doctype=reference_doctype, - reference_name=reference_name, - ) + doctype="Error Log", + error=traceback, + method=title, + reference_doctype=reference_doctype, + reference_name=reference_name, ).insert(ignore_permissions=True) From c691537e61b5d2d7f0151e4b13a7842d083bdbbb Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 13:28:49 +0530 Subject: [PATCH 3/4] chore: Add typing for ease of development --- frappe/database/database.py | 2 +- frappe/model/base_document.py | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index d4677a1295..edc57ae543 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1066,7 +1066,7 @@ class Database(object): now_datetime() - relativedelta(minutes=minutes), )[0][0] - def get_db_table_columns(self, table): + def get_db_table_columns(self, table) -> List[str]: """Returns list of column names from given table.""" columns = frappe.cache().hget("table_columns", table) if columns is None: diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 02eb2ab38c..f8d60d0763 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -2,6 +2,7 @@ # License: MIT. See LICENSE import datetime import json +from typing import Dict, List import frappe from frappe import _ @@ -252,7 +253,7 @@ class BaseDocument(object): def get_valid_dict( self, sanitize=True, convert_dates_to_str=False, ignore_nulls=False, ignore_virtual=False - ): + ) -> Dict: d = frappe._dict() for fieldname in self.meta.get_valid_columns(): d[fieldname] = self.get(fieldname) @@ -329,7 +330,7 @@ class BaseDocument(object): if key not in self.__dict__: self.__dict__[key] = None - def get_valid_columns(self): + def get_valid_columns(self) -> List[str]: if self.doctype not in frappe.local.valid_columns: if self.doctype in DOCTYPES_FOR_DOCTYPE: from frappe.model.meta import get_table_columns @@ -342,7 +343,7 @@ class BaseDocument(object): return frappe.local.valid_columns[self.doctype] - def is_new(self): + def is_new(self) -> bool: return self.get("__islocal") @property @@ -359,7 +360,7 @@ class BaseDocument(object): no_default_fields=False, convert_dates_to_str=False, no_child_table_fields=False, - ): + ) -> Dict: doc = self.get_valid_dict(convert_dates_to_str=convert_dates_to_str) doc["doctype"] = self.doctype From 418dcdd2f445d39da4fa01bce94f19f973d27d50 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 21 Apr 2022 13:32:28 +0530 Subject: [PATCH 4/4] fix!: Use event as a differentiator for frappe.utils.log --- frappe/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index d0dfe44760..13e6911634 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -277,7 +277,7 @@ def get_traceback(with_context=False) -> str: def log(event, details): - frappe.logger().info(details) + frappe.logger(event).info(details) def dict_to_str(args, sep="&"):