[v14] Add error to Attempt in useAsync (#32117)

* Add error to Attempt in useAsync

* Ensure attempt.error refers to the same object as expectedError

* Refactor error handling to use instanceof with attempt.error

* makeErrorAttempt: Require an Error object

In real code, the function can throw any object. But since makeErrorAttempt
is a helper function, we can make it require an Error object.

web/packages/shared/hooks/useAsync.test.ts

@@ -171,3 +171,26 @@ test('run does not update state after being re-run when the callback returns a r
     await secondRunPromise;
   });
 });
+
+test('error and statusText are set when the callback returns a rejected promise', async () => {
+  const expectedError = new Error('whoops');
+
+  const { result, waitFor } = renderHook(() =>
+    useAsync(() => Promise.reject(expectedError))
+  );
+
+  let runPromise: Promise<any>;
+  let [, run] = result.current;
+
+  act(() => {
+    runPromise = run();
+  });
+  await waitFor(() => result.current[0].status === 'error');
+
+  const attempt = result.current[0];
+  expect(attempt['error']).toBe(expectedError);
+  expect(attempt['statusText']).toEqual(expectedError.message);
+
+  // The promise returned from run always succeeds, but any errors are captured as the second arg.
+  await expect(runPromise).resolves.toEqual([null, expectedError]);
+});

web/packages/shared/hooks/useAsync.ts

