[v13] Retry search in Connect if current workspace cert has expired

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<Args extends unknown[], AttemptData>(
   cb: (...args: Args) => Promise<AttemptData>
@@ -114,14 +139,7 @@ export function useAsync<Args extends unknown[], AttemptData>(
     [setState, cb, isMounted]
   );
 
-  const setAttempt = useCallback(
-    (attempt: Attempt<AttemptData>) => {
-      setState(attempt);
-    },
-    [setState]
-  );
-
-  return [state, run, setAttempt] as const;
+  return [state, run, setState] as const;
 }
 
 function useIsMounted() {

web/packages/teleterm/src/services/tshd/testHelpers.ts

@@ -49,3 +49,25 @@ export const makeKube = (props: Partial<tsh.Kube> = {}): tsh.Kube => ({
 
 export const makeLabelsList = (labels: Record<string, string>): tsh.Label[] =>
   Object.entries(labels).map(([name, value]) => ({ name, value }));
+
+export const makeRootCluster = (
+  props: Partial<tsh.Cluster> = {}
+): 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,
+});

web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/ClusterLogin.tsx

@@ -59,7 +59,7 @@ export function ClusterLoginPresentation({
         <Text typography="h4">
           Login to <b>{title}</b>
         </Text>
-        <ButtonIcon ml="auto" p={3} onClick={onCloseDialog}>
+        <ButtonIcon ml="auto" p={3} onClick={onCloseDialog} aria-label="Close">
           <Icons.Close fontSize="20px" />
         </ButtonIcon>
       </DialogHeader>

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(
     <MockAppContextProvider appContext={appContext}>
@@ -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,19 +272,73 @@ 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(
+    <MockAppContextProvider appContext={appContext}>
+      <SearchBarConnected />
+      <ModalsHost />
+    </MockAppContextProvider>
+  );
+
+  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: () => {},
   changeActivePicker: () => {},
   onInputValueChange: () => {},
   activePicker: pickers.actionPicker,
   inputRef: undefined,
+  pauseUserInteraction: async cb => {
+    cb();
+  },
+  addWindowEventListener: () => ({
+    cleanup: () => {},
+  }),
 });

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);

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 }) => (
         <SearchContextProvider>{children}</SearchContextProvider>
@@ -52,38 +53,93 @@ describe('lockOpen', () => {
       focus: inputFocus,
     } as unknown as HTMLInputElement;
 
+    let pauseInteractionPromise: Promise<void>;
     act(() => {
-      result.current.open();
+      pauseInteractionPromise = result.current.pauseUserInteraction(
+        () => action
+      );
     });
 
-    let lockOpenPromise: Promise<void>;
-    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 }) => (
+        <SearchContextProvider>{children}</SearchContextProvider>
+      ),
+    });
+
+    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 }) => (
+        <SearchContextProvider>{children}</SearchContextProvider>
+      ),
+    });
+
+    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;
+    });
   });
 });