Fix slack connector creation (#4303)

* Make it fail fast + succeed validation if rate limiting is happening

* Add logging + reduce spam
This commit is contained in:
Chris Weaver 2025-03-19 11:26:49 -07:00 committed by GitHub
parent 4dafc3aa6d
commit ae774105e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 30 additions and 7 deletions

View File

@ -480,6 +480,7 @@ def _process_message(
class SlackConnector(SlimConnector, CheckpointConnector): class SlackConnector(SlimConnector, CheckpointConnector):
MAX_WORKERS = 2 MAX_WORKERS = 2
FAST_TIMEOUT = 1
def __init__( def __init__(
self, self,
@ -493,7 +494,7 @@ class SlackConnector(SlimConnector, CheckpointConnector):
self.channel_regex_enabled = channel_regex_enabled self.channel_regex_enabled = channel_regex_enabled
self.batch_size = batch_size self.batch_size = batch_size
self.client: WebClient | None = None self.client: WebClient | None = None
self.fast_client: WebClient | None = None
# just used for efficiency # just used for efficiency
self.text_cleaner: SlackTextCleaner | None = None self.text_cleaner: SlackTextCleaner | None = None
self.user_cache: dict[str, BasicExpertInfo | 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: def load_credentials(self, credentials: dict[str, Any]) -> dict[str, Any] | None:
bot_token = credentials["slack_bot_token"] bot_token = credentials["slack_bot_token"]
self.client = WebClient(token=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) self.text_cleaner = SlackTextCleaner(client=self.client)
return None return None
@ -676,12 +681,12 @@ class SlackConnector(SlimConnector, CheckpointConnector):
2. Ensure the bot has enough scope to list channels. 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). 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.") raise ConnectorMissingCredentialError("Slack credentials not loaded.")
try: try:
# 1) Validate connection to workspace # 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): if not auth_response.get("ok", False):
error_msg = auth_response.get( error_msg = auth_response.get(
"error", "Unknown error from Slack auth_test" "error", "Unknown error from Slack auth_test"
@ -689,7 +694,7 @@ class SlackConnector(SlimConnector, CheckpointConnector):
raise ConnectorValidationError(f"Failed Slack auth_test: {error_msg}") raise ConnectorValidationError(f"Failed Slack auth_test: {error_msg}")
# 2) Minimal test to confirm listing channels works # 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"] limit=1, types=["public_channel"]
) )
if not test_resp.get("ok", False): 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 # 3) If channels are specified and regex is not enabled, verify each is accessible
if self.channels and not self.channel_regex_enabled: if self.channels and not self.channel_regex_enabled:
accessible_channels = get_channels( accessible_channels = get_channels(
client=self.client, client=self.fast_client,
exclude_archived=True, exclude_archived=True,
get_public=True, get_public=True,
get_private=True, get_private=True,
@ -729,7 +734,16 @@ class SlackConnector(SlimConnector, CheckpointConnector):
except SlackApiError as e: except SlackApiError as e:
slack_error = e.response.get("error", "") 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( raise InsufficientPermissionsError(
"Slack bot token lacks the necessary scope to list/access channels. " "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)." "Please ensure your Slack app has 'channels:read' (and/or 'groups:read' for private channels)."

View File

@ -32,10 +32,14 @@ from onyx.server.manage.models import SlackChannelConfig
from onyx.server.manage.models import SlackChannelConfigCreationRequest 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_app_token
from onyx.server.manage.validate_tokens import validate_bot_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 onyx.utils.telemetry import create_milestone_and_report
from shared_configs.contextvars import get_current_tenant_id from shared_configs.contextvars import get_current_tenant_id
logger = setup_logger()
router = APIRouter(prefix="/manage") 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" 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 = [] all_channels = []
next_cursor = None next_cursor = None
current_page = 0 current_page = 0
@ -431,6 +435,7 @@ def get_all_channels_from_slack_api(
except SlackApiError as e: except SlackApiError as e:
# Handle rate limiting or other API errors # Handle rate limiting or other API errors
logger.exception("Error fetching channels from Slack API")
raise HTTPException( raise HTTPException(
status_code=500, status_code=500,
detail=f"Error fetching channels from Slack API: {str(e)}", detail=f"Error fetching channels from Slack API: {str(e)}",

View File

@ -184,6 +184,10 @@ export function SlackChannelConfigFormFields({
name: channel.name, name: channel.name,
value: channel.id, value: channel.id,
})); }));
},
{
shouldRetryOnError: false, // don't spam the Slack API
dedupingInterval: 60000, // Limit re-fetching to once per minute
} }
); );