From 1e9ede40fa89b4f4ad859cd17aa72e4b3ef94530 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 14 Apr 2023 14:19:01 +0530 Subject: [PATCH] fix: stale `frappe.local` (#20695) * fix: stale `frappe.local` Co-Authored-By: Aditya Hase * fix: force re-init in request To ensure that any one bad request can not completely cause recurring loop of broken requests due to bad locals, we just force-init locals on every request. --------- Co-authored-by: Aditya Hase --- frappe/__init__.py | 4 ++-- frappe/app.py | 22 ++++++++++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 9d7befe2d1..4d67afe492 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -182,9 +182,9 @@ if TYPE_CHECKING: # end: static analysis hack -def init(site: str, sites_path: str = ".", new_site: bool = False) -> None: +def init(site: str, sites_path: str = ".", new_site: bool = False, force=False) -> None: """Initialize frappe for the current site. Reset thread locals `frappe.local`""" - if getattr(local, "initialised", None): + if getattr(local, "initialised", None) and not force: return local.error_log = [] diff --git a/frappe/app.py b/frappe/app.py index a647b251c8..fab8facd3f 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -74,12 +74,18 @@ def application(request: Request): rollback = sync_database(rollback) finally: + # Important note: + # this function *must* always return a response, hence any exception thrown outside of + # try..catch block like this finally block needs to be handled appropriately. + if request.method in UNSAFE_HTTP_METHODS and frappe.db and rollback: frappe.db.rollback() - if getattr(frappe.local, "initialised", False): - for after_request_task in frappe.get_hooks("after_request"): - frappe.call(after_request_task, response=response, request=request) + try: + run_after_request_hooks(request, response) + except Exception as e: + # We can not handle exceptions safely here. + frappe.logger().error("Failed to run after request hook", exc_info=True) log_request(request, response) process_response(response) @@ -89,12 +95,20 @@ def application(request: Request): return response +def run_after_request_hooks(request, response): + if not getattr(frappe.local, "initialised", False): + return + + for after_request_task in frappe.get_hooks("after_request"): + frappe.call(after_request_task, response=response, request=request) + + def init_request(request): frappe.local.request = request frappe.local.is_ajax = frappe.get_request_header("X-Requested-With") == "XMLHttpRequest" site = _site or request.headers.get("X-Frappe-Site-Name") or get_site_name(request.host) - frappe.init(site=site, sites_path=_sites_path) + frappe.init(site=site, sites_path=_sites_path, force=True) if not (frappe.local.conf and frappe.local.conf.db_name): # site does not exist