From eb369384a7645a1e116e86607c76e7f38c8055a3 Mon Sep 17 00:00:00 2001 From: pablonyx Date: Thu, 27 Feb 2025 18:05:28 -0800 Subject: [PATCH] Log server side auth error + slackbot pagination fix (#4149) --- backend/onyx/server/manage/slack_bot.py | 63 ++++++++++++++----- .../channels/SlackChannelConfigFormFields.tsx | 42 ++++++++----- web/src/app/auth/error/layout.tsx | 16 +++++ web/src/app/auth/error/page.tsx | 25 +++++--- 4 files changed, 106 insertions(+), 40 deletions(-) create mode 100644 web/src/app/auth/error/layout.tsx diff --git a/backend/onyx/server/manage/slack_bot.py b/backend/onyx/server/manage/slack_bot.py index 53e0d0bbe..ec79e2c21 100644 --- a/backend/onyx/server/manage/slack_bot.py +++ b/backend/onyx/server/manage/slack_bot.py @@ -343,7 +343,8 @@ def list_bot_configs( ] -MAX_CHANNELS = 200 +MAX_SLACK_PAGES = 5 +SLACK_API_CHANNELS_PER_PAGE = 100 @router.get( @@ -355,8 +356,8 @@ def get_all_channels_from_slack_api( _: User | None = Depends(current_admin_user), ) -> list[SlackChannel]: """ - Fetches all channels from the Slack API. - If the workspace has 200 or more channels, we raise an error. + Fetches channels the bot is a member of from the Slack API. + Handles pagination with a limit to avoid excessive API calls. """ tokens = fetch_slack_bot_tokens(db_session, bot_id) if not tokens or "bot_token" not in tokens: @@ -365,28 +366,60 @@ def get_all_channels_from_slack_api( ) client = WebClient(token=tokens["bot_token"]) + all_channels = [] + next_cursor = None + current_page = 0 try: - response = client.conversations_list( - types="public_channel,private_channel", - exclude_archived=True, - limit=MAX_CHANNELS, - ) + # Use users_conversations with limited pagination + while current_page < MAX_SLACK_PAGES: + current_page += 1 + + # Make API call with cursor if we have one + if next_cursor: + response = client.users_conversations( + types="public_channel,private_channel", + exclude_archived=True, + cursor=next_cursor, + limit=SLACK_API_CHANNELS_PER_PAGE, + ) + else: + response = client.users_conversations( + types="public_channel,private_channel", + exclude_archived=True, + limit=SLACK_API_CHANNELS_PER_PAGE, + ) + + # Add channels to our list + if "channels" in response and response["channels"]: + all_channels.extend(response["channels"]) + + # Check if we need to paginate + if ( + "response_metadata" in response + and "next_cursor" in response["response_metadata"] + ): + next_cursor = response["response_metadata"]["next_cursor"] + if next_cursor: + if current_page == MAX_SLACK_PAGES: + raise HTTPException( + status_code=400, + detail="Workspace has too many channels to paginate over in this call.", + ) + continue + + # If we get here, no more pages + break channels = [ SlackChannel(id=channel["id"], name=channel["name"]) - for channel in response["channels"] + for channel in all_channels ] - if len(channels) == MAX_CHANNELS: - raise HTTPException( - status_code=400, - detail=f"Workspace has {MAX_CHANNELS} or more channels.", - ) - return channels except SlackApiError as e: + # Handle rate limiting or other API errors 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 13a80f6bb..92553b97b 100644 --- a/web/src/app/admin/bots/[bot-id]/channels/SlackChannelConfigFormFields.tsx +++ b/web/src/app/admin/bots/[bot-id]/channels/SlackChannelConfigFormFields.tsx @@ -199,17 +199,17 @@ export function SlackChannelConfigFormFields({ Default Configuration -

+

This default configuration will apply across all Slack channels the bot is added to in the Slack workspace, as well as direct messages (DMs), unless disabled.

-
+
-

+

Warning: Disabling the default configuration means the bot won't respond in Slack channels or DMs unless explicitly configured for them. @@ -238,20 +238,28 @@ export function SlackChannelConfigFormFields({ />

) : ( - - {({ field, form }: { field: any; form: any }) => ( - { - form.setFieldValue("channel_name", selected.name); - }} - initialSearchTerm={field.value} - onSearchTermChange={(term) => { - form.setFieldValue("channel_name", term); - }} - /> - )} - + <> + + {({ field, form }: { field: any; form: any }) => ( + { + form.setFieldValue("channel_name", selected.name); + }} + initialSearchTerm={field.value} + onSearchTermChange={(term) => { + form.setFieldValue("channel_name", term); + }} + /> + )} + +

+ Note: This list shows public and private channels where the + bot is a member (up to 500 channels). If you don't see a + channel, make sure the bot is added to that channel in Slack + first, or type the channel name manually. +

+ )} )} diff --git a/web/src/app/auth/error/layout.tsx b/web/src/app/auth/error/layout.tsx new file mode 100644 index 000000000..f9e9b6315 --- /dev/null +++ b/web/src/app/auth/error/layout.tsx @@ -0,0 +1,16 @@ +export default function AuthErrorLayout({ + children, +}: { + children: React.ReactNode; +}) { + // Log error to console for debugging + console.error( + "Authentication error page was accessed - this should not happen in normal flow" + ); + + // In a production environment, you might want to send this to your error tracking service + // For example, if using a service like Sentry: + // captureException(new Error("Authentication error page was accessed unexpectedly")); + + return <>{children}; +} diff --git a/web/src/app/auth/error/page.tsx b/web/src/app/auth/error/page.tsx index 3bee49841..ec2e01e7b 100644 --- a/web/src/app/auth/error/page.tsx +++ b/web/src/app/auth/error/page.tsx @@ -4,6 +4,7 @@ import AuthFlowContainer from "@/components/auth/AuthFlowContainer"; import { Button } from "@/components/ui/button"; import Link from "next/link"; import { FiLogIn } from "react-icons/fi"; +import { NEXT_PUBLIC_CLOUD_ENABLED } from "@/lib/constants"; const Page = () => { return ( @@ -15,19 +16,21 @@ const Page = () => {

We encountered an issue while attempting to log you in.

-
-

Possible Issues:

+
+

+ Possible Issues: +

    -
  • -
    +
  • +
    Incorrect or expired login credentials
    • -
  • -
    +
  • +
    Temporary authentication system disruption
    • -
  • -
    +
  • +
    Account access restrictions or permissions
@@ -41,6 +44,12 @@ const Page = () => {

We recommend trying again. If you continue to experience problems, please reach out to your system administrator for assistance. + {NEXT_PUBLIC_CLOUD_ENABLED && ( + + A member of our team has been automatically notified about this + issue. + + )}