diff --git a/web/packages/shared/hooks/useAsync.ts b/web/packages/shared/hooks/useAsync.ts index 1536dba4e0ec2..43ce02fdbdc90 100644 --- a/web/packages/shared/hooks/useAsync.ts +++ b/web/packages/shared/hooks/useAsync.ts @@ -22,9 +22,19 @@ import { useCallback, useState, useRef, useEffect } from 'react'; * * * The first element is the representation of the attempt to run that async function as data, the * so called attempt object. - * * The second element is a function which when called starts to execute the async function. + * * The second element is a function which when called starts to execute the async function (the + * `run` function). * * The third element is a function that lets you directly update the attempt object if needed. * + * `useAsync` automatically ignores stale promises either when the underlying component gets + * unmounted. If the `run` function gets executed again while the promise from the previous + * execution still hasn't finished, the return value of the previous promise will be ignored. + * + * The primary interface through which you should interact with the result of the callback is the + * attempt object. The `run` function has a return value as well which is useful if you need to + * chain its result with some other action inside an event handler or in `useEffect`. The return + * value of the `run` function corresponds to the return value of that specific invocation of the + * callback passed to `useAsync`. This means you need to manually handle the `CanceledError` case. * * @example * export function useUserProfile(userId) { @@ -44,7 +54,7 @@ import { useCallback, useState, useRef, useEffect } from 'react'; * if (!fetchUserProfileAttempt.status) { * fetchUserProfile() * } - * }, [fetchUserProfileAttempt]) + * }, [fetchUserProfileAttempt, fetchUserProfile]) * * switch (fetchUserProfileAttempt.status) { * case '': @@ -57,6 +67,21 @@ import { useCallback, useState, useRef, useEffect } from 'react'; * } * } * + * @example Consuming the `run` return value + * function UserProfile(props) { + * const { fetchUserProfileAttempt, fetchUserProfile } = useUserProfile(props.id); + * + * useEffect(async () => { + * if (!fetchUserProfileAttempt.status) { + * const [profile, err] = fetchUserProfile() + * + * if (err && !(err instanceof CanceledError)) { + * // Handle the error. + * } + * } + * }, [fetchUserProfileAttempt, fetchUserProfile]) + * } + * */ export function useAsync( cb: (...args: Args) => Promise @@ -114,14 +139,7 @@ export function useAsync( [setState, cb, isMounted] ); - const setAttempt = useCallback( - (attempt: Attempt) => { - setState(attempt); - }, - [setState] - ); - - return [state, run, setAttempt] as const; + return [state, run, setState] as const; } function useIsMounted() { diff --git a/web/packages/teleterm/src/services/tshd/testHelpers.ts b/web/packages/teleterm/src/services/tshd/testHelpers.ts index 051d6a8f38fe6..1922ebee6e907 100644 --- a/web/packages/teleterm/src/services/tshd/testHelpers.ts +++ b/web/packages/teleterm/src/services/tshd/testHelpers.ts @@ -49,3 +49,25 @@ export const makeKube = (props: Partial = {}): tsh.Kube => ({ export const makeLabelsList = (labels: Record): tsh.Label[] => Object.entries(labels).map(([name, value]) => ({ name, value })); + +export const makeRootCluster = ( + props: Partial = {} +): tsh.Cluster => ({ + uri: '/clusters/teleport-local', + name: 'teleport-local', + connected: true, + leaf: false, + proxyHost: 'teleport-local:3080', + authClusterId: '73c4746b-d956-4f16-9848-4e3469f70762', + loggedInUser: { + activeRequestsList: [], + assumedRequests: {}, + name: 'admin', + acl: {}, + sshLoginsList: [], + rolesList: [], + requestableRolesList: [], + suggestedReviewersList: [], + }, + ...props, +}); diff --git a/web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/ClusterLogin.tsx b/web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/ClusterLogin.tsx index b94b54ea4d345..4056866f1889c 100644 --- a/web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/ClusterLogin.tsx +++ b/web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/ClusterLogin.tsx @@ -59,7 +59,7 @@ export function ClusterLoginPresentation({ Login to {title} - + diff --git a/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx b/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx index b71170771c982..64996b898cedf 100644 --- a/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx @@ -15,20 +15,28 @@ */ import React from 'react'; +import userEvent from '@testing-library/user-event'; import { render, screen, waitFor } from 'design/utils/testing'; import { makeSuccessAttempt } from 'shared/hooks/useAsync'; +import Logger, { NullService } from 'teleterm/logger'; import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider'; import { ResourceSearchError } from 'teleterm/ui/services/resources'; import ModalsHost from 'teleterm/ui/ModalsHost'; +import { makeRootCluster } from 'teleterm/services/tshd/testHelpers'; import * as pickers from './pickers/pickers'; import * as useActionAttempts from './pickers/useActionAttempts'; +import * as useSearch from './useSearch'; import * as SearchContext from './SearchContext'; import { SearchBarConnected } from './SearchBar'; +beforeAll(() => { + Logger.init(new NullService()); +}); + beforeEach(() => { jest.restoreAllMocks(); }); @@ -181,7 +189,7 @@ it('notifies about resource search errors and allows to display details', () => .spyOn(SearchContext, 'useSearchContext') .mockImplementation(() => mockedSearchContext); jest.spyOn(appContext.modalsService, 'openRegularDialog'); - jest.spyOn(mockedSearchContext, 'lockOpen'); + jest.spyOn(mockedSearchContext, 'pauseUserInteraction'); render( @@ -204,7 +212,7 @@ it('notifies about resource search errors and allows to display details', () => errors: [resourceSearchError], }) ); - expect(mockedSearchContext.lockOpen).toHaveBeenCalled(); + expect(mockedSearchContext.pauseUserInteraction).toHaveBeenCalled(); }); it('maintains focus on the search input after closing a resource search error modal', async () => { @@ -264,14 +272,62 @@ it('maintains focus on the search input after closing a resource search error mo expect(screen.getByRole('menu')).toBeInTheDocument(); }); -const getMockedSearchContext = () => ({ +it('shows a login modal when a request to a cluster from the current workspace fails with a retryable error', async () => { + const user = userEvent.setup(); + const cluster = makeRootCluster(); + const resourceSearchError = new ResourceSearchError( + cluster.uri, + 'server', + new Error('ssh: cert has expired') + ); + const resourceSearchResult = { + results: [], + errors: [resourceSearchError], + search: 'foo', + }; + const resourceSearch = async () => resourceSearchResult; + jest + .spyOn(useSearch, 'useResourceSearch') + .mockImplementation(() => resourceSearch); + + const appContext = new MockAppContext(); + appContext.workspacesService.setState(draft => { + draft.rootClusterUri = cluster.uri; + }); + appContext.clustersService.setState(draftState => { + draftState.clusters.set(cluster.uri, cluster); + }); + + render( + + + + + ); + + await user.type(screen.getByRole('searchbox'), 'foo'); + + // Verify that the login modal was shown after typing in the search box. + await waitFor(() => { + expect(screen.getByTestId('Modal')).toBeInTheDocument(); + }); + expect(screen.getByTestId('Modal')).toHaveTextContent('Login to'); + + // Verify that the search bar stays open after closing the modal. + screen.getByLabelText('Close').click(); + await waitFor(() => { + expect(screen.queryByTestId('Modal')).not.toBeInTheDocument(); + }); + expect(screen.getByRole('menu')).toBeInTheDocument(); +}); + +const getMockedSearchContext = (): SearchContext.SearchContext => ({ inputValue: 'foo', filters: [], setFilter: () => {}, removeFilter: () => {}, isOpen: true, open: () => {}, - lockOpen: async () => {}, close: () => {}, closeAndResetInput: () => {}, resetInput: () => {}, @@ -279,4 +335,10 @@ const getMockedSearchContext = () => ({ onInputValueChange: () => {}, activePicker: pickers.actionPicker, inputRef: undefined, + pauseUserInteraction: async cb => { + cb(); + }, + addWindowEventListener: () => ({ + cleanup: () => {}, + }), }); diff --git a/web/packages/teleterm/src/ui/Search/SearchBar.tsx b/web/packages/teleterm/src/ui/Search/SearchBar.tsx index 6c1ae5a51df65..dcd0c37c58be5 100644 --- a/web/packages/teleterm/src/ui/Search/SearchBar.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchBar.tsx @@ -58,6 +58,7 @@ function SearchBar() { isOpen, open, close, + addWindowEventListener, } = useSearchContext(); const ctx = useAppContext(); ctx.clustersService.useState(); @@ -75,10 +76,12 @@ function SearchBar() { } }; if (isOpen) { - window.addEventListener('click', onClickOutside); - return () => window.removeEventListener('click', onClickOutside); + const { cleanup } = addWindowEventListener('click', onClickOutside, { + capture: true, + }); + return cleanup; } - }, [close, isOpen]); + }, [close, isOpen, addWindowEventListener]); function handleOnFocus(e: React.FocusEvent) { open(e.relatedTarget); diff --git a/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx b/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx index 1d610ec2062a4..31cce8c34b2f5 100644 --- a/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx @@ -16,12 +16,12 @@ import React from 'react'; import '@testing-library/jest-dom'; -import { render, screen } from '@testing-library/react'; +import { fireEvent, createEvent, render, screen } from '@testing-library/react'; import { renderHook, act } from '@testing-library/react-hooks'; import { SearchContextProvider, useSearchContext } from './SearchContext'; -describe('lockOpen', () => { +describe('pauseUserInteraction', () => { let resolveSuccessAction, rejectFailureAction; const successAction = new Promise(resolve => { resolveSuccessAction = resolve; @@ -32,17 +32,18 @@ describe('lockOpen', () => { test.each([ { - name: 'prevents the search bar from being closed for the duration of the action', + name: 'prevents the window listeners from being added for the duration of the action', action: successAction, finishAction: resolveSuccessAction, }, { - name: 'properly cleans up the ref even after the action fails', + name: 'properly cleans up the state even after the action fails', action: failureAction, finishAction: rejectFailureAction, }, ])('$name', async ({ action, finishAction }) => { const inputFocus = jest.fn(); + const onWindowClick = jest.fn(); const { result } = renderHook(() => useSearchContext(), { wrapper: ({ children }) => ( {children} @@ -52,38 +53,93 @@ describe('lockOpen', () => { focus: inputFocus, } as unknown as HTMLInputElement; + let pauseInteractionPromise: Promise; act(() => { - result.current.open(); + pauseInteractionPromise = result.current.pauseUserInteraction( + () => action + ); }); - let lockOpenPromise: Promise; - act(() => { - lockOpenPromise = result.current.lockOpen(action); - }); - - // Closing while the search bar is locked open should be a noop. - act(() => { - result.current.close(); - }); - expect(result.current.isOpen).toBe(true); + // Adding a window event while the interaction is paused should be a noop. + result.current.addWindowEventListener('click', onWindowClick); + fireEvent(window, createEvent.click(window)); + expect(onWindowClick).not.toHaveBeenCalled(); await act(async () => { finishAction(); try { - await lockOpenPromise; + await pauseInteractionPromise; } catch { // Ignore the error happening when `finishAction` rejects `action`. } }); - // The search bar should be no longer locked open, so close should behave as expected. + // User interaction is no longer paused, so addWindowEventListener should add a listener. + result.current.addWindowEventListener('click', onWindowClick); + fireEvent(window, createEvent.click(window)); + expect(onWindowClick).toHaveBeenCalledTimes(1); + + // Verify that the search input has received focus after pauseInteractionPromise finishes. + expect(inputFocus).toHaveBeenCalledTimes(1); + }); +}); + +describe('addWindowEventListener', () => { + it('returns a cleanup function', () => { + const onWindowClick = jest.fn(); + const { result } = renderHook(() => useSearchContext(), { + wrapper: ({ children }) => ( + {children} + ), + }); + + const { cleanup } = result.current.addWindowEventListener( + 'click', + onWindowClick, + // Add an extra arg to make sure that the same set of args is passed to removeEventListener as + // it is to addEventListener. + { capture: true } + ); + + fireEvent(window, createEvent.click(window)); + expect(onWindowClick).toHaveBeenCalledTimes(1); + + cleanup(); + + fireEvent(window, createEvent.click(window)); + // Verify that the listener was removed by the cleanup function. + expect(onWindowClick).toHaveBeenCalledTimes(1); + }); + + it('does not return a cleanup function when user interaction is paused', async () => { + let resolveAction; + const action = new Promise(resolve => { + resolveAction = resolve; + }); + + const { result } = renderHook(() => useSearchContext(), { + wrapper: ({ children }) => ( + {children} + ), + }); + + let pauseInteractionPromise; act(() => { - result.current.close(); + pauseInteractionPromise = result.current.pauseUserInteraction( + () => action + ); }); - expect(result.current.isOpen).toBe(false); - // Called the first time during `open`, then again after `lockOpen` finishes. - expect(inputFocus).toHaveBeenCalledTimes(2); + const { cleanup } = result.current.addWindowEventListener( + 'click', + jest.fn() + ); + expect(cleanup).toBeUndefined(); + + await act(async () => { + resolveAction(); + await pauseInteractionPromise; + }); }); }); diff --git a/web/packages/teleterm/src/ui/Search/SearchContext.tsx b/web/packages/teleterm/src/ui/Search/SearchContext.tsx index 8d3c4d3cd5238..ad65425ddf31a 100644 --- a/web/packages/teleterm/src/ui/Search/SearchContext.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchContext.tsx @@ -37,21 +37,27 @@ export interface SearchContext { changeActivePicker(picker: SearchPicker): void; isOpen: boolean; open(fromElement?: Element): void; - lockOpen(action: Promise): Promise; close(): void; closeAndResetInput(): void; resetInput(): void; setFilter(filter: SearchFilter): void; removeFilter(filter: SearchFilter): void; + pauseUserInteraction(action: () => Promise): Promise; + addWindowEventListener: AddWindowEventListener; } +export type AddWindowEventListener = ( + ...args: Parameters +) => { + cleanup: () => void; +}; + const SearchContext = createContext(null); export const SearchContextProvider: FC = props => { const previouslyActive = useRef(); const inputRef = useRef(); const [isOpen, setIsOpen] = useState(false); - const [isLockedOpen, setIsLockedOpen] = useState(false); const [inputValue, setInputValue] = useState(''); const [activePicker, setActivePicker] = useState(actionPicker); // TODO(ravicious): Consider using another data structure for search filters as we know that we @@ -69,25 +75,17 @@ export const SearchContextProvider: FC = props => { } const close = useCallback(() => { - if (isLockedOpen) { - return; - } - setIsOpen(false); setActivePicker(actionPicker); if (previouslyActive.current instanceof HTMLElement) { previouslyActive.current.focus(); } - }, [isLockedOpen]); + }, []); const closeAndResetInput = useCallback(() => { - if (isLockedOpen) { - return; - } - close(); setInputValue(''); - }, [isLockedOpen, close]); + }, [close]); const resetInput = useCallback(() => { setInputValue(''); @@ -109,28 +107,61 @@ export const SearchContextProvider: FC = props => { setIsOpen(true); } + const [isUserInteractionPaused, setIsUserInteractionPaused] = useState(false); /** - * lockOpen forces the search bar to stay open for the duration of the action. It also restores - * focus on the search input after the action is done. + * pauseUserInteraction temporarily causes addWindowEventListener not to add listeners for the + * duration of the action. It also restores focus on the search input after the action is done. * - * This is useful in situations where want the search bar to not close when the user interacts - * with modals shown from the search bar. + * This is useful in situations where want the search bar to show some other element the user can + * interact with, for example a modal. When the user interacts with the modal, we don't want the + * search bar listeners to intercept those interactions, which could for example cause the search + * bar to close or make the user unable to press Enter in the modal as the search bar would + * swallow that event. */ - async function lockOpen(action: Promise): Promise { - setIsLockedOpen(true); - - try { - await action; - } finally { - // By the time the action passes, the user might have caused the focus to be lost on the - // search input, so let's bring it back. - // - // focus needs to be executed before the state update, otherwise the search bar will close for - // some reason. - inputRef.current?.focus(); - setIsLockedOpen(false); - } - } + const pauseUserInteraction = useCallback( + async (action: () => Promise): Promise => { + setIsUserInteractionPaused(true); + + try { + await action(); + } finally { + // By the time the action passes, the user might have caused the focus to be lost on the + // search input, so let's bring it back. + // + // focus needs to be executed before the state update, otherwise the search bar will close + // for some reason. + inputRef.current?.focus(); + setIsUserInteractionPaused(false); + } + }, + [] + ); + + /** + * addWindowEventListener is meant to be used in useEffect calls which register event listeners + * related to the search bar. It automatically removes the listener when the user interaction gets + * paused. + * + * pauseUserInteraction is supposed to be called in situations where the search bar is obstructed + * by another element (such as a modal) and we don't want interactions with that other element to + * have any effect on the search bar. + */ + const addWindowEventListener = useCallback( + (...args: Parameters) => { + if (isUserInteractionPaused) { + return { cleanup: undefined }; + } + + window.addEventListener(...args); + + return { + cleanup: () => { + window.removeEventListener(...args); + }, + }; + }, + [isUserInteractionPaused] + ); function setFilter(filter: SearchFilter) { // UI prevents adding more than one filter of the same type @@ -165,9 +196,10 @@ export const SearchContextProvider: FC = props => { resetInput, isOpen, open, - lockOpen, close, closeAndResetInput, + pauseUserInteraction, + addWindowEventListener, }} children={props.children} /> diff --git a/web/packages/teleterm/src/ui/Search/actions.tsx b/web/packages/teleterm/src/ui/Search/actions.tsx index 431566372f44b..af187ee50efb2 100644 --- a/web/packages/teleterm/src/ui/Search/actions.tsx +++ b/web/packages/teleterm/src/ui/Search/actions.tsx @@ -22,6 +22,7 @@ import { connectToKube, connectToServer, } from 'teleterm/ui/services/workspacesService'; +import { retryWithRelogin } from 'teleterm/ui/utils'; export interface SimpleAction { type: 'simple-action'; @@ -95,7 +96,9 @@ export function mapToActions( searchResult: result, parameter: { getSuggestions: () => - ctx.resourcesService.getDbUsers(result.resource.uri), + retryWithRelogin(ctx, result.resource.uri, () => + ctx.resourcesService.getDbUsers(result.resource.uri) + ), placeholder: 'Provide db username', }, perform: dbUser => { diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.story.tsx b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.story.tsx index b2dd570fc01ce..5cbfeeb85ecf4 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.story.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.story.tsx @@ -324,6 +324,7 @@ const SearchResultItems = () => { attempts={[attempt]} onPick={() => {}} onBack={() => {}} + addWindowEventListener={() => ({ cleanup: () => {} })} render={searchResult => { const Component = ComponentMap[searchResult.kind]; @@ -350,6 +351,7 @@ const AuxiliaryItems = () => ( onBack={() => {}} render={() => null} attempts={[]} + addWindowEventListener={() => ({ cleanup: () => {} })} ExtraTopComponent={ <> 0 ) { const showErrorsInModal = () => { - lockOpen( - new Promise(resolve => { - modalsService.openRegularDialog({ - kind: 'resource-search-errors', - errors: resourceSearchAttempt.data.errors, - getClusterName, - onCancel: () => resolve(undefined), - }); - }) + pauseUserInteraction( + () => + new Promise(resolve => { + modalsService.openRegularDialog({ + kind: 'resource-search-errors', + errors: resourceSearchAttempt.data.errors, + getClusterName, + onCancel: () => resolve(undefined), + }); + }) ); }; @@ -204,6 +206,7 @@ export function ActionPicker(props: { input: ReactElement }) { attempts={actionAttempts} onPick={onPick} onBack={close} + addWindowEventListener={addWindowEventListener} render={item => { const Component = ComponentMap[item.searchResult.kind]; return { @@ -419,7 +422,7 @@ export function DatabaseItem(props: SearchResultItem) { gap={1} > - Set up a db connection for{' '} + Set up a db connection to{' '} diff --git a/web/packages/teleterm/src/ui/Search/pickers/ParameterPicker.tsx b/web/packages/teleterm/src/ui/Search/pickers/ParameterPicker.tsx index ed7a0e654fe56..a33c437b3f009 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ParameterPicker.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ParameterPicker.tsx @@ -35,8 +35,13 @@ interface ParameterPickerProps { } export function ParameterPicker(props: ParameterPickerProps) { - const { inputValue, closeAndResetInput, changeActivePicker, resetInput } = - useSearchContext(); + const { + inputValue, + closeAndResetInput, + changeActivePicker, + resetInput, + addWindowEventListener, + } = useSearchContext(); const [suggestionsAttempt, fetch] = useAsync( props.action.parameter.getSuggestions ); @@ -77,6 +82,7 @@ export function ParameterPicker(props: ParameterPickerProps) { attempts={[inputSuggestionAttempt, attempt]} onPick={onPick} onBack={onBack} + addWindowEventListener={addWindowEventListener} render={item => ({ key: item, Component: ( diff --git a/web/packages/teleterm/src/ui/Search/pickers/ResultList.tsx b/web/packages/teleterm/src/ui/Search/pickers/ResultList.tsx index 93093b60c6b76..a8d80ed33498d 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ResultList.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ResultList.tsx @@ -22,11 +22,12 @@ import React, { useState, } from 'react'; import styled, { css } from 'styled-components'; - import { Attempt } from 'shared/hooks/useAsync'; import LinearProgress from 'teleterm/ui/components/LinearProgress'; +import { AddWindowEventListener } from '../SearchContext'; + type ResultListProps = { /** * List of attempts containing results to render. @@ -41,10 +42,17 @@ type ResultListProps = { onPick(item: T): void; onBack(): void; render(item: T): { Component: ReactElement; key: string }; + addWindowEventListener: AddWindowEventListener; }; export function ResultList(props: ResultListProps) { - const { attempts, ExtraTopComponent, onPick, onBack } = props; + const { + attempts, + ExtraTopComponent, + onPick, + onBack, + addWindowEventListener, + } = props; const activeItemRef = useRef(); const [activeItemIndex, setActiveItemIndex] = useState(0); @@ -98,9 +106,11 @@ export function ResultList(props: ResultListProps) { } }; - window.addEventListener('keydown', handleKeyDown); - return () => window.removeEventListener('keydown', handleKeyDown); - }, [items, onPick, onBack, activeItemIndex]); + const { cleanup } = addWindowEventListener('keydown', handleKeyDown, { + capture: true, + }); + return cleanup; + }, [items, onPick, onBack, activeItemIndex, addWindowEventListener]); return ( <> diff --git a/web/packages/teleterm/src/ui/Search/pickers/useActionAttempts.ts b/web/packages/teleterm/src/ui/Search/pickers/useActionAttempts.ts index 590e5cc15c6d0..198cb5df3c4bf 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/useActionAttempts.ts +++ b/web/packages/teleterm/src/ui/Search/pickers/useActionAttempts.ts @@ -14,7 +14,13 @@ * limitations under the License. */ -import { useEffect, useLayoutEffect, useMemo, useRef } from 'react'; +import { + useEffect, + useLayoutEffect, + useMemo, + useRef, + useCallback, +} from 'react'; import { makeEmptyAttempt, makeSuccessAttempt, @@ -32,19 +38,77 @@ import { mapToActions } from 'teleterm/ui/Search/actions'; import Logger from 'teleterm/logger'; import { useAppContext } from 'teleterm/ui/appContextProvider'; import { useSearchContext } from 'teleterm/ui/Search/SearchContext'; +import { SearchFilter } from 'teleterm/ui/Search/searchResult'; +import { routing } from 'teleterm/ui/uri'; +import { isRetryable } from 'teleterm/ui/utils/retryWithRelogin'; export function useActionAttempts() { const searchLogger = useRef(new Logger('search')); const ctx = useAppContext(); - // Both states are used by mapToActions. - ctx.workspacesService.useState(); - ctx.clustersService.useState(); + const { modalsService, workspacesService } = ctx; const searchContext = useSearchContext(); - const { inputValue, filters } = searchContext; + const { inputValue, filters, pauseUserInteraction } = searchContext; const [resourceSearchAttempt, runResourceSearch, setResourceSearchAttempt] = useAsync(useResourceSearch()); - const runResourceSearchDebounced = useDebounce(runResourceSearch, 200); + /** + * runRetryableResourceSearch implements retryWithRelogin logic for resource search. We check if + * among all resource search requests there's a request belonging to the current workspace and if + * it failed with a retryable error. If so, we show the login modal. + * + * We're interested only in errors coming from clusters within the current workspace because + * otherwise we'd nag the user to log in to each cluster they've added to the app. + * + * Technically we could wrap runResourceSearch into a custom promise which would make it fit into + * retryWithRelogin. However, by doing this we'd give up some of the finer control over the whole + * process, for example we couldn't as easily call pauseUserInteraction only when necessary. + * + * This logic _cannot_ be included within the callback passed to useAsync and has to be performed + * outside of it. If it was included within useAsync, then this logic would fire for any request + * made to the cluster, even for stale requests which are ignored by useAsync. + */ + const runRetryableResourceSearch = useCallback( + async (search: string, filters: SearchFilter[]): Promise => { + const activeRootClusterUri = workspacesService.getRootClusterUri(); + const [results, err] = await runResourceSearch(search, filters); + // Since resource search uses Promise.allSettled underneath, the only error that could be + // returned here is CanceledError from useAsync. In that case, we can just return early. + if (err) { + return; + } + + const hasActiveWorkspaceRequestFailedWithRetryableError = + results.errors.some( + error => + routing.belongsToProfile(activeRootClusterUri, error.clusterUri) && + isRetryable(error.cause) + ); + + if (!hasActiveWorkspaceRequestFailedWithRetryableError) { + return; + } + + await pauseUserInteraction( + () => + new Promise(resolve => { + modalsService.openClusterConnectDialog({ + clusterUri: activeRootClusterUri, + onSuccess: () => resolve(), + onCancel: () => resolve(), + }); + }) + ); + + // Retrying the request no matter if the user logged in through the modal or not, for the same + // reasons as described in retryWithRelogin. + runResourceSearch(search, filters); + }, + [modalsService, workspacesService, runResourceSearch, pauseUserInteraction] + ); + const runDebouncedResourceSearch = useDebounce( + runRetryableResourceSearch, + 200 + ); const resourceActionsAttempt = useMemo( () => mapAttempt(resourceSearchAttempt, ({ results, search }) => { @@ -74,13 +138,13 @@ export function useActionAttempts() { // 3. Now you see the stale results for `foo`, because the debounce didn't kick in yet. setResourceSearchAttempt(makeEmptyAttempt()); - runResourceSearchDebounced(inputValue, filters); + runDebouncedResourceSearch(inputValue, filters); }, [ inputValue, filters, setResourceSearchAttempt, runFilterSearch, - runResourceSearchDebounced, + runDebouncedResourceSearch, ]); return { diff --git a/web/packages/teleterm/src/ui/Search/useSearch.ts b/web/packages/teleterm/src/ui/Search/useSearch.ts index 4759ab18e01cc..7a261669314ae 100644 --- a/web/packages/teleterm/src/ui/Search/useSearch.ts +++ b/web/packages/teleterm/src/ui/Search/useSearch.ts @@ -43,12 +43,11 @@ import type * as resourcesServiceTypes from 'teleterm/ui/services/resources'; */ export function useResourceSearch() { const { clustersService, resourcesService } = useAppContext(); - clustersService.useState(); return useCallback( async ( search: string, - restrictions: SearchFilter[] + filters: SearchFilter[] ): Promise<{ results: resourcesServiceTypes.SearchResult[]; errors: resourcesServiceTypes.ResourceSearchError[]; @@ -67,10 +66,10 @@ export function useResourceSearch() { return { results: [], errors: [], search }; } - const clusterSearchFilter = restrictions.find( + const clusterSearchFilter = filters.find( s => s.filter === 'cluster' ) as ClusterSearchFilter; - const resourceTypeSearchFilter = restrictions.find( + const resourceTypeSearchFilter = filters.find( s => s.filter === 'resource-type' ) as ResourceTypeSearchFilter; @@ -125,11 +124,9 @@ export function useResourceSearch() { */ export function useFilterSearch() { const { clustersService, workspacesService } = useAppContext(); - clustersService.useState(); - workspacesService.useState(); return useCallback( - (search: string, restrictions: SearchFilter[]): FilterSearchResult[] => { + (search: string, filters: SearchFilter[]): FilterSearchResult[] => { const getClusters = () => { let clusters = clustersService.getClusters(); // Cluster filter should not be visible if there is only one cluster @@ -179,10 +176,8 @@ export function useFilterSearch() { })); }; - const shouldReturnClusters = !restrictions.some( - r => r.filter === 'cluster' - ); - const shouldReturnResourceTypes = !restrictions.some( + const shouldReturnClusters = !filters.some(r => r.filter === 'cluster'); + const shouldReturnResourceTypes = !filters.some( r => r.filter === 'resource-type' ); diff --git a/web/packages/teleterm/src/ui/utils/retryWithRelogin.ts b/web/packages/teleterm/src/ui/utils/retryWithRelogin.ts index 79a9865d30295..c88abfff9394f 100644 --- a/web/packages/teleterm/src/ui/utils/retryWithRelogin.ts +++ b/web/packages/teleterm/src/ui/utils/retryWithRelogin.ts @@ -49,13 +49,7 @@ export async function retryWithRelogin( try { return await actionToRetry(); } catch (error) { - // TODO(ravicious): Replace this with actual check on metadata. - const isRetryable = - error instanceof Error && - (error.message.includes('ssh: handshake failed') || - error.message.includes('ssh: cert has expired')); - - if (isRetryable) { + if (isRetryable(error)) { retryableErrorFromActionToRetry = error; logger.info(`Activating relogin on error ${error}`); } else { @@ -84,6 +78,15 @@ export async function retryWithRelogin( return await actionToRetry(); } +export function isRetryable(error: unknown): boolean { + // TODO(ravicious): Replace this with actual check on metadata. + return ( + error instanceof Error && + (error.message.includes('ssh: handshake failed') || + error.message.includes('ssh: cert has expired')) + ); +} + // Notice that we don't differentiate between onSuccess and onCancel. In both cases, we're going to // retry the action anyway in case the cert was refreshed externally before the modal was closed, // for example through tsh login.