From 52686f79cb81525ab75f9465b0a0f9e60a0fe1eb Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 30 Nov 2023 17:06:09 +0530 Subject: [PATCH] refactor: code cleanup - better boot config name - send sentry after - because frappe namespce doesn't exist if it starts first - remove import in app.py because __init__ is always imported so no need. - leave telemetry JS always present, this is used even when telemetry is not enabled. --- frappe/app.py | 6 +----- frappe/hooks.py | 1 + frappe/public/js/sentry.bundle.js | 2 +- frappe/utils/error.py | 10 +++------- frappe/utils/sentry.py | 16 +++------------- frappe/www/app.py | 8 ++------ 6 files changed, 11 insertions(+), 32 deletions(-) diff --git a/frappe/app.py b/frappe/app.py index 99f4bb35a4..5ddabfbfc9 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -22,7 +22,7 @@ import frappe.rate_limiter import frappe.recorder import frappe.utils.response from frappe import _ -from frappe.auth import UNSAFE_HTTP_METHODS, HTTPRequest, validate_auth +from frappe.auth import SAFE_HTTP_METHODS, UNSAFE_HTTP_METHODS, HTTPRequest, validate_auth # noqa from frappe.middlewares import StaticDataMiddleware from frappe.utils import CallbackManager, cint, get_site_name from frappe.utils.data import escape_html @@ -60,10 +60,6 @@ if frappe._tune_gc: import frappe.website.router # Website router import frappe.website.website_generator # web page doctypes - # Import sentry only if DSN is set - if os.getenv("FRAPPE_SENTRY_DSN"): - import frappe.utils.sentry - # end: module pre-loading diff --git a/frappe/hooks.py b/frappe/hooks.py index 3bb3d2f9a6..ec7436ac19 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -29,6 +29,7 @@ app_include_js = [ "form.bundle.js", "controls.bundle.js", "report.bundle.js", + "telemetry.bundle.js", ] app_include_css = [ diff --git a/frappe/public/js/sentry.bundle.js b/frappe/public/js/sentry.bundle.js index 12deee3423..4ba9eba75b 100644 --- a/frappe/public/js/sentry.bundle.js +++ b/frappe/public/js/sentry.bundle.js @@ -1,6 +1,6 @@ import * as Sentry from "@sentry/browser"; Sentry.init({ - dsn: frappe.boot.sentry, + dsn: frappe.boot.sentry_dsn, release: frappe?.boot?.versions?.frappe, }); diff --git a/frappe/utils/error.py b/frappe/utils/error.py index e826f25296..50432a01d5 100644 --- a/frappe/utils/error.py +++ b/frappe/utils/error.py @@ -38,7 +38,7 @@ def log_error( ): """Log error to Error Log""" from frappe.monitor import get_trace_id - from frappe.utils.telemetry import capture + from frappe.utils.sentry import capture_exception # Parameter ALERT: # the title and message may be swapped @@ -68,13 +68,9 @@ def log_error( reference_name=reference_name, trace_id=trace_id, ) - capture("error_logged", "frappe", properties={"title": title, "trace_id": trace_id}) - if frappe.get_system_settings("enable_telemetry"): - from frappe.app import capture_exception - - # Capture exception data if telemetry is enabled - capture_exception(message=f"{title}\n{traceback}") + # Capture exception data if telemetry is enabled + capture_exception(message=f"{title}\n{traceback}") if frappe.flags.read_only or defer_insert: error_log.deferred_insert() diff --git a/frappe/utils/sentry.py b/frappe/utils/sentry.py index baadec6411..1032098abe 100644 --- a/frappe/utils/sentry.py +++ b/frappe/utils/sentry.py @@ -65,19 +65,9 @@ def capture_exception( def add_bootinfo(bootinfo): - """Called from hook, sends DSN so client side can setup error monitoring. - - Config needs to be present in site_config in following format: - - "error_reporting": { - "sentry": { - "dsn": "...", - ... - } - } - """ + """Called from hook, sends DSN so client side can setup error monitoring.""" if not frappe.get_system_settings("enable_telemetry"): return - if sentry_info := os.getenv("FRAPPE_SENTRY_DSN"): - bootinfo.sentry = sentry_info + if sentry_dsn := os.getenv("FRAPPE_SENTRY_DSN"): + bootinfo.sentry_dsn = sentry_dsn diff --git a/frappe/www/app.py b/frappe/www/app.py index 412812f9d7..44a146cc63 100644 --- a/frappe/www/app.py +++ b/frappe/www/app.py @@ -49,12 +49,8 @@ def get_context(context): include_icons = hooks.get("app_include_icons", []) frappe.local.preload_assets["icons"].extend(include_icons) - if frappe.get_system_settings("enable_telemetry"): - if os.getenv("FRAPPE_SENTRY_DSN"): - include_js = ["sentry.bundle.js"] + include_js - - if hasattr(frappe.local, "posthog"): - include_js = ["telemetry.bundle.js"] + include_js + if frappe.get_system_settings("enable_telemetry") and os.getenv("FRAPPE_SENTRY_DSN"): + include_js.append("sentry.bundle.js") context.update( {