From a163d6f7ed650ceb1f2a8458feabbfe2ec7a0472 Mon Sep 17 00:00:00 2001
From: burnettk <burnettk@users.noreply.github.com>
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/src/spiffworkflow_backend/exceptions/api_error.py b/src/spiffworkflow_backend/exceptions/api_error.py
index ab5bf1c3..46d2ad54 100644
--- a/src/spiffworkflow_backend/exceptions/api_error.py
+++ b/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