From b70ce7b387009868f3ee24955fd0195e39dd7d9b Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Tue, 30 Jun 2026 20:19:42 +0530 Subject: [PATCH] fix(channels): recover Discord/OAuth channel stuck in Connecting (#4299) (#4306) Co-authored-by: M3gA-Mind --- app/src/components/channels/DiscordConfig.tsx | 33 +++- .../channels/__tests__/DiscordConfig.test.tsx | 46 ++++++ .../useOAuthConnectionListener.test.tsx | 152 +++++++++++++++++- app/src/hooks/useOAuthConnectionListener.ts | 111 ++++++++++++- 4 files changed, 335 insertions(+), 7 deletions(-) diff --git a/app/src/components/channels/DiscordConfig.tsx b/app/src/components/channels/DiscordConfig.tsx index 7604151b8..24519adf0 100644 --- a/app/src/components/channels/DiscordConfig.tsx +++ b/app/src/components/channels/DiscordConfig.tsx @@ -219,9 +219,38 @@ const DiscordConfig = ({ definition }: DiscordConfigProps) => { }); if (oauthResponse.result?.oauthUrl) { await openUrl(oauthResponse.result.oauthUrl); + } else { + // No URL means the core couldn't start the flow. Surface it now + // instead of leaving the badge pinned at `connecting` until the + // listener's timeout fires (#4299). + log('oauth_connect returned no oauthUrl for discord'); + dispatch( + setChannelConnectionStatus({ + channel: 'discord', + authMode: spec.mode, + status: 'error', + lastError: t( + 'channels.discord.oauthStartFailed', + "Couldn't start Discord sign-in. Try again." + ), + }) + ); } - } catch { - // best-effort + } catch (err) { + // Opening the browser / starting the flow failed — don't swallow + // it, or the badge hangs at `connecting` (#4299). + log('oauth_connect failed for discord: %o', err); + dispatch( + setChannelConnectionStatus({ + channel: 'discord', + authMode: spec.mode, + status: 'error', + lastError: t( + 'channels.discord.oauthStartFailed', + "Couldn't start Discord sign-in. Try again." + ), + }) + ); } } return; diff --git a/app/src/components/channels/__tests__/DiscordConfig.test.tsx b/app/src/components/channels/__tests__/DiscordConfig.test.tsx index 345009aef..f5e57a85f 100644 --- a/app/src/components/channels/__tests__/DiscordConfig.test.tsx +++ b/app/src/components/channels/__tests__/DiscordConfig.test.tsx @@ -3,8 +3,10 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { FALLBACK_DEFINITIONS } from '../../../lib/channels/definitions'; import { channelConnectionsApi } from '../../../services/api/channelConnectionsApi'; +import { callCoreRpc } from '../../../services/coreRpcClient'; import { upsertChannelConnection } from '../../../store/channelConnectionsSlice'; import { createTestStore, renderWithProviders } from '../../../test/test-utils'; +import { openUrl } from '../../../utils/openUrl'; import DiscordConfig from '../DiscordConfig'; const coreStateMock = vi.hoisted(() => vi.fn(() => ({ snapshot: { sessionToken: 'jwt-abc' } }))); @@ -182,4 +184,48 @@ describe('DiscordConfig', () => { expect(screen.queryByText('Login with OpenHuman')).not.toBeInTheDocument(); expect(screen.getAllByText('Bot Token').length).toBeGreaterThanOrEqual(1); }); + + // #4299: an OAuth init failure (oauth_connect throws, or returns no URL) must + // surface as an error instead of leaving the badge pinned at `connecting`. + it('surfaces an error when oauth_connect throws', async () => { + const store = createTestStore(); + vi.mocked(channelConnectionsApi.connectChannel).mockResolvedValue({ + status: 'pending_auth', + auth_action: 'discord_oauth', + restart_required: false, + }); + vi.mocked(callCoreRpc).mockRejectedValue(new Error('rpc boom')); + + renderWithProviders(, { store }); + + // Auth modes render in definition order: bot_token (0), oauth (1). + fireEvent.click(screen.getAllByRole('button', { name: 'Connect' })[1]); + + await waitFor(() => { + const oauth = store.getState().channelConnections.connections.discord.oauth; + expect(oauth?.status).toBe('error'); + expect(oauth?.lastError).toMatch(/Couldn't start Discord sign-in/); + }); + }); + + it('surfaces an error when oauth_connect returns no oauthUrl', async () => { + const store = createTestStore(); + vi.mocked(channelConnectionsApi.connectChannel).mockResolvedValue({ + status: 'pending_auth', + auth_action: 'discord_oauth', + restart_required: false, + }); + vi.mocked(callCoreRpc).mockResolvedValue({ result: {} }); + + renderWithProviders(, { store }); + + fireEvent.click(screen.getAllByRole('button', { name: 'Connect' })[1]); + + await waitFor(() => { + const oauth = store.getState().channelConnections.connections.discord.oauth; + expect(oauth?.status).toBe('error'); + expect(oauth?.lastError).toMatch(/Couldn't start Discord sign-in/); + }); + expect(openUrl).not.toHaveBeenCalled(); + }); }); diff --git a/app/src/hooks/__tests__/useOAuthConnectionListener.test.tsx b/app/src/hooks/__tests__/useOAuthConnectionListener.test.tsx index 558819d27..0672ab435 100644 --- a/app/src/hooks/__tests__/useOAuthConnectionListener.test.tsx +++ b/app/src/hooks/__tests__/useOAuthConnectionListener.test.tsx @@ -1,13 +1,17 @@ -import { renderHook } from '@testing-library/react'; +import { act, renderHook } from '@testing-library/react'; import type { ReactNode } from 'react'; import { Provider } from 'react-redux'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { store } from '../../store'; import { resetChannelConnectionsState, setChannelConnectionStatus, } from '../../store/channelConnectionsSlice'; +import { + beginDeepLinkAuthProcessing, + completeDeepLinkAuthProcessing, +} from '../../store/deepLinkAuthState'; import { useOAuthConnectionListener } from '../useOAuthConnectionListener'; const wrapper = ({ children }: { children: ReactNode }) => ( @@ -144,3 +148,147 @@ describe('useOAuthConnectionListener (#2128)', () => { expect(store.getState().channelConnections.connections.discord.oauth).toBeUndefined(); }); }); + +// #4299: recover from a `connecting` badge that no OAuth deep link ever resolves +// (Discord rejected the redirect_uri, user cancelled / closed the browser). +const GRACE_MS = 2_500; +const TIMEOUT_MS = 120_000; + +const setConnecting = () => + store.dispatch( + setChannelConnectionStatus({ channel: 'discord', authMode: 'oauth', status: 'connecting' }) + ); + +const oauthStatus = () => store.getState().channelConnections.connections.discord.oauth?.status; + +describe('useOAuthConnectionListener stuck-connecting recovery (#4299)', () => { + beforeEach(() => { + vi.useFakeTimers(); + store.dispatch(resetChannelConnectionsState()); + }); + + afterEach(() => { + completeDeepLinkAuthProcessing(); + vi.useRealTimers(); + store.dispatch(resetChannelConnectionsState()); + }); + + it('flips to error when the user returns (focus) and no deep link arrives', () => { + setConnecting(); + renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { + wrapper, + }); + + act(() => { + window.dispatchEvent(new Event('focus')); + }); + act(() => { + vi.advanceTimersByTime(GRACE_MS); + }); + + const connection = store.getState().channelConnections.connections.discord.oauth; + expect(connection?.status).toBe('error'); + expect(connection?.lastError).toMatch(/Couldn't finish connecting Discord/); + }); + + it('flips to error on the visibilitychange path too', () => { + setConnecting(); + renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { + wrapper, + }); + + act(() => { + document.dispatchEvent(new Event('visibilitychange')); + }); + act(() => { + vi.advanceTimersByTime(GRACE_MS); + }); + + expect(oauthStatus()).toBe('error'); + }); + + it('does NOT flip when a success deep link lands within the grace window', () => { + setConnecting(); + renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { + wrapper, + }); + + act(() => { + window.dispatchEvent(new Event('focus')); + }); + act(() => { + vi.advanceTimersByTime(GRACE_MS - 500); + dispatchOAuthSuccess('discord'); + }); + act(() => { + vi.advanceTimersByTime(GRACE_MS + TIMEOUT_MS); + }); + + expect(oauthStatus()).toBe('connected'); + }); + + it('flips to error after the absolute timeout when the user never returns', () => { + setConnecting(); + renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { + wrapper, + }); + + act(() => { + vi.advanceTimersByTime(TIMEOUT_MS); + }); + act(() => { + vi.advanceTimersByTime(GRACE_MS); + }); + + expect(oauthStatus()).toBe('error'); + }); + + it('does not arm recovery when the channel is not connecting', () => { + store.dispatch( + setChannelConnectionStatus({ channel: 'discord', authMode: 'oauth', status: 'connected' }) + ); + renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { + wrapper, + }); + + act(() => { + window.dispatchEvent(new Event('focus')); + vi.advanceTimersByTime(TIMEOUT_MS + GRACE_MS); + }); + + expect(oauthStatus()).toBe('connected'); + }); + + it('skips recovery while a deep-link auth round-trip is in flight', () => { + setConnecting(); + renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { + wrapper, + }); + + beginDeepLinkAuthProcessing(); + act(() => { + window.dispatchEvent(new Event('focus')); + vi.advanceTimersByTime(GRACE_MS); + }); + + expect(oauthStatus()).toBe('connecting'); + }); + + it('clears timers on unmount so a late timeout cannot mutate state', () => { + setConnecting(); + const { unmount } = renderHook( + () => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), + { wrapper } + ); + + act(() => { + window.dispatchEvent(new Event('focus')); + }); + unmount(); + act(() => { + vi.advanceTimersByTime(TIMEOUT_MS + GRACE_MS); + }); + + expect(oauthStatus()).toBe('connecting'); + }); +}); diff --git a/app/src/hooks/useOAuthConnectionListener.ts b/app/src/hooks/useOAuthConnectionListener.ts index 96f78bf1e..348e75cbc 100644 --- a/app/src/hooks/useOAuthConnectionListener.ts +++ b/app/src/hooks/useOAuthConnectionListener.ts @@ -16,15 +16,25 @@ * * Centralising this means new channels with OAuth auth modes inherit correct * pending-state transitions for free. + * + * It also recovers from the *no deep link ever arrives* case (#4299): when + * Discord rejects the redirect_uri, the user cancels, or the browser lands on + * an error page, neither `oauth:success` nor `oauth:error` fires, so the badge + * would otherwise stay pinned at `connecting` forever. While a channel sits at + * `connecting` this hook watches for the user returning to the app (window + * focus / tab visible) and an absolute timeout, then flips to a retryable + * `error` if no OAuth event resolved it. */ import debug from 'debug'; -import { useEffect } from 'react'; +import { useEffect, useRef } from 'react'; +import { useT } from '../lib/i18n/I18nContext'; import { setChannelConnectionStatus, upsertChannelConnection, } from '../store/channelConnectionsSlice'; -import { useAppDispatch } from '../store/hooks'; +import { getDeepLinkAuthState } from '../store/deepLinkAuthState'; +import { useAppDispatch, useAppSelector } from '../store/hooks'; import type { ChannelAuthMode, ChannelType } from '../types/channels'; const log = debug('channels:oauth-listener'); @@ -35,6 +45,23 @@ const log = debug('channels:oauth-listener'); // render. (CodeRabbit on PR #2256.) const DEFAULT_OAUTH_CAPABILITIES = ['read', 'write'] as const; +// How long a channel may sit at `connecting` with no OAuth deep link before we +// give up and surface a retryable error. Covers the "user never came back to +// the app" path (closed the browser tab on Discord's redirect_uri error page, +// switched away permanently). Mirrors `OAuthProviderButton`'s 300s login +// fallback but shorter — a channel connect that hasn't round-tripped in 2min is +// not coming back. (#4299) +const OAUTH_CONNECTING_TIMEOUT_MS = 120_000; + +// Grace window applied after a recovery trigger (window focus / tab visible / +// absolute timeout) before flipping `connecting` → `error`. A *successful* +// OAuth deep link arrives shortly after the OS refocuses the app, but its +// dispatch is delayed by `focusMainWindow()` + the async app-version gate in +// `desktopDeepLinkListener.handleOAuthDeepLink`. Waiting out this grace lets a +// real success/error event win the race (and cancel the timer) so we never +// flash a false error on the happy path. (#4299) +const OAUTH_RECOVER_GRACE_MS = 2_500; + interface OAuthSuccessDetail { integrationId?: string; toolkit?: string; @@ -73,15 +100,37 @@ export function useOAuthConnectionListener({ capabilitiesOnSuccess = DEFAULT_OAUTH_CAPABILITIES, }: UseOAuthConnectionListenerOptions): void { const dispatch = useAppDispatch(); + const { t } = useT(); + + // Current status for this channel/authMode. Drives whether recovery watchers + // are armed (only while `connecting`). `statusRef` mirrors it so the deferred + // grace callback reads the freshest value without itself being an effect dep. + const status = useAppSelector( + state => state.channelConnections.connections[channel]?.[authMode]?.status + ); + const statusRef = useRef(status); + statusRef.current = status; useEffect(() => { const channelKey = channel.toLowerCase(); + // Shared timers so a real oauth:success / oauth:error can cancel a pending + // recovery, guaranteeing the deep link always wins the race. + let graceTimer: ReturnType | undefined; + let absoluteTimer: ReturnType | undefined; + const clearTimers = () => { + if (graceTimer !== undefined) clearTimeout(graceTimer); + if (absoluteTimer !== undefined) clearTimeout(absoluteTimer); + graceTimer = undefined; + absoluteTimer = undefined; + }; + const handleSuccess = (event: Event) => { const detail = (event as CustomEvent).detail; const toolkit = detail?.toolkit?.toLowerCase(); if (!toolkit || toolkit !== channelKey) return; + clearTimers(); log('oauth success for channel=%s authMode=%s', channel, authMode); dispatch( upsertChannelConnection({ @@ -101,6 +150,7 @@ export function useOAuthConnectionListener({ const provider = detail?.provider?.toLowerCase(); if (!provider || provider !== channelKey) return; + clearTimers(); const lastError = detail?.message || 'OAuth sign-in did not complete. Try again and approve access to continue.'; @@ -110,9 +160,64 @@ export function useOAuthConnectionListener({ window.addEventListener('oauth:success', handleSuccess); window.addEventListener('oauth:error', handleError); + + // Recovery is only relevant while the badge is pinned at `connecting`. + if (status !== 'connecting') { + return () => { + window.removeEventListener('oauth:success', handleSuccess); + window.removeEventListener('oauth:error', handleError); + clearTimers(); + }; + } + + // Flip to a retryable error, but only if the flow is *still* unresolved: + // a deep link landing during the grace window updates the store and makes + // this a no-op. Skip while a deep-link auth round-trip is in flight (the + // callback flips `isProcessing` before dispatching its event, same guard as + // `OAuthProviderButton`). + const recover = (label: string) => { + if (statusRef.current !== 'connecting') return; + if (getDeepLinkAuthState().isProcessing) { + log('[%s] recover via %s skipped (deep-link processing)', channel, label); + return; + } + const channelLabel = channel.charAt(0).toUpperCase() + channel.slice(1); + log('[%s] recover via %s -> error (no oauth deep link arrived)', channel, label); + dispatch( + setChannelConnectionStatus({ + channel, + authMode, + status: 'error', + lastError: t( + 'channels.oauth.connectRecoverFailed', + "Couldn't finish connecting {channel}. The sign-in window closed before access was granted — try again." + ).replace('{channel}', channelLabel), + }) + ); + }; + + const scheduleRecover = (label: string) => { + if (graceTimer !== undefined) clearTimeout(graceTimer); + graceTimer = setTimeout(() => recover(label), OAUTH_RECOVER_GRACE_MS); + }; + + // User returned to the app from the system browser without a deep link. + const handleFocus = () => scheduleRecover('focus'); + const handleVisibility = () => { + if (document.visibilityState === 'visible') scheduleRecover('visibilitychange'); + }; + + window.addEventListener('focus', handleFocus); + document.addEventListener('visibilitychange', handleVisibility); + // Backstop for the user who never refocuses the app at all. + absoluteTimer = setTimeout(() => scheduleRecover('timeout'), OAUTH_CONNECTING_TIMEOUT_MS); + return () => { window.removeEventListener('oauth:success', handleSuccess); window.removeEventListener('oauth:error', handleError); + window.removeEventListener('focus', handleFocus); + document.removeEventListener('visibilitychange', handleVisibility); + clearTimers(); }; - }, [dispatch, channel, authMode, capabilitiesOnSuccess]); + }, [dispatch, channel, authMode, capabilitiesOnSuccess, status, t]); }