Fix Slack premature Reacts and Notification (#571)

backend/danswer/bots/slack/handlers/handle_message.py

@@ -3,6 +3,7 @@
 
 from retry import retry
 from slack_sdk import WebClient
+from slack_sdk.errors import SlackApiError
 from sqlalchemy.orm import Session
 
 from danswer.bots.slack.blocks import build_documents_blocks
@@ -19,7 +20,9 @@
 from danswer.configs.danswerbot_configs import DANSWER_BOT_DISABLE_DOCS_ONLY_ANSWER
 from danswer.configs.danswerbot_configs import DANSWER_BOT_DISPLAY_ERROR_MSGS
 from danswer.configs.danswerbot_configs import DANSWER_BOT_NUM_RETRIES
+from danswer.configs.danswerbot_configs import DANSWER_REACT_EMOJI
 from danswer.configs.danswerbot_configs import ENABLE_DANSWERBOT_REFLEXION
+from danswer.connectors.slack.utils import make_slack_api_rate_limited
 from danswer.db.engine import get_sqlalchemy_engine
 from danswer.db.models import SlackBotConfig
 from danswer.direct_qa.answer_question import answer_qa_query
@@ -30,6 +33,37 @@
 logger_base = setup_logger()
 
 
+def send_msg_ack_to_user(details: SlackMessageInfo, client: WebClient) -> None:
+    if details.is_bot_msg and details.sender:
+        respond_in_thread(
+            client=client,
+            channel=details.channel_to_respond,
+            thread_ts=details.msg_to_respond,
+            receiver_ids=[details.sender],
+            text="Hi, we're evaluating your query :face_with_monocle:",
+        )
+        return
+
+    slack_call = make_slack_api_rate_limited(client.reactions_add)
+    slack_call(
+        name=DANSWER_REACT_EMOJI,
+        channel=details.channel_to_respond,
+        timestamp=details.msg_to_respond,
+    )
+
+
+def remove_react(details: SlackMessageInfo, client: WebClient) -> None:
+    if details.is_bot_msg:
+        return
+
+    slack_call = make_slack_api_rate_limited(client.reactions_remove)
+    slack_call(
+        name=DANSWER_REACT_EMOJI,
+        channel=details.channel_to_respond,
+        timestamp=details.msg_to_respond,
+    )
+
+
 def handle_message(
     message_info: SlackMessageInfo,
     channel_config: SlackBotConfig | None,
@@ -114,6 +148,11 @@ def handle_message(
                 thread_ts=None,
             )
 
+    try:
+        send_msg_ack_to_user(message_info, client)
+    except SlackApiError as e:
+        logger.error(f"Was not able to react to user message due to: {e}")
+
     @retry(
         tries=num_retries,
         delay=0.25,
@@ -137,7 +176,9 @@ def _get_answer(question: QuestionRequest) -> QAResponse:
             else:
                 raise RuntimeError(answer.error_msg)
 
+    answer_failed = False
     try:
+        # This includes throwing out answer via reflexion
         answer = _get_answer(
             QuestionRequest(
                 query=msg,
@@ -150,6 +191,7 @@ def _get_answer(question: QuestionRequest) -> QAResponse:
             )
         )
     except Exception as e:
+        answer_failed = True
         logger.exception(
             f"Unable to process message - did not successfully answer "
             f"in {num_retries} attempts"
@@ -164,6 +206,13 @@ def _get_answer(question: QuestionRequest) -> QAResponse:
                 text=f"Encountered exception when trying to answer: \n\n```{e}```",
                 thread_ts=message_ts_to_respond_to,
             )
+
+    try:
+        remove_react(message_info, client)
+    except SlackApiError as e:
+        logger.error(f"Failed to remove Reaction due to: {e}")
+
+    if answer_failed:
         return True
 
     if answer.eval_res_valid is False:
@@ -195,8 +244,6 @@ def _get_answer(question: QuestionRequest) -> QAResponse:
         )
         return True
 
-    # convert raw response into "nicely" formatted Slack message
-
     # If called with the DanswerBot slash command, the question is lost so we have to reshow it
     restate_question_block = get_restate_blocks(msg, is_bot_msg)
 
@@ -215,7 +262,7 @@ def _get_answer(question: QuestionRequest) -> QAResponse:
             client=client,
             channel=channel,
             receiver_ids=send_to,
-            text="Something has gone wrong! The Slack blocks failed to load...",
+            text="Hello! Danswer has some results for you!",
             blocks=restate_question_block + answer_blocks + document_blocks,
             thread_ts=message_ts_to_respond_to,
             # don't unfurl, since otherwise we will have 5+ previews which makes the message very long

backend/danswer/bots/slack/listener.py

@@ -4,7 +4,6 @@
 from typing import cast
 
 from slack_sdk import WebClient
-from slack_sdk.errors import SlackApiError
 from slack_sdk.socket_mode import SocketModeClient
 from slack_sdk.socket_mode.request import SocketModeRequest
 from slack_sdk.socket_mode.response import SocketModeResponse
@@ -21,9 +20,7 @@
 from danswer.bots.slack.utils import get_channel_name_from_id
 from danswer.bots.slack.utils import respond_in_thread
 from danswer.configs.danswerbot_configs import DANSWER_BOT_RESPOND_EVERY_CHANNEL
-from danswer.configs.danswerbot_configs import DANSWER_REACT_EMOJI
 from danswer.configs.danswerbot_configs import NOTIFY_SLACKBOT_NO_ANSWER
-from danswer.connectors.slack.utils import make_slack_api_rate_limited
 from danswer.db.engine import get_sqlalchemy_engine
 from danswer.dynamic_configs.interface import ConfigNotFoundError
 from danswer.utils.logger import setup_logger
@@ -207,37 +204,6 @@ def build_request_details(
     raise RuntimeError("Programming fault, this should never happen.")
 
 
-def send_msg_ack_to_user(details: SlackMessageInfo, client: SocketModeClient) -> None:
-    if details.is_bot_msg and details.sender:
-        respond_in_thread(
-            client=client.web_client,
-            channel=details.channel_to_respond,
-            thread_ts=details.msg_to_respond,
-            receiver_ids=[details.sender],
-            text="Hi, we're evaluating your query :face_with_monocle:",
-        )
-        return
-
-    slack_call = make_slack_api_rate_limited(client.web_client.reactions_add)
-    slack_call(
-        name=DANSWER_REACT_EMOJI,
-        channel=details.channel_to_respond,
-        timestamp=details.msg_to_respond,
-    )
-
-
-def remove_react(details: SlackMessageInfo, client: SocketModeClient) -> None:
-    if details.is_bot_msg:
-        return
-
-    slack_call = make_slack_api_rate_limited(client.web_client.reactions_remove)
-    slack_call(
-        name=DANSWER_REACT_EMOJI,
-        channel=details.channel_to_respond,
-        timestamp=details.msg_to_respond,
-    )
-
-
 def apologize_for_fail(
     details: SlackMessageInfo,
     client: SocketModeClient,
@@ -264,7 +230,7 @@ def process_message(
 
     details = build_request_details(req, client)
     channel = details.channel_to_respond
-    channel_name = get_channel_name_from_id(
+    channel_name, is_dm = get_channel_name_from_id(
         client=client.web_client, channel_id=channel
     )
 
@@ -279,18 +245,13 @@ def process_message(
         if (
             slack_bot_config is None
             and not respond_every_channel
-            # DMs are unnamed, don't filter those out
-            and channel_name is not None
+            # Can't have configs for DMs so don't toss them out
+            and not is_dm
             # If @DanswerBot or /DanswerBot, always respond with the default configs
             and not (details.is_bot_msg or details.bipass_filters)
         ):
             return
 
-        try:
-            send_msg_ack_to_user(details, client)
-        except SlackApiError as e:
-            logger.error(f"Was not able to react to user message due to: {e}")
-
         failed = handle_message(
             message_info=details,
             channel_config=slack_bot_config,
@@ -301,11 +262,6 @@ def process_message(
         if failed and notify_no_answer:
             apologize_for_fail(details, client)
 
-        try:
-            remove_react(details, client)
-        except SlackApiError as e:
-            logger.error(f"Failed to remove Reaction due to: {e}")
-
 
 def acknowledge_message(req: SocketModeRequest, client: SocketModeClient) -> None:
     response = SocketModeResponse(envelope_id=req.envelope_id)

backend/danswer/bots/slack/utils.py

@@ -172,12 +172,17 @@ def get_channel_from_id(client: WebClient, channel_id: str) -> dict[str, Any]:
     return response["channel"]
 
 
-def get_channel_name_from_id(client: WebClient, channel_id: str) -> str | None:
+def get_channel_name_from_id(
+    client: WebClient, channel_id: str
+) -> tuple[str | None, bool]:
     try:
-        return get_channel_from_id(client, channel_id).get("name")
-    except SlackApiError:
-        # Private channels such as DMs don't have a name
-        return None
+        channel_info = get_channel_from_id(client, channel_id)
+        name = channel_info.get("name")
+        is_dm = any([channel_info.get("is_im"), channel_info.get("is_mpim")])
+        return name, is_dm
+    except SlackApiError as e:
+        logger.exception(f"Couldn't fetch channel name from id: {channel_id}")
+        raise e
 
 
 def fetch_userids_from_emails(user_emails: list[str], client: WebClient) -> list[str]: