From ae774105e31ff36b9ab833763d358bc5dbe621b2 Mon Sep 17 00:00:00 2001 From: Chris Weaver <25087905+Weves@users.noreply.github.com> Date: Wed, 19 Mar 2025 11:26:49 -0700 Subject: [PATCH] Fix slack connector creation (#4303) * Make it fail fast + succeed validation if rate limiting is happening * Add logging + reduce spam --- backend/onyx/connectors/slack/connector.py | 26 ++++++++++++++----- backend/onyx/server/manage/slack_bot.py | 7 ++++- .../channels/SlackChannelConfigFormFields.tsx | 4 +++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/backend/onyx/connectors/slack/connector.py b/backend/onyx/connectors/slack/connector.py index 138c3b422..66da040d9 100644 --- a/backend/onyx/connectors/slack/connector.py +++ b/backend/onyx/connectors/slack/connector.py @@ -480,6 +480,7 @@ def _process_message( class SlackConnector(SlimConnector, CheckpointConnector): MAX_WORKERS = 2 + FAST_TIMEOUT = 1 def __init__( self, @@ -493,7 +494,7 @@ class SlackConnector(SlimConnector, CheckpointConnector): self.channel_regex_enabled = channel_regex_enabled self.batch_size = batch_size self.client: WebClient | None = None - + self.fast_client: WebClient | None = None # just used for efficiency self.text_cleaner: SlackTextCleaner | None = None self.user_cache: dict[str, BasicExpertInfo | None] = {} @@ -501,6 +502,10 @@ class SlackConnector(SlimConnector, CheckpointConnector): def load_credentials(self, credentials: dict[str, Any]) -> dict[str, Any] | None: bot_token = credentials["slack_bot_token"] self.client = WebClient(token=bot_token) + # use for requests that must return quickly (e.g. realtime flows where user is waiting) + self.fast_client = WebClient( + token=bot_token, timeout=SlackConnector.FAST_TIMEOUT + ) self.text_cleaner = SlackTextCleaner(client=self.client) return None @@ -676,12 +681,12 @@ class SlackConnector(SlimConnector, CheckpointConnector): 2. Ensure the bot has enough scope to list channels. 3. Check that every channel specified in self.channels exists (only when regex is not enabled). """ - if self.client is None: + if self.fast_client is None: raise ConnectorMissingCredentialError("Slack credentials not loaded.") try: # 1) Validate connection to workspace - auth_response = self.client.auth_test() + auth_response = self.fast_client.auth_test() if not auth_response.get("ok", False): error_msg = auth_response.get( "error", "Unknown error from Slack auth_test" @@ -689,7 +694,7 @@ class SlackConnector(SlimConnector, CheckpointConnector): raise ConnectorValidationError(f"Failed Slack auth_test: {error_msg}") # 2) Minimal test to confirm listing channels works - test_resp = self.client.conversations_list( + test_resp = self.fast_client.conversations_list( limit=1, types=["public_channel"] ) if not test_resp.get("ok", False): @@ -709,7 +714,7 @@ class SlackConnector(SlimConnector, CheckpointConnector): # 3) If channels are specified and regex is not enabled, verify each is accessible if self.channels and not self.channel_regex_enabled: accessible_channels = get_channels( - client=self.client, + client=self.fast_client, exclude_archived=True, get_public=True, get_private=True, @@ -729,7 +734,16 @@ class SlackConnector(SlimConnector, CheckpointConnector): except SlackApiError as e: slack_error = e.response.get("error", "") - if slack_error == "missing_scope": + if slack_error == "ratelimited": + # Handle rate limiting specifically + retry_after = int(e.response.headers.get("Retry-After", 1)) + logger.warning( + f"Slack API rate limited during validation. Retry suggested after {retry_after} seconds. " + "Proceeding with validation, but be aware that connector operations might be throttled." + ) + # Continue validation without failing - the connector is likely valid but just rate limited + return + elif slack_error == "missing_scope": raise InsufficientPermissionsError( "Slack bot token lacks the necessary scope to list/access channels. " "Please ensure your Slack app has 'channels:read' (and/or 'groups:read' for private channels)." diff --git a/backend/onyx/server/manage/slack_bot.py b/backend/onyx/server/manage/slack_bot.py index d47f8d828..0da66a5f6 100644 --- a/backend/onyx/server/manage/slack_bot.py +++ b/backend/onyx/server/manage/slack_bot.py @@ -32,10 +32,14 @@ from onyx.server.manage.models import SlackChannelConfig from onyx.server.manage.models import SlackChannelConfigCreationRequest from onyx.server.manage.validate_tokens import validate_app_token from onyx.server.manage.validate_tokens import validate_bot_token +from onyx.utils.logger import setup_logger from onyx.utils.telemetry import create_milestone_and_report from shared_configs.contextvars import get_current_tenant_id +logger = setup_logger() + + router = APIRouter(prefix="/manage") @@ -376,7 +380,7 @@ def get_all_channels_from_slack_api( status_code=404, detail="Bot token not found for the given bot ID" ) - client = WebClient(token=tokens["bot_token"]) + client = WebClient(token=tokens["bot_token"], timeout=1) all_channels = [] next_cursor = None current_page = 0 @@ -431,6 +435,7 @@ def get_all_channels_from_slack_api( except SlackApiError as e: # Handle rate limiting or other API errors + logger.exception("Error fetching channels from Slack API") raise HTTPException( status_code=500, detail=f"Error fetching channels from Slack API: {str(e)}", diff --git a/web/src/app/admin/bots/[bot-id]/channels/SlackChannelConfigFormFields.tsx b/web/src/app/admin/bots/[bot-id]/channels/SlackChannelConfigFormFields.tsx index 1e82f04b9..9222205b8 100644 --- a/web/src/app/admin/bots/[bot-id]/channels/SlackChannelConfigFormFields.tsx +++ b/web/src/app/admin/bots/[bot-id]/channels/SlackChannelConfigFormFields.tsx @@ -184,6 +184,10 @@ export function SlackChannelConfigFormFields({ name: channel.name, value: channel.id, })); + }, + { + shouldRetryOnError: false, // don't spam the Slack API + dedupingInterval: 60000, // Limit re-fetching to once per minute } );