fix(app): only send recovery policy when the run resumes

app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/IgnoreErrorSkipStep.tsx

@@ -1,4 +1,4 @@
-import * as React from 'react'
+import { useState, useEffect } from 'react'
 import head from 'lodash/head'
 import { css } from 'styled-components'
 import { useTranslation } from 'react-i18next'
@@ -62,10 +62,16 @@ export function IgnoreErrorStepHome({
     goBackPrevStep,
   } = routeUpdateActions
 
-  const [selectedOption, setSelectedOption] = React.useState<IgnoreOption>(
+  const [selectedOption, setSelectedOption] = useState<IgnoreOption>(
     head(IGNORE_OPTIONS_IN_ORDER) as IgnoreOption
   )
 
+  // Reset client choice to ignore all errors whenever navigating back to this view. This prevents unexpected
+  // behavior after pressing "go back" and ending up on this screen.
+  useEffect(() => {
+    void ignoreErrorKindThisRun(false)
+  }, [])
+
   // In order to keep routing linear, all extended "skip" flows should be kept as separate recovery options with
   // go back functionality that routes to this view. Those "skip" views encapsulate the generic "skip" view.
   // See the "manually fill well and skip" recovery option for an example.
@@ -84,7 +90,7 @@ export function IgnoreErrorStepHome({
 
   // See ignoreOnce comment.
   const ignoreAlways = (): void => {
-    void ignoreErrorKindThisRun().then(() => {
+    void ignoreErrorKindThisRun(true).then(() => {
       switch (errorKind) {
         case ERROR_KINDS.NO_LIQUID_DETECTED:
           void proceedToRouteAndStep(

app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts

@@ -44,7 +44,7 @@ describe('useRecoveryCommands', () => {
   const mockChainRunCommands = vi.fn().mockResolvedValue([])
   const mockReportActionSelectedResult = vi.fn()
   const mockReportRecoveredRunResult = vi.fn()
-  const mockUpdateErrorRecoveryPolicy = vi.fn()
+  const mockUpdateErrorRecoveryPolicy = vi.fn(() => Promise.resolve())
 
   const props = {
     runId: mockRunId,
@@ -70,7 +70,7 @@ describe('useRecoveryCommands', () => {
       chainRunCommands: mockChainRunCommands,
     } as any)
     vi.mocked(useUpdateErrorRecoveryPolicy).mockReturnValue({
-      updateErrorRecoveryPolicy: mockUpdateErrorRecoveryPolicy,
+      mutateAsync: mockUpdateErrorRecoveryPolicy,
     } as any)
   })
 
@@ -302,12 +302,18 @@ describe('useRecoveryCommands', () => {
       failedCommandByRunRecord: mockFailedCommandWithError,
     }
 
-    const { result } = renderHook(() => useRecoveryCommands(testProps))
+    const { result, rerender } = renderHook(() =>
+      useRecoveryCommands(testProps)
+    )
 
     await act(async () => {
-      await result.current.ignoreErrorKindThisRun()
+      await result.current.ignoreErrorKindThisRun(true)
     })
 
+    rerender()
+
+    result.current.skipFailedCommand()
+
     const expectedPolicyRules = buildIgnorePolicyRules(
       'aspirateInPlace',
       'mockErrorType'
@@ -318,16 +324,33 @@ describe('useRecoveryCommands', () => {
     )
   })
 
-  it('should reject with an error when failedCommand or error is null', async () => {
+  it('should call proceedToRouteAndStep with ERROR_WHILE_RECOVERING route when updateErrorRecoveryPolicy rejects', async () => {
+    const mockFailedCommandWithError = {
+      ...mockFailedCommand,
+      commandType: 'aspirateInPlace',
+      error: {
+        errorType: 'mockErrorType',
+      },
+    }
+
     const testProps = {
       ...props,
-      failedCommand: null,
+      failedCommandByRunRecord: mockFailedCommandWithError,
     }
 
+    mockUpdateErrorRecoveryPolicy.mockRejectedValueOnce(
+      new Error('Update policy failed')
+    )
+
     const { result } = renderHook(() => useRecoveryCommands(testProps))
 
-    await expect(result.current.ignoreErrorKindThisRun()).rejects.toThrow(
-      'Could not execute command. No failed command.'
+    await act(async () => {
+      await result.current.ignoreErrorKindThisRun(true)
+    })
+
+    expect(mockUpdateErrorRecoveryPolicy).toHaveBeenCalled()
+    expect(mockProceedToRouteAndStep).toHaveBeenCalledWith(
+      RECOVERY_MAP.ERROR_WHILE_RECOVERING.ROUTE
     )
   })
 })

app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts

@@ -1,4 +1,4 @@
-import { useCallback } from 'react'
+import { useCallback, useState } from 'react'
 import head from 'lodash/head'
 
 import {
@@ -50,8 +50,10 @@ export interface UseRecoveryCommandsResult {
   cancelRun: () => 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. Ignore this errorKind for the rest of this run.
+   * The server is not informed of recovery policy changes until a terminal recovery command occurs that does not result
+   * in termination of the run. */
+  ignoreErrorKindThisRun: (ignoreErrors: boolean) => Promise<void>
   /* A non-terminal recovery command */
   retryFailedCommand: () => Promise<CommandData[]>
   /* A non-terminal recovery command */
@@ -77,6 +79,8 @@ export function useRecoveryCommands({
   analytics,
   selectedRecoveryOption,
 }: UseRecoveryCommandsParams): UseRecoveryCommandsResult {
+  const [ignoreErrors, setIgnoreErrors] = useState(false)
+
   const { proceedToRouteAndStep } = routeUpdateActions
   const { chainRunCommands } = useChainRunCommands(
     runId,
@@ -86,7 +90,9 @@ export function useRecoveryCommands({
     mutateAsync: resumeRunFromRecovery,
   } = useResumeRunFromRecoveryMutation()
   const { stopRun } = useStopRunMutation()
-  const { updateErrorRecoveryPolicy } = useUpdateErrorRecoveryPolicy(runId)
+  const {
+    mutateAsync: updateErrorRecoveryPolicy,
+  } = useUpdateErrorRecoveryPolicy(runId)
   const { makeSuccessToast } = recoveryToastUtils
 
   const chainRunRecoveryCommands = useCallback(
@@ -182,42 +188,81 @@ export function useRecoveryCommands({
     }
   }, [chainRunRecoveryCommands, failedCommandByRunRecord, failedLabwareUtils])
 
+  const ignoreErrorKindThisRun = (ignoreErrors: boolean): Promise<void> => {
+    setIgnoreErrors(ignoreErrors)
+    return Promise.resolve()
+  }
+
+  // Only send the finalized error policy to the server during a terminal recovery command that does not terminate the run.
+  // If the request to update the policy fails, route to the error modal.
+  const handleIgnoringErrorKind = useCallback((): Promise<void> => {
+    if (ignoreErrors) {
+      if (failedCommandByRunRecord?.error != null) {
+        const ignorePolicyRules = buildIgnorePolicyRules(
+          failedCommandByRunRecord.commandType,
+          failedCommandByRunRecord.error.errorType
+        )
+
+        return updateErrorRecoveryPolicy(ignorePolicyRules)
+          .then(() => Promise.resolve())
+          .catch(() =>
+            Promise.reject(new Error('Failed to update recovery policy.'))
+          )
+      } else {
+        void proceedToRouteAndStep(RECOVERY_MAP.ERROR_WHILE_RECOVERING.ROUTE)
+        return Promise.reject(
+          new Error('Could not execute command. No failed command.')
+        )
+      }
+    } else {
+      return Promise.resolve()
+    }
+  }, [
+    failedCommandByRunRecord?.error?.errorType,
+    failedCommandByRunRecord?.commandType,
+    ignoreErrors,
+  ])
+
   const resumeRun = useCallback((): void => {
-    void resumeRunFromRecovery(runId).then(() => {
-      analytics.reportActionSelectedResult(selectedRecoveryOption, 'succeeded')
-      makeSuccessToast()
-    })
-  }, [runId, resumeRunFromRecovery, makeSuccessToast])
+    void handleIgnoringErrorKind()
+      .then(() => resumeRunFromRecovery(runId))
+      .then(() => {
+        analytics.reportActionSelectedResult(
+          selectedRecoveryOption,
+          'succeeded'
+        )
+        makeSuccessToast()
+      })
+  }, [
+    runId,
+    ignoreErrors,
+    resumeRunFromRecovery,
+    handleIgnoringErrorKind,
+    selectedRecoveryOption,
+    makeSuccessToast,
+  ])
 
   const cancelRun = useCallback((): void => {
     analytics.reportActionSelectedResult(selectedRecoveryOption, 'succeeded')
     stopRun(runId)
   }, [runId])
 
   const skipFailedCommand = useCallback((): void => {
-    void resumeRunFromRecovery(runId).then(() => {
-      analytics.reportActionSelectedResult(selectedRecoveryOption, 'succeeded')
-      makeSuccessToast()
-    })
-  }, [runId, resumeRunFromRecovery, makeSuccessToast])
-
-  const ignoreErrorKindThisRun = useCallback((): Promise<void> => {
-    if (failedCommandByRunRecord?.error != null) {
-      const ignorePolicyRules = buildIgnorePolicyRules(
-        failedCommandByRunRecord.commandType,
-        failedCommandByRunRecord.error.errorType
-      )
-
-      updateErrorRecoveryPolicy(ignorePolicyRules)
-      return Promise.resolve()
-    } else {
-      return Promise.reject(
-        new Error('Could not execute command. No failed command.')
-      )
-    }
+    void handleIgnoringErrorKind().then(() =>
+      resumeRunFromRecovery(runId).then(() => {
+        analytics.reportActionSelectedResult(
+          selectedRecoveryOption,
+          'succeeded'
+        )
+        makeSuccessToast()
+      })
+    )
   }, [
-    failedCommandByRunRecord?.error?.errorType,
-    failedCommandByRunRecord?.commandType,
+    runId,
+    resumeRunFromRecovery,
+    handleIgnoringErrorKind,
+    selectedRecoveryOption,
+    makeSuccessToast,
   ])
 
   const releaseGripperJaws = useCallback((): Promise<CommandData[]> => {