feat(app): "Ignore this error" during Error Recovery (#15719)

Closes EXEC-461
Opentrons · Jul 22, 2024 · 5643bbe · 5643bbe
1 parent c431da3
commit 5643bbe
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 62 deletions.
diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/FillWellAndSkip.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/FillWellAndSkip.tsx
@@ -86,7 +86,7 @@ export function SkipToNextStep(
     proceedToRouteAndStep,
   } = routeUpdateActions
   const { selectedRecoveryOption } = currentRecoveryOptionUtils
-  const { skipFailedCommand, resumeRun } = recoveryCommands
+  const { skipFailedCommand } = recoveryCommands
   const { ROBOT_SKIPPING_STEP, IGNORE_AND_SKIP } = RECOVERY_MAP
   const { t } = useTranslation('error_recovery')
 
@@ -100,11 +100,9 @@ export function SkipToNextStep(
   }
 
   const primaryBtnOnClick = (): Promise<void> => {
-    return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE)
-      .then(() => skipFailedCommand())
-      .then(() => {
-        resumeRun()
-      })
+    return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE).then(() => {
+      skipFailedCommand()
+    })
   }
 
   const buildBodyText = (): JSX.Element => {

diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepNewTips.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepNewTips.tsx
@@ -48,17 +48,15 @@ export function SkipStepNewTips(
 
 export function SkipStepWithNewTips(props: RecoveryContentProps): JSX.Element {
   const { recoveryCommands, routeUpdateActions } = props
-  const { skipFailedCommand, resumeRun } = recoveryCommands
+  const { skipFailedCommand } = recoveryCommands
   const { setRobotInMotion } = routeUpdateActions
   const { ROBOT_SKIPPING_STEP } = RECOVERY_MAP
   const { t } = useTranslation('error_recovery')
 
   const primaryBtnOnClick = (): Promise<void> => {
-    return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE)
-      .then(() => skipFailedCommand())
-      .then(() => {
-        resumeRun()
-      })
+    return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE).then(() => {
+      skipFailedCommand()
+    })
   }
 
   const buildBodyText = (): JSX.Element => {

diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepSameTips.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepSameTips.tsx
@@ -29,17 +29,15 @@ export function SkipStepSameTips(props: RecoveryContentProps): JSX.Element {
 
 export function SkipStepSameTipsInfo(props: RecoveryContentProps): JSX.Element {
   const { routeUpdateActions, recoveryCommands } = props
-  const { skipFailedCommand, resumeRun } = recoveryCommands
+  const { skipFailedCommand } = recoveryCommands
   const { setRobotInMotion } = routeUpdateActions
   const { ROBOT_SKIPPING_STEP } = RECOVERY_MAP
   const { t } = useTranslation('error_recovery')
 
   const primaryBtnOnClick = (): Promise<void> => {
-    return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE)
-      .then(() => skipFailedCommand())
-      .then(() => {
-        resumeRun()
-      })
+    return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE).then(() => {
+      skipFailedCommand()
+    })
   }
 
   const buildBodyText = (): JSX.Element => {

diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/FillWellAndSkip.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/FillWellAndSkip.test.tsx
@@ -141,14 +141,12 @@ describe('SkipToNextStep', () => {
   let mockGoBackPrevStep: Mock
   let mockProceedToRouteAndStep: Mock
   let mockSkipFailedCommand: Mock
-  let mockResumeRun: Mock
 
   beforeEach(() => {
     mockSetRobotInMotion = vi.fn(() => Promise.resolve())
     mockGoBackPrevStep = vi.fn()
     mockProceedToRouteAndStep = vi.fn()
     mockSkipFailedCommand = vi.fn(() => Promise.resolve())
-    mockResumeRun = vi.fn()
 
     props = {
       ...mockRecoveryContentProps,
@@ -159,7 +157,6 @@ describe('SkipToNextStep', () => {
       } as any,
       recoveryCommands: {
         skipFailedCommand: mockSkipFailedCommand,
-        resumeRun: mockResumeRun,
       } as any,
     }
   })
@@ -197,15 +194,9 @@ describe('SkipToNextStep', () => {
     await waitFor(() => {
       expect(mockSkipFailedCommand).toHaveBeenCalled()
     })
-    await waitFor(() => {
-      expect(mockResumeRun).toHaveBeenCalled()
-    })
 
     expect(mockSetRobotInMotion.mock.invocationCallOrder[0]).toBeLessThan(
       mockSkipFailedCommand.mock.invocationCallOrder[0]
     )
-    expect(mockSkipFailedCommand.mock.invocationCallOrder[0]).toBeLessThan(
-      mockResumeRun.mock.invocationCallOrder[0]
-    )
   })
 })
diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepNewTips.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepNewTips.test.tsx
@@ -124,12 +124,10 @@ describe('SkipStepWithNewTips', () => {
   let props: React.ComponentProps<typeof SkipStepWithNewTips>
   let mockSetRobotInMotion: Mock
   let mockSkipFailedCommand: Mock
-  let mockResumeRun: Mock
 
   beforeEach(() => {
     mockSetRobotInMotion = vi.fn(() => Promise.resolve())
     mockSkipFailedCommand = vi.fn(() => Promise.resolve())
-    mockResumeRun = vi.fn()
 
     props = {
       ...mockRecoveryContentProps,
@@ -138,7 +136,6 @@ describe('SkipStepWithNewTips', () => {
       } as any,
       recoveryCommands: {
         skipFailedCommand: mockSkipFailedCommand,
-        resumeRun: mockResumeRun,
       } as any,
     }
   })
@@ -169,15 +166,9 @@ describe('SkipStepWithNewTips', () => {
     await waitFor(() => {
       expect(mockSkipFailedCommand).toHaveBeenCalled()
     })
-    await waitFor(() => {
-      expect(mockResumeRun).toHaveBeenCalled()
-    })
 
     expect(mockSetRobotInMotion.mock.invocationCallOrder[0]).toBeLessThan(
       mockSkipFailedCommand.mock.invocationCallOrder[0]
     )
-    expect(mockSkipFailedCommand.mock.invocationCallOrder[0]).toBeLessThan(
-      mockResumeRun.mock.invocationCallOrder[0]
-    )
   })
 })
diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepSameTips.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepSameTips.test.tsx
@@ -72,12 +72,10 @@ describe('SkipStepSameTipsInfo', () => {
   let props: React.ComponentProps<typeof SkipStepSameTipsInfo>
   let mockSetRobotInMotion: Mock
   let mockSkipFailedCommand: Mock
-  let mockResumeRun: Mock
 
   beforeEach(() => {
     mockSetRobotInMotion = vi.fn(() => Promise.resolve())
     mockSkipFailedCommand = vi.fn(() => Promise.resolve())
-    mockResumeRun = vi.fn()
 
     props = {
       ...mockRecoveryContentProps,
@@ -86,7 +84,6 @@ describe('SkipStepSameTipsInfo', () => {
       } as any,
       recoveryCommands: {
         skipFailedCommand: mockSkipFailedCommand,
-        resumeRun: mockResumeRun,
       } as any,
     }
   })
@@ -114,18 +111,13 @@ describe('SkipStepSameTipsInfo', () => {
         RECOVERY_MAP.ROBOT_SKIPPING_STEP.ROUTE
       )
     })
+
     await waitFor(() => {
       expect(mockSkipFailedCommand).toHaveBeenCalled()
     })
-    await waitFor(() => {
-      expect(mockResumeRun).toHaveBeenCalled()
-    })
 
     expect(mockSetRobotInMotion.mock.invocationCallOrder[0]).toBeLessThan(
       mockSkipFailedCommand.mock.invocationCallOrder[0]
     )
-    expect(mockSkipFailedCommand.mock.invocationCallOrder[0]).toBeLessThan(
-      mockResumeRun.mock.invocationCallOrder[0]
-    )
   })
 })
diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts
@@ -221,25 +221,23 @@ describe('useRecoveryCommands', () => {
     )
   })
 
-  it('should call skipFailedCommand and resolve after a timeout', async () => {
+  it('should call skipFailedCommand and show success toast on success', async () => {
     const { result } = renderHook(() =>
       useRecoveryCommands({
         runId: mockRunId,
         failedCommand: mockFailedCommand,
         failedLabwareUtils: mockFailedLabwareUtils,
         routeUpdateActions: mockRouteUpdateActions,
-        recoveryToastUtils: {} as any,
+        recoveryToastUtils: { makeSuccessToast: mockMakeSuccessToast } as any,
       })
     )
 
-    const consoleSpy = vi.spyOn(console, 'log')
-
     await act(async () => {
       await result.current.skipFailedCommand()
     })
 
-    expect(consoleSpy).toHaveBeenCalledWith('SKIPPING TO NEXT STEP')
-    expect(result.current.skipFailedCommand()).resolves.toBeUndefined()
+    expect(mockResumeRunFromRecovery).toHaveBeenCalledWith(mockRunId)
+    expect(mockMakeSuccessToast).toHaveBeenCalled()
   })
 
   it('should call ignoreErrorKindThisRun and resolve immediately', async () => {

diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts
@@ -29,8 +29,8 @@ export interface UseRecoveryCommandsResult {
   resumeRun: () => void
   /* A terminal recovery command that causes ER to exit as the run status becomes "stop-requested" */
   cancelRun: () => void
-  /* A non-terminal recovery command, but should generally be chained with a resumeRun. */
-  skipFailedCommand: () => Promise<void>
+  /* A terminal recovery command, that causes ER to exit as the run status becomes "running" */
+  skipFailedCommand: () => void
   /* A non-terminal recovery command. Ignore this errorKind for the rest of this run. */
   ignoreErrorKindThisRun: () => Promise<void>
   /* A non-terminal recovery command */
@@ -111,16 +111,11 @@ export function useRecoveryCommands({
     stopRun(runId)
   }, [runId])
 
-  // TODO(jh, 06-18-24): If this command is actually terminal for error recovery, remove the resumeRun currently promise
-  // chained where this is used. Also update docstring in iface.
-  const skipFailedCommand = React.useCallback((): Promise<void> => {
-    console.log('SKIPPING TO NEXT STEP')
-    return new Promise<void>(resolve => {
-      setTimeout(() => {
-        resolve()
-      }, 2000)
+  const skipFailedCommand = React.useCallback((): void => {
+    void resumeRunFromRecovery(runId).then(() => {
+      makeSuccessToast()
     })
-  }, [])
+  }, [runId, resumeRunFromRecovery, makeSuccessToast])
 
   const ignoreErrorKindThisRun = React.useCallback((): Promise<void> => {
     console.log('IGNORING ALL ERRORS OF THIS KIND THIS RUN')