From b9783ae0edc6206022f8d0f10e3564c6ac22f8b4 Mon Sep 17 00:00:00 2001 From: burnettk Date: Fri, 3 Feb 2023 11:06:40 -0500 Subject: [PATCH] clean up sentry notification and avoid logger.exception when we do not want sentry --- .../exceptions/api_error.py | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py index ab5bf1c3a..46d2ad549 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py @@ -171,18 +171,30 @@ def set_user_sentry_context() -> None: set_tag("username", username) +def should_notify_sentry(exception: Exception) -> bool: + """Determine if we should notify sentry. + + We want to capture_exception to log the exception to sentry, but we don't want to log: + 1. ApiErrors that are just invalid tokens + 2. NotAuthorizedError. we usually call check-permissions before calling an API to + make sure we'll have access, but there are some cases + where it's more convenient to just make the call from the frontend and handle the 403 appropriately. + """ + if isinstance(exception, ApiError): + if exception.error_code == "invalid_token": + return False + if isinstance(exception, NotAuthorizedError): + return False + return True + + @api_error_blueprint.app_errorhandler(Exception) # type: ignore def handle_exception(exception: Exception) -> flask.wrappers.Response: """Handles unexpected exceptions.""" set_user_sentry_context() sentry_link = None - # we want to capture_exception to log the exception to sentry, but we don't want to log: - # 1. ApiErrors that are just invalid tokens - # 2. NotAuthorizedError - if ( - not isinstance(exception, ApiError) or exception.error_code != "invalid_token" - ) and not isinstance(exception, NotAuthorizedError): + if should_notify_sentry(exception): id = capture_exception(exception) if isinstance(exception, ApiError): @@ -198,10 +210,16 @@ def handle_exception(exception: Exception) -> flask.wrappers.Response: f"https://sentry.io/{organization_slug}/{project_slug}/events/{id}" ) - # !!!NOTE!!!: do this after sentry stuff since calling logger.exception - # seems to break the sentry sdk context where we no longer get back - # an event id or send out tags like username - current_app.logger.exception(exception) + # !!!NOTE!!!: do this after sentry stuff since calling logger.exception + # seems to break the sentry sdk context where we no longer get back + # an event id or send out tags like username + current_app.logger.exception(exception) + else: + current_app.logger.error( + f"Received exception: {exception}. Since we do not want this particular" + " exception in sentry, we cannot use logger.exception, so there will be no" + " backtrace. see api_error.py" + ) error_code = "internal_server_error" status_code = 500