@@ -93,8 +93,9 @@ export function useAsync<Args extends unknown[], AttemptData>(
   const run = useCallback(
     (...args: Args) => {
       setState(prevState => ({
-        ...prevState,
         status: 'processing',
+        data: prevState['data'],
+        statusText: prevState['statusText'],
       }));
 
       const promise = cb(...args);
@@ -125,9 +126,9 @@ export function useAsync<Args extends unknown[], AttemptData>(
             return [null, new CanceledError()] as [AttemptData, Error];
           }
 
-          setState(prevState => ({
-            ...prevState,
+          setState(() => ({
             status: 'error',
+            error: err,
             statusText: err?.message,
             data: null,
           }));
@@ -163,13 +164,43 @@ export class CanceledError extends Error {
   }
 }
 
-export type AttemptStatus = 'processing' | 'success' | 'error' | '';
+export type AttemptStatus = Attempt<any>['status'];
 
-export type Attempt<T> = {
-  data?: T;
-  status: AttemptStatus;
-  statusText: string;
-};
+export type Attempt<T> =
+  | {
+      status: '';
+      data: null;
+      /**
+       * @deprecated statusText is present for compatibility purposes only. To use statusText, check
+       * if status equals 'error' first.
+       */
+      statusText: string;
+    }
+  | {
+      status: 'processing';
+      /** data is either null or contains data from the previous success attempt if the attempt was retried. */
+      data: null | T;
+      /**
+       * @deprecated statusText is present for compatibility purposes only. To use statusText, check
+       * if status equals 'error' first.
+       */
+      statusText: string;
+    }
+  | {
+      status: 'success';
+      data: T;
+      /**
+       * @deprecated statusText is present for compatibility purposes only. To use statusText, check
+       * if status equals 'error' first.
+       */
+      statusText: string;
+    }
+  | {
+      status: 'error';
+      data: null;
+      statusText: string;
+      error: any;
+    };
 
 export function hasFinished<T>(attempt: Attempt<T>): boolean {
   return attempt.status === 'success' || attempt.status === 'error';
@@ -199,11 +230,26 @@ export function makeProcessingAttempt<T>(): Attempt<T> {
   };
 }
 
-export function makeErrorAttempt<T>(statusText: string): Attempt<T> {
+export function makeErrorAttempt<T>(error: Error): Attempt<T> {
+  return {
+    data: null,
+    status: 'error',
+    error: error,
+    statusText: error.message,
+  };
+}
+
+/**
+ * @deprecated Use makeErrorAttempt instead.
+ */
+export function makeErrorAttemptWithStatusText<T>(
+  statusText: string
+): Attempt<T> {
   return {
     data: null,
     status: 'error',
     statusText,
+    error: new Error(statusText),
   };
 }
 

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

@@ -17,7 +17,7 @@ limitations under the License.
 import React from 'react';
 
 import { Box } from 'design';
-import { Attempt } from 'shared/hooks/useAsync';
+import { Attempt, makeErrorAttempt } from 'shared/hooks/useAsync';
 
 import * as types from 'teleterm/ui/services/clusters/types';
 import {
@@ -70,12 +70,9 @@ function makeProps(): ClusterLoginPresentationProps {
   };
 }
 
-export const Error = () => {
+export const Err = () => {
   const props = makeProps();
-  props.initAttempt = {
-    status: 'error',
-    statusText: 'some error message',
-  };
+  props.initAttempt = makeErrorAttempt(new Error('some error message'));
 
   return (
     <TestContainer>

web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx

@@ -192,7 +192,7 @@ function AgentSetup({ rootClusterUri }: { rootClusterUri: RootClusterUri }) {
           return;
         }
 
-        if (joinClusterAttempt.statusText !== AgentProcessError.name) {
+        if (!(joinClusterAttempt.error instanceof AgentProcessError)) {
           return <StandardError error={joinClusterAttempt.statusText} />;
         }
 

web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.tsx

@@ -258,7 +258,7 @@ function prettifyCurrentAction(currentAction: CurrentAction): {
           return noop; // noop, not used, at this point it should be start processing.
         }
         default: {
-          return assertUnreachable(currentAction.attempt.status);
+          return assertUnreachable(currentAction.attempt);
         }
       }
     }
@@ -272,7 +272,7 @@ function prettifyCurrentAction(currentAction: CurrentAction): {
           };
         }
         case 'error': {
-          if (currentAction.attempt.statusText !== AgentProcessError.name) {
+          if (!(currentAction.attempt.error instanceof AgentProcessError)) {
             return {
               Icon: StyledWarning,
               title: 'Failed to start agent',
@@ -306,7 +306,7 @@ function prettifyCurrentAction(currentAction: CurrentAction): {
           return noop; // noop, not used, at this point it should be observe-process running.
         }
         default: {
-          return assertUnreachable(currentAction.attempt.status);
+          return assertUnreachable(currentAction.attempt);
         }
       }
       break;
@@ -377,7 +377,7 @@ function prettifyCurrentAction(currentAction: CurrentAction): {
           return noop; // noop, not used, at this point it should be observe-process exited.
         }
         default: {
-          return assertUnreachable(currentAction.attempt.status);
+          return assertUnreachable(currentAction.attempt);
         }
       }
     }

web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.test.tsx

@@ -117,7 +117,7 @@ test('startAgent re-throws errors that are thrown while spawning the process', a
   expect(error).toBeInstanceOf(AgentProcessError);
   expect(result.current.currentAction).toStrictEqual({
     kind: 'start',
-    attempt: makeErrorAttempt(AgentProcessError.name),
+    attempt: makeErrorAttempt(new AgentProcessError()),
     agentProcessState: {
       status: 'error',
       message: 'ENOENT',

web/packages/teleterm/src/ui/DocumentGateway/DocumentGateway.story.tsx

@@ -19,7 +19,7 @@ import React from 'react';
 import {
   makeEmptyAttempt,
   makeProcessingAttempt,
-  makeErrorAttempt,
+  makeErrorAttemptWithStatusText,
   makeSuccessAttempt,
 } from 'shared/hooks/useAsync';
 
@@ -91,7 +91,7 @@ export function OnlineWithFailedDbNameAttempt() {
   return (
     <DocumentGateway
       {...onlineDocumentGatewayProps}
-      changeDbNameAttempt={makeErrorAttempt<void>(
+      changeDbNameAttempt={makeErrorAttemptWithStatusText<void>(
         'Something went wrong with setting database name.'
       )}
     />
@@ -102,7 +102,7 @@ export function OnlineWithFailedPortAttempt() {
   return (
     <DocumentGateway
       {...onlineDocumentGatewayProps}
-      changePortAttempt={makeErrorAttempt<void>(
+      changePortAttempt={makeErrorAttemptWithStatusText<void>(
         'Something went wrong with setting port.'
       )}
     />
@@ -113,10 +113,10 @@ export function OnlineWithFailedDbNameAndPortAttempts() {
   return (
     <DocumentGateway
       {...onlineDocumentGatewayProps}
-      changeDbNameAttempt={makeErrorAttempt<void>(
+      changeDbNameAttempt={makeErrorAttemptWithStatusText<void>(
         'Something went wrong with setting database name.'
       )}
-      changePortAttempt={makeErrorAttempt<void>(
+      changePortAttempt={makeErrorAttemptWithStatusText<void>(
         'Something went wrong with setting port.'
       )}
     />
@@ -142,7 +142,7 @@ export function OfflineWithFailedConnectAttempt() {
       gateway={undefined}
       defaultPort="62414"
       connected={false}
-      connectAttempt={makeErrorAttempt<void>(
+      connectAttempt={makeErrorAttemptWithStatusText<void>(
         'listen tcp 127.0.0.1:62414: bind: address already in use'
       )}
     />

web/packages/teleterm/src/ui/DocumentTerminal/Terminal/Terminal.tsx

@@ -21,7 +21,7 @@ import { debounce } from 'shared/utils/highbar';
 import {
   Attempt,
   makeEmptyAttempt,
-  makeErrorAttempt,
+  makeErrorAttemptWithStatusText,
   makeSuccessAttempt,
 } from 'shared/hooks/useAsync';
 
@@ -59,7 +59,7 @@ export function Terminal(props: TerminalProps) {
   useEffect(() => {
     const removeOnStartErrorListener = props.ptyProcess.onStartError(
       message => {
-        setStartPtyProcessAttempt(makeErrorAttempt(message));
+        setStartPtyProcessAttempt(makeErrorAttemptWithStatusText(message));
       }
     );