From b42b95135c9802a4089bb4e68c4de642b05c2b75 Mon Sep 17 00:00:00 2001 From: Marc Vilanova Date: Thu, 21 Aug 2025 15:14:36 -0700 Subject: [PATCH 1/4] fix(remove user): handles user removal failures more gracefully --- src/dispatch/case/flows.py | 46 +++++++++++++----- src/dispatch/incident/flows.py | 22 ++++++++- .../dispatch_slack/case/interactive.py | 34 ++++++++++++-- src/dispatch/plugins/dispatch_slack/plugin.py | 47 ++++++++++++++++--- 4 files changed, 125 insertions(+), 24 deletions(-) diff --git a/src/dispatch/case/flows.py b/src/dispatch/case/flows.py index d92758f1e6cb..f358eca41822 100644 --- a/src/dispatch/case/flows.py +++ b/src/dispatch/case/flows.py @@ -197,20 +197,42 @@ def case_remove_participant_flow( log.warning("No conversation enabled for this case.") return - slack_conversation_plugin.instance.remove_user( - conversation_id=case.conversation.channel_id, - user_email=user_email - ) + try: + slack_conversation_plugin.instance.remove_user( + conversation_id=case.conversation.channel_id, + user_email=user_email + ) - event_service.log_case_event( - db_session=db_session, - source=slack_conversation_plugin.plugin.title, - description=f"{user_email} removed from conversation (channel ID: {case.conversation.channel_id})", - case_id=case.id, - type=EventType.participant_updated, - ) + event_service.log_case_event( + db_session=db_session, + source=slack_conversation_plugin.plugin.title, + description=f"{user_email} removed from conversation (channel ID: {case.conversation.channel_id})", + case_id=case.id, + type=EventType.participant_updated, + ) - log.info(f"Removed {user_email} from conversation in channel {case.conversation.channel_id}") + log.info(f"Removed {user_email} from conversation in channel {case.conversation.channel_id}") + + except Exception as slack_error: + # Check if this is a users_not_found error from Slack + error_msg = str(slack_error) + if "users_not_found" in error_msg: + log.warning( + f"User {user_email} not found in Slack workspace. " + f"They may have been deactivated or never had access. " + f"Case conversation: {case.conversation.channel_id}" + ) + # Still log the event to maintain audit trail + event_service.log_case_event( + db_session=db_session, + source=slack_conversation_plugin.plugin.title, + description=f"Attempted to remove {user_email} from conversation but user not found in Slack", + case_id=case.id, + type=EventType.participant_updated, + ) + else: + # Re-raise for other Slack errors + raise except Exception as e: log.exception(f"Failed to remove user from Slack conversation: {e}") diff --git a/src/dispatch/incident/flows.py b/src/dispatch/incident/flows.py index e9c63bd3fb30..12b8c94d0717 100644 --- a/src/dispatch/incident/flows.py +++ b/src/dispatch/incident/flows.py @@ -1206,5 +1206,23 @@ def incident_remove_participant_flow( log.info(f"Removed {user_email} from conversation in channel {incident.conversation.channel_id}") - except Exception as e: - log.exception(f"Failed to remove user from Slack conversation: {e}") + except Exception as slack_error: + # Check if this is a users_not_found error from Slack + error_msg = str(slack_error) + if "users_not_found" in error_msg: + log.warning( + f"User {user_email} not found in Slack workspace. " + f"They may have been deactivated or never had access. " + f"Incident conversation: {incident.conversation.channel_id}" + ) + # Still log the event to maintain audit trail + event_service.log_incident_event( + db_session=db_session, + source=slack_conversation_plugin.plugin.title, + description=f"Attempted to remove {user_email} from conversation but user not found in Slack", + incident_id=incident.id, + type=EventType.participant_updated, + ) + else: + # Re-raise for other Slack errors + raise diff --git a/src/dispatch/plugins/dispatch_slack/case/interactive.py b/src/dispatch/plugins/dispatch_slack/case/interactive.py index 3b63186c3fa8..7ee536c724f6 100644 --- a/src/dispatch/plugins/dispatch_slack/case/interactive.py +++ b/src/dispatch/plugins/dispatch_slack/case/interactive.py @@ -469,7 +469,23 @@ def engage( return engagement = form_data[DefaultBlockIds.description_input] - user = client.users_lookupByEmail(email=user_email) + + try: + user = client.users_lookupByEmail(email=user_email) + except SlackApiError as e: + if e.response["error"] == SlackAPIErrorCode.USERS_NOT_FOUND: + log.warning( + f"Failed to find Slack user for email {user_email}. " + "User may have been deactivated or never had Slack access." + ) + client.chat_postMessage( + text=f"Unable to engage user {user_email} - user not found in Slack workspace.", + channel=case.conversation.channel_id, + thread_ts=case.conversation.thread_id if case.has_thread else None, + ) + return + else: + raise result = client.chat_postMessage( text="Engaging user...", @@ -1983,9 +1999,19 @@ def edit_button_click( ack() case = case_service.get(db_session=db_session, case_id=int(context["subject"].id)) - assignee_initial_user = client.users_lookupByEmail(email=case.assignee.individual.email)[ - "user" - ]["id"] + try: + assignee_initial_user = client.users_lookupByEmail(email=case.assignee.individual.email)[ + "user" + ]["id"] + except SlackApiError as e: + if e.response["error"] == SlackAPIErrorCode.USERS_NOT_FOUND: + log.warning( + f"Assignee {case.assignee.individual.email} not found in Slack workspace. " + "Using None for initial assignee selection." + ) + assignee_initial_user = None + else: + raise blocks = [ title_input(initial_value=case.title), diff --git a/src/dispatch/plugins/dispatch_slack/plugin.py b/src/dispatch/plugins/dispatch_slack/plugin.py index 8b47fe63d15d..8436456b8304 100644 --- a/src/dispatch/plugins/dispatch_slack/plugin.py +++ b/src/dispatch/plugins/dispatch_slack/plugin.py @@ -390,13 +390,48 @@ def set_description(self, conversation_id: str, description: str): return set_conversation_description(client, conversation_id, description) def remove_user(self, conversation_id: str, user_email: str): - """Removes a user from a conversation.""" + """Removes a user from a conversation. + + Args: + conversation_id: The Slack conversation/channel ID + user_email: The email address of the user to remove + + Returns: + The API response if successful, None if user not found + + Raises: + SlackApiError: For non-recoverable Slack API errors + """ client = create_slack_client(self.configuration) - user_id = resolve_user(client, user_email).get("id") - if user_id: - return remove_member_from_channel( - client=client, conversation_id=conversation_id, user_id=user_id - ) + + try: + user_info = resolve_user(client, user_email) + user_id = user_info.get("id") + + if user_id: + return remove_member_from_channel( + client=client, conversation_id=conversation_id, user_id=user_id + ) + else: + logger.warning( + "Cannot remove user %s from conversation %s: " + "User ID not found in resolve_user response", + user_email, conversation_id + ) + return None + + except SlackApiError as e: + if e.response.get("error") == SlackAPIErrorCode.USERS_NOT_FOUND: + logger.warning( + "User %s not found in Slack workspace. " + "Cannot remove from conversation %s. " + "User may have been deactivated or never had Slack access.", + user_email, conversation_id + ) + return None + else: + # Re-raise for other Slack API errors + raise def add_bookmark(self, conversation_id: str, weblink: str, title: str): """Adds a bookmark to the conversation.""" From 1891779d2f6dce9dc87ba0549de33dc29a4180f9 Mon Sep 17 00:00:00 2001 From: Marc Vilanova Date: Thu, 21 Aug 2025 15:24:42 -0700 Subject: [PATCH 2/4] log at plugin level --- src/dispatch/case/flows.py | 66 ++++++++++++---------------------- src/dispatch/incident/flows.py | 42 +++++++--------------- 2 files changed, 34 insertions(+), 74 deletions(-) diff --git a/src/dispatch/case/flows.py b/src/dispatch/case/flows.py index f358eca41822..b794e8ce4182 100644 --- a/src/dispatch/case/flows.py +++ b/src/dispatch/case/flows.py @@ -184,55 +184,33 @@ def case_remove_participant_flow( ) # we also try to remove the user from the Slack conversation - try: - slack_conversation_plugin = plugin_service.get_active_instance( - db_session=db_session, project_id=case.project.id, plugin_type="conversation" - ) - - if not slack_conversation_plugin: - log.warning(f"{user_email} not updated. No conversation plugin enabled.") - return + slack_conversation_plugin = plugin_service.get_active_instance( + db_session=db_session, project_id=case.project.id, plugin_type="conversation" + ) - if not case.conversation: - log.warning("No conversation enabled for this case.") - return + if not slack_conversation_plugin: + log.warning(f"{user_email} not updated. No conversation plugin enabled.") + return - try: - slack_conversation_plugin.instance.remove_user( - conversation_id=case.conversation.channel_id, - user_email=user_email - ) + if not case.conversation: + log.warning("No conversation enabled for this case.") + return - event_service.log_case_event( - db_session=db_session, - source=slack_conversation_plugin.plugin.title, - description=f"{user_email} removed from conversation (channel ID: {case.conversation.channel_id})", - case_id=case.id, - type=EventType.participant_updated, - ) + try: + slack_conversation_plugin.instance.remove_user( + conversation_id=case.conversation.channel_id, + user_email=user_email + ) - log.info(f"Removed {user_email} from conversation in channel {case.conversation.channel_id}") + event_service.log_case_event( + db_session=db_session, + source=slack_conversation_plugin.plugin.title, + description=f"{user_email} removed from conversation (channel ID: {case.conversation.channel_id})", + case_id=case.id, + type=EventType.participant_updated, + ) - except Exception as slack_error: - # Check if this is a users_not_found error from Slack - error_msg = str(slack_error) - if "users_not_found" in error_msg: - log.warning( - f"User {user_email} not found in Slack workspace. " - f"They may have been deactivated or never had access. " - f"Case conversation: {case.conversation.channel_id}" - ) - # Still log the event to maintain audit trail - event_service.log_case_event( - db_session=db_session, - source=slack_conversation_plugin.plugin.title, - description=f"Attempted to remove {user_email} from conversation but user not found in Slack", - case_id=case.id, - type=EventType.participant_updated, - ) - else: - # Re-raise for other Slack errors - raise + log.info(f"Removed {user_email} from conversation in channel {case.conversation.channel_id}") except Exception as e: log.exception(f"Failed to remove user from Slack conversation: {e}") diff --git a/src/dispatch/incident/flows.py b/src/dispatch/incident/flows.py index 12b8c94d0717..76c41e14569c 100644 --- a/src/dispatch/incident/flows.py +++ b/src/dispatch/incident/flows.py @@ -1178,19 +1178,19 @@ def incident_remove_participant_flow( ) # we also try to remove the user from the Slack conversation - try: - slack_conversation_plugin = plugin_service.get_active_instance( - db_session=db_session, project_id=incident.project.id, plugin_type="conversation" - ) + slack_conversation_plugin = plugin_service.get_active_instance( + db_session=db_session, project_id=incident.project.id, plugin_type="conversation" + ) - if not slack_conversation_plugin: - log.warning(f"{user_email} not updated. No conversation plugin enabled.") - return + if not slack_conversation_plugin: + log.warning(f"{user_email} not updated. No conversation plugin enabled.") + return - if not incident.conversation: - log.warning("No conversation enabled for this incident.") - return + if not incident.conversation: + log.warning("No conversation enabled for this incident.") + return + try: slack_conversation_plugin.instance.remove_user( conversation_id=incident.conversation.channel_id, user_email=user_email @@ -1206,23 +1206,5 @@ def incident_remove_participant_flow( log.info(f"Removed {user_email} from conversation in channel {incident.conversation.channel_id}") - except Exception as slack_error: - # Check if this is a users_not_found error from Slack - error_msg = str(slack_error) - if "users_not_found" in error_msg: - log.warning( - f"User {user_email} not found in Slack workspace. " - f"They may have been deactivated or never had access. " - f"Incident conversation: {incident.conversation.channel_id}" - ) - # Still log the event to maintain audit trail - event_service.log_incident_event( - db_session=db_session, - source=slack_conversation_plugin.plugin.title, - description=f"Attempted to remove {user_email} from conversation but user not found in Slack", - incident_id=incident.id, - type=EventType.participant_updated, - ) - else: - # Re-raise for other Slack errors - raise + except Exception as e: + log.exception(f"Failed to remove user from Slack conversation: {e}") From 369d038e6a061d7e81b4a4046fa2da804b4d0fbf Mon Sep 17 00:00:00 2001 From: Marc Vilanova <39573146+mvilanova@users.noreply.github.com> Date: Mon, 25 Aug 2025 11:46:24 -0700 Subject: [PATCH 3/4] Update src/dispatch/plugins/dispatch_slack/case/interactive.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com> --- src/dispatch/plugins/dispatch_slack/case/interactive.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dispatch/plugins/dispatch_slack/case/interactive.py b/src/dispatch/plugins/dispatch_slack/case/interactive.py index 7ee536c724f6..0d78d67d0c97 100644 --- a/src/dispatch/plugins/dispatch_slack/case/interactive.py +++ b/src/dispatch/plugins/dispatch_slack/case/interactive.py @@ -2004,7 +2004,7 @@ def edit_button_click( "user" ]["id"] except SlackApiError as e: - if e.response["error"] == SlackAPIErrorCode.USERS_NOT_FOUND: + if e.response.get("error") == SlackAPIErrorCode.USERS_NOT_FOUND: log.warning( f"Assignee {case.assignee.individual.email} not found in Slack workspace. " "Using None for initial assignee selection." From e06ef80cf5e02f5c506d1138be6abee9c6dfdb3b Mon Sep 17 00:00:00 2001 From: Marc Vilanova <39573146+mvilanova@users.noreply.github.com> Date: Mon, 25 Aug 2025 11:46:30 -0700 Subject: [PATCH 4/4] Update src/dispatch/plugins/dispatch_slack/case/interactive.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com> --- src/dispatch/plugins/dispatch_slack/case/interactive.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dispatch/plugins/dispatch_slack/case/interactive.py b/src/dispatch/plugins/dispatch_slack/case/interactive.py index 0d78d67d0c97..f960ad509f3d 100644 --- a/src/dispatch/plugins/dispatch_slack/case/interactive.py +++ b/src/dispatch/plugins/dispatch_slack/case/interactive.py @@ -473,7 +473,7 @@ def engage( try: user = client.users_lookupByEmail(email=user_email) except SlackApiError as e: - if e.response["error"] == SlackAPIErrorCode.USERS_NOT_FOUND: + if e.response.get("error") == SlackAPIErrorCode.USERS_NOT_FOUND: log.warning( f"Failed to find Slack user for email {user_email}. " "User may have been deactivated or never had Slack access